-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Refactor stash-box fingerprint submissions #6782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
66a445c
ddd9b67
1bc2b83
49b673d
972efda
505d744
438636a
e29fd8a
81f890b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -144,4 +144,8 @@ models: | |
| fields: | ||
| career_length: | ||
| resolver: true | ||
| FingerprintSubmission: | ||
| fields: | ||
| scene: | ||
| resolver: true | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| package api | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| "github.com/stashapp/stash/pkg/models" | ||
| ) | ||
|
|
||
| func (r *fingerprintSubmissionResolver) Scene(ctx context.Context, obj *models.FingerprintSubmission) (*models.Scene, error) { | ||
| var ret *models.Scene | ||
| if err := r.withReadTxn(ctx, func(ctx context.Context) error { | ||
| var err error | ||
| ret, err = r.repository.Scene.Find(ctx, obj.SceneID) | ||
| return err | ||
| }); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if ret == nil { | ||
| return nil, fmt.Errorf("scene %d not found", obj.SceneID) | ||
| } | ||
|
|
||
| return ret, nil | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import ( | |
| "context" | ||
| "fmt" | ||
| "strconv" | ||
| "time" | ||
|
|
||
| "github.com/stashapp/stash/internal/manager" | ||
| "github.com/stashapp/stash/pkg/logger" | ||
|
|
@@ -14,7 +15,7 @@ import ( | |
| ) | ||
|
|
||
| func (r *mutationResolver) SubmitStashBoxFingerprints(ctx context.Context, input StashBoxFingerprintSubmissionInput) (bool, error) { | ||
| b, err := resolveStashBox(input.StashBoxIndex, input.StashBoxEndpoint) | ||
| b, err := resolveStashBox(input.StashBoxIndex, input.StashBoxEndpoint) //nolint:staticcheck | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
@@ -222,3 +223,121 @@ func (r *mutationResolver) SubmitStashBoxPerformerDraft(ctx context.Context, inp | |
|
|
||
| return res, err | ||
| } | ||
|
|
||
| func (r *mutationResolver) QueueFingerprintSubmission(ctx context.Context, input QueueFingerprintInput) (bool, error) { | ||
| sceneID, err := strconv.Atoi(input.SceneID) | ||
| if err != nil { | ||
| return false, fmt.Errorf("invalid scene ID: %w", err) | ||
| } | ||
|
|
||
| submission := &models.FingerprintSubmission{ | ||
| Endpoint: input.Endpoint, | ||
| StashID: input.StashID, | ||
| SceneID: sceneID, | ||
| Vote: models.FingerprintVote(input.Vote), | ||
| CreatedAt: time.Now(), | ||
| } | ||
|
|
||
| if err := r.withTxn(ctx, func(ctx context.Context) error { | ||
| // Remove any existing submission for this stash ID before creating a new one | ||
| if err := r.repository.FingerprintSubmission.Delete(ctx, input.Endpoint, input.StashID); err != nil { | ||
| return err | ||
| } | ||
| return r.repository.FingerprintSubmission.Create(ctx, submission) | ||
| }); err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| return true, nil | ||
| } | ||
|
|
||
| func (r *mutationResolver) RemoveFingerprintSubmission(ctx context.Context, input RemoveFingerprintInput) (bool, error) { | ||
| if err := r.withTxn(ctx, func(ctx context.Context) error { | ||
| return r.repository.FingerprintSubmission.Delete(ctx, input.Endpoint, input.StashID) | ||
| }); err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| return true, nil | ||
| } | ||
|
|
||
| func (r *mutationResolver) SubmitFingerprintSubmissions(ctx context.Context, stashBoxEndpoint string) (bool, error) { | ||
| b, err := resolveStashBox(nil, &stashBoxEndpoint) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| var submissions []*models.FingerprintSubmission | ||
| if err := r.withReadTxn(ctx, func(ctx context.Context) error { | ||
| var err error | ||
| submissions, err = r.repository.FingerprintSubmission.FindByEndpoint(ctx, stashBoxEndpoint) | ||
| return err | ||
| }); err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| if len(submissions) == 0 { | ||
| return true, nil | ||
| } | ||
|
|
||
| ids := make([]int, len(submissions)) | ||
| for i, sub := range submissions { | ||
| ids[i] = sub.SceneID | ||
| } | ||
|
|
||
| var scenes []*models.Scene | ||
| if err := r.withReadTxn(ctx, func(ctx context.Context) error { | ||
| var err error | ||
| scenes, err = r.sceneService.FindByIDs(ctx, ids, scene.LoadFiles) | ||
| return err | ||
| }); err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| sceneMap := make(map[int]*models.Scene) | ||
| for _, s := range scenes { | ||
| sceneMap[s.ID] = s | ||
| } | ||
|
|
||
| client := r.newStashBoxClient(*b) | ||
|
|
||
| if len(submissions) > 40 { | ||
| // Submit async to avoid timeouts for large batches | ||
| go r.submitFingerprintBatch(client, submissions, sceneMap) | ||
| } else { | ||
| r.submitFingerprintBatch(client, submissions, sceneMap) | ||
| } | ||
|
Comment on lines
+304
to
+309
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Making a couple notes in this area:
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 40 is just a number that seems reasonable. The problem is fingerprints are submitted one by one, so if you have hundreds of them, the browser request times out before they can finish submitting, and then the batch gets cancelled without cleaning up. So past a certain number of fingerprints it's impossible to ever empty the queue. The goroutine runs until it's done or the server restarts. I agree a task would be more appropriate, but this isn't a permanent solution. Once batch submission support is added to stash-box, we can submit all of them at once without being worried about timeouts, and then we can remove the goroutine. Here's the relevant PR: stashapp/stash-box#1009 |
||
|
|
||
| return true, nil | ||
| } | ||
|
|
||
| func (r *mutationResolver) submitFingerprintBatch(client *stashbox.Client, submissions []*models.FingerprintSubmission, sceneMap map[int]*models.Scene) { | ||
| var successfulSubmissions []*models.FingerprintSubmission | ||
| for _, sub := range submissions { | ||
| s, ok := sceneMap[sub.SceneID] | ||
| if !ok { | ||
| logger.Warnf("Scene %d not found for fingerprint submission, skipping", sub.SceneID) | ||
| continue | ||
| } | ||
|
|
||
| if err := client.SubmitFingerprintsWithVote(context.Background(), s, sub.StashID, sub.Vote); err != nil { | ||
| logger.Warnf("Failed to submit fingerprint for scene %d: %v", sub.SceneID, err) | ||
| continue | ||
| } | ||
|
|
||
| successfulSubmissions = append(successfulSubmissions, sub) | ||
| } | ||
|
|
||
| if len(successfulSubmissions) > 0 { | ||
| if err := r.withTxn(context.Background(), func(ctx context.Context) error { | ||
| for _, sub := range successfulSubmissions { | ||
| if err := r.repository.FingerprintSubmission.Delete(ctx, sub.Endpoint, sub.StashID); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| return nil | ||
| }); err != nil { | ||
| logger.Warnf("Failed to delete fingerprint submissions: %v", err) | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| package api | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "github.com/stashapp/stash/pkg/models" | ||
| ) | ||
|
|
||
| func (r *queryResolver) PendingFingerprintSubmissions(ctx context.Context, stashBoxEndpoint string) (ret []*models.FingerprintSubmission, err error) { | ||
| if err := r.withReadTxn(ctx, func(ctx context.Context) error { | ||
| ret, err = r.repository.FingerprintSubmission.FindByEndpoint(ctx, stashBoxEndpoint) | ||
| return err | ||
| }); err != nil { | ||
| return nil, err | ||
| } | ||
| return ret, nil | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| package models | ||
|
|
||
| import ( | ||
| "context" | ||
| "time" | ||
| ) | ||
|
|
||
| type FingerprintVote string | ||
|
|
||
| const ( | ||
| FingerprintVoteValid FingerprintVote = "VALID" | ||
| FingerprintVoteInvalid FingerprintVote = "INVALID" | ||
| ) | ||
|
|
||
| func (e FingerprintVote) IsValid() bool { | ||
| switch e { | ||
| case FingerprintVoteValid, FingerprintVoteInvalid: | ||
| return true | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| func (e FingerprintVote) String() string { | ||
| return string(e) | ||
| } | ||
|
|
||
| type FingerprintSubmission struct { | ||
| Endpoint string `json:"endpoint"` | ||
| StashID string `json:"stash_id"` | ||
| SceneID int `json:"scene_id"` | ||
| Vote FingerprintVote `json:"vote"` | ||
| CreatedAt time.Time `json:"created_at"` | ||
| } | ||
|
|
||
| type FingerprintSubmissionReader interface { | ||
| FindByEndpoint(ctx context.Context, endpoint string) ([]*FingerprintSubmission, error) | ||
| } | ||
|
|
||
| type FingerprintSubmissionWriter interface { | ||
| Create(ctx context.Context, newObject *FingerprintSubmission) error | ||
| Delete(ctx context.Context, endpoint string, stashID string) error | ||
| } | ||
|
|
||
| type FingerprintSubmissionReaderWriter interface { | ||
| FingerprintSubmissionReader | ||
| FingerprintSubmissionWriter | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With these new fields will it break other Stashbox instance requests? From my understanding they don't return a null they would just error out the entire query.
Not 100% on this on, might need some education.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These fields were added to stash-box in version 0.6.0 IIRC, which was in early Jan 25. If the stash-box instance doesn't support the fields, it will fail, but that's a fairly old version..
That said, TPDB does not support these fields, and will break. Supporting them can be done by just stubbing them out and returning 0/false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I can notify TPDB when this gets closer to a merge so that it minimizes breakages.