* Add a space after // comments
For consistency, the commentFormatting lint checker suggests a space
after each // comment block. This commit handles all the spots in
the code where that is needed.
* Rewrite documentation on functions
Use the Go idiom of commenting:
* First sentence declares the purpose.
* First word is the name being declared
The reason this style is preferred is such that grep is able to find
names the user might be interested in. Consider e.g.,
go doc -all pkg/ffmpeg | grep -i transcode
in which case a match will tell you the name of the function you are
interested in.
* Remove old code comment-blocks
There are some commented out old code blocks in the code base. These are
either 3 years old, or 2 years old. By now, I don't think their use is
going to come back any time soon, and Git will track old pieces of
deleted code anyway.
Opt for deletion.
* Reorder imports
Split stdlib imports from non-stdlib imports in files we are touching.
* Use a range over an iteration variable
Probably more go-idiomatic, and the code needed comment-fixing anyway.
* Use time.After rather than rolling our own
The idiom here is common enough that the stdlib contains a function for
it. Use the stdlib function over our own variant.
* Enable the commentFormatting linter
* Don't capitalize local variables
ValidCodecs -> validCodecs
* Capitalize deprecation markers
A deprecated marker should be capitalized.
* Use re.MustCompile for static regexes
If the regex fails to compile, it's a programmer error, and should be
treated as such. The regex is entirely static.
* Simplify else-if constructions
Rewrite
else { if cond {}}
to
else if cond {}
* Use a switch statement to analyze formats
Break an if-else chain. While here, simplify code flow.
Also introduce a proper static error for unsupported image formats,
paving the way for being able to check against the error.
* Rewrite ifElse chains into switch statements
The "Effective Go" https://golang.org/doc/effective_go#switch document
mentions it is more idiomatic to write if-else chains as switches when
it is possible.
Find all the plain rewrite occurrences in the code base and rewrite.
In some cases, the if-else chains are replaced by a switch scrutinizer.
That is, the code sequence
if x == 1 {
..
} else if x == 2 {
..
} else if x == 3 {
...
}
can be rewritten into
switch x {
case 1:
..
case 2:
..
case 3:
..
}
which is clearer for the compiler: it can decide if the switch is
better served by a jump-table then a branch-chain.
* Rewrite switches, introduce static errors
Introduce two new static errors:
* `ErrNotImplmented`
* `ErrNotSupported`
And use these rather than forming new generative errors whenever the
code is called. Code can now test on the errors (since they are static
and the pointers to them wont change).
Also rewrite ifElse chains into switches in this part of the code base.
* Introduce a StashBoxError in configuration
Since all stashbox errors are the same, treat them as such in the code
base. While here, rewrite an ifElse chain.
In the future, it might be beneifical to refactor configuration errors
into one error which can handle missing fields, which context the error
occurs in and so on. But for now, try to get an overview of the error
categories by hoisting them into static errors.
* Get rid of an else-block in transaction handling
If we succesfully `recover()`, we then always `panic()`. This means the
rest of the code is not reachable, so we can avoid having an else-block
here.
It also solves an ifElse-chain style check in the code base.
* Use strings.ReplaceAll
Rewrite
strings.Replace(s, o, n, -1)
into
strings.ReplaceAll(s, o, n)
To make it consistent and clear that we are doing an all-replace in the
string rather than replacing parts of it. It's more of a nitpick since
there are no implementation differences: the stdlib implementation is
just to supply -1.
* Rewrite via gocritic's assignOp
Statements of the form
x = x + e
is rewritten into
x += e
where applicable.
* Formatting
* Review comments handled
Stash-box is a proper noun.
Rewrite a switch into an if-chain which returns on the first error
encountered.
* Use context.TODO() over context.Background()
Patch in the same vein as everything else: use the TODO() marker so we
can search for it later and link it into the context tree/tentacle once
it reaches down to this level in the code base.
* Tell the linter to ignore a section in manager_tasks.go
The section is less readable, so mark it with a nolint for now. Because
the rewrite enables a ifElseChain, also mark that as nolint for now.
* Use strings.ReplaceAll over strings.Replace
* Apply an ifElse rewrite
else { if .. { .. } } rewrite into else if { .. }
* Use switch-statements over ifElseChains
Rewrite chains of if-else into switch statements. Where applicable,
add an early nil-guard to simplify case analysis. Also, in
ScanTask's Start(..), invert the logic to outdent the whole block, and
help the reader: if it's not a scene, the function flow is now far more
local to the top of the function, and it's clear that the rest of the
function has to do with scene management.
* Enable gocritic on the code base.
Disable appendAssign for now since we aren't passing that check yet.
* Document the nolint additions
* Document StashBoxBatchPerformerTagInput
* Use the request context
The code uses context.Background() in a flow where there is a
http.Request. Use the requests context instead.
* Use a true context in the plugin example
Let AddTag/RemoveTag take a context and use that context throughout
the example.
* Avoid the use of context.Background
Prefer context.TODO over context.Background deep in the call chain.
This marks the site as something which we need to context-handle
later, and also makes it clear to the reader that the context is
sort-of temporary in the code base.
While here, be consistent in handling the `act` variable in each
branch of the if .. { .. } .. check.
* Prefer context.TODO over context.Background
For the different scraping operations here, there is a context
higher up the call chain, which we ought to use. Mark the call-sites
as TODO for now, so we can come back later on a sweep of which parts
can be context-lifted.
* Thread context upwards
Initialization requires context for transactions. Thread the context
upward the call chain.
At the intialization call, add a context.TODO since we can't break this
yet. The singleton assumption prevents us from pulling it up into main for
now.
* make tasks context-aware
Change the task interface to understand contexts.
Pass the context down in some of the branches where it is needed.
* Make QueryStashBoxScene context-aware
This call naturally sits inside the request-context. Use it.
* Introduce a context in the JS plugin code
This allows us to use a context for HTTP calls inside the system.
Mark the context with a TODO at top level for now.
* Nitpick error formatting
Use %v rather than %s for error interfaces.
Do not begin an error strong with a capital letter.
* Avoid the use of http.Get in FFMPEG download chain
Since http.Get has no context, it isn't possible to break out or have
policy induced. The call will block until the GET completes. Rewrite
to use a http Request and provide a context.
Thread the context through the call chain for now. provide
context.TODO() at the top level of the initialization chain.
* Make getRemoteCDPWSAddress aware of contexts
Eliminate a call to http.Get and replace it with a context-aware
variant.
Push the context upwards in the call chain, but plug it before the
scraper interface so we don't have to rewrite said interface yet.
Plugged with context.TODO()
* Scraper: make the getImage function context-aware
Use a context, and pass it upwards. Plug it with context.TODO()
up the chain before the rewrite gets too much out of hand for now.
Minor tweaks along the way, remove a call to context.Background()
deep in the call chain.
* Make NOTIFY request context-aware
The call sits inside a Request-handler. So it's natural to use the
requests context as the context for the outgoing HTTP request.
* Use a context in the url scraper code
We are sitting in code which has a context, so utilize it for the
request as well.
* Use a context when checking versions
When we check the version of stash on Github, use a context. Thread
the context up to the initialization routine of the HTTP/GraphQL
server and plug it with a context.TODO() for now.
This paves the way for providing a context to the HTTP server code in a
future patch.
* Make utils func ReadImage context-aware
In almost all of the cases, there is a context in the call chain which
is a natural use. This is true for all the GraphQL mutations.
The exception is in task_stash_box_tag, so plug that task with
context.TODO() for now.
* Make stash-box get context-aware
Thread a context through the call chain until we hit the Client API.
Plug it with context.TODO() there for now.
* Enable the noctx linter
The code is now free of any uncontexted HTTP request. This means we
pass the noctx linter, and we can enable it in the code base.
* Add collation to directory listings. Closes#1806
Introduce a new `locale` arg to the `Query.directory` field. Set "en"
as the default for the field for backward compatibility. Use the given
locale, sending it through a language matcher, and use `x/text` as the
collation engine for the matched language.
Augment the file `ListDirs` call to optionally take a Collator. If the
Collator is given, sort file listings according to the collators rules.
While here, document the GraphQL schema a bit more.
Add matchers by looking at the current front-end locales, and make sure
each of these occur in the matcher list.
* Language matcher touchups
* Avoid having `en-US` twice.
* Introduce `en-AU`.
* Pass IgnoreCase and Numeric collation
Allow the collator to be configured with options. Pass the options
IgnoreCase and Numeric to the collator.
Reduce allocations. Don't create intermediary arrays which we then
consume right after. Manually fuse the arrays and decode straight into
the sum instead.
Furthermore, don't invoke a Reader, but carve out the locations via a
loop, directly.
These two changes taken together speeds up oshash computations by a
factor of 10 according to the benchmark tests. The main reason for
this change is a much lowered memory allocation rate which in turn
improves GC pressure.
While here, add a benchmark for oshash computations and use it for
testing the performance.
The io/ioutil package has been deprecated as of Go 1.16, see
https://golang.org/doc/go1.16#ioutil. This commit replaces the existing
io/ioutil functions with their new definitions in io and os packages.
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
* Log 3 unchecked errors
Rather than ignore errors, log them at
the WARNING log level.
The server has been functioning without these, so assume they are not at
the ERROR level.
* Log errors in concurrency test
If we can't initialize the configuration, treat the test as a failure.
* Undo the errcheck on configurations for now.
* Handle unchecked errors in pkg/manager
* Resolve unchecked errors
* Handle DLNA/DMS unchecked errors
* Handle error checking in concurrency test
Generalize config initialization, so we can initialize a configuration
without writing it to disk.
Use this in the test case, since otherwise the test fails to write.
* Handle the remaining unchecked errors
* Heed gosimple in update test
* Use one-line if-initializer statements
While here, fix a wrong variable capture error.
* testing.T doesn't support %w
use %v instead which is supported.
* Remove unused query builder functions
The Int/String criterion handler functions are now generalized.
Thus, there's no need to keep these functions around anymore.
* Mark filterBuilder.addRecursiveWith nolint
The function is useful in the future and no other refactors are looking
nice.
Keep the function around, but tell the linter to ignore it.
* Remove utils.Btoi
There are no users of this utility function
* Return error on scan failure
If we fail to scan the row when looking for the
unique checksum index, then report the error upwards.
* Fix comments on exported functions
* Fix typos
* Fix startup error
* Avoid redundant logging in migrations
Return the error and let the caller handle the logging of the error if
needed.
While here, defer m.Close() to the function boundary.
* Treat errors as values
Use %v rather than %s and pass the errors directly.
* Generate a wrapped error on stat-failure
* Log 3 unchecked errors
Rather than ignore errors, log them at
the WARNING log level.
The server has been functioning without these, so assume they are not at
the ERROR level.
* Propagate errors upward
Failure in path generation was ignored. Propagate the errors upward the
call stack, so it can be handled at the level of orchestration.
* Warn on errors
Log errors rather than quenching them.
Errors are logged at the Warn-level for now.
* Check error when creating test databases
Use the builtin log package and stop the program fatally on error.
* Add warnings to uncheck task errors
Focus on the task system in a single commit, logging unchecked
errors as warnings.
* Warn-on-error in API routes
Look through the API routes, and make sure errors are being logged if
they occur. Prefer the Warn-log-level because none of these has proven
to be fatal in the system up until now.
* Propagate error when adding Util API
* Propagate error on adding util API
* Return unhandled error
* JS log API: propagate and log errors
* JS Plugins: log GQL addition failures.
* Warn on failure to write to stdin
* Warn on failure to stop task
* Wrap viper.BindEnv
The current viper code only errors if no name is provided, so it should
never fail. Rewrite the code flow to factor through a panic-function.
This removes error warnings from this part of the code.
* Log errors in concurrency test
If we can't initialize the configuration, treat the test as a failure.
* Warn on errors in configuration code
* Plug an unchecked error in gallery zip walking
* Warn on screenshot serving failure
* Warn on encoder screenshot failure
* Warn on errors in path-handling code
* Undo the errcheck on configurations for now.
* Use one-line initializers where applicable
rather than using
err := f()
if err!= nil { ..
prefer the shorter
if err := f(); err != nil { ..
If f() isn't too long of a name, or wraps a function with a body.
* Remove stuff which isn't being used
Some fields, functions and structs aren't in use by the project. Remove
them for janitorial reasons.
* Remove more unused code
All of these functions are currently not in use. Clean up the code by
removal, since the version control has the code if need be.
* Remove unused functions
There's a large set of unused functions and variables in the code base.
Remove these, so it clearer what code to support going forward.
Dead code has been eliminated.
Where applicable, comment const-sections in tests, so reserved
identifiers are still known.
* Fix use-def of tsURL
The first def of tsURL doesn't matter because there's no use before
we hit the 2nd def.
* Remove dead code assignment
Setting logFile = "" is effectively dead code, because there's no use
of it later.
* Comment out found
The variable 'found' is dead in the function (because no post-process
action is following it). Comment it for now.
* Comment dead code in tests
These might provide hints as to what isn't covered at the moment.
* Dead code removal
In the case of constants where iota is involved, move the iota so it
matches the current key values.
This avoids problems with persistently stored key IDs.
* Fix error string capitalization
Error strings often follow another string. Hence, they should not be
capitalized, unless referencing a name.
* Uncapitalize more error strings
While here, use %v on the error directly, which makes it easier to wrap
the error later with %w if need be.
* Uncapitalize more error strings
While here, rename Url to URL as a nitpick.
* utils: vtt: add tests
In lieu of documentation.
* utils: vtt: rewrite for correctness and simplicity
Now handles fractional seconds and negative values correctly.
* Add funscript route to scenes
Adds a /scene/:id/funscript route which serves a funscript file, if present.
Current convention is that these are files stored with the same path, but with the extension ".funscript".
* Look for funscript during scan
This is stored in the Scene record and used to drive UI changes for funscript support.
Currently, that's limited to a funscript link in the Scene's file info.
* Add filtering and sorting for interactive
* Add Handy connection key to interface config
* Add Handy client and placeholder component.
Uses defucilis/thehandy, but not thehandy-react as I had difficulty integrating the context with the existing components.
Instead, the expensive calculation for the server time offset is put in localStorage for reuse.
A debounce was added when scrubbing the video, as otherwise it spammed the Handy API with updates to the current offset.
* Add regex string filter criterion
* Use query interface for auto tagging
* Use Query interface for filename parser
* Remove query regex interfaces
* Add selective auto tag
* Use page size 0 as no limit
Fixed annoyingly noisy transcoding progress log messages.
Fixed minor minor issue with directories than are named "blah.mp4" being detected as files to be scanned.
* Allow clearing of tag images
* Allow clearing of studio images
* Allow clearing of performer images
* Allow clearing of movie images
* Add filtering for missing images
WithoutPants