From e6f32596a13136bf9f55614c39056ebe3bc547ea Mon Sep 17 00:00:00 2001 From: soup Date: Sat, 22 Jul 2023 14:49:28 +0200 Subject: [PATCH] fix(releases): delete older than X (#993) * fix(release): DeleteOlder func for zero duration resolves a bug in the `DeleteOlder` function where recent 24-hour data wasn't deleted when set to `delete everything`. We now correctly set the olderThanTimestamp to a future date when duration is zero, ensuring complete deletion of all records. * fix(releases): delete older --------- Co-authored-by: ze0s --- internal/database/release.go | 50 ++++++++++++--------------- internal/domain/release.go | 7 ++-- internal/http/release.go | 47 +++++++++---------------- internal/release/service.go | 11 ++---- web/src/api/APIClient.ts | 10 ++++-- web/src/screens/settings/Releases.tsx | 2 +- 6 files changed, 56 insertions(+), 71 deletions(-) diff --git a/internal/database/release.go b/internal/database/release.go index 519fdc7..3e7502b 100644 --- a/internal/database/release.go +++ b/internal/database/release.go @@ -18,7 +18,6 @@ import ( sq "github.com/Masterminds/squirrel" "github.com/lib/pq" "github.com/rs/zerolog" - "github.com/rs/zerolog/log" ) type ReleaseRepo struct { @@ -561,43 +560,37 @@ CROSS JOIN ( return &rls, nil } -func (repo *ReleaseRepo) Delete(ctx context.Context) error { +func (repo *ReleaseRepo) Delete(ctx context.Context, req *domain.DeleteReleaseRequest) error { tx, err := repo.db.BeginTx(ctx, nil) if err != nil { - return err + return errors.Wrap(err, "could not start transaction") } defer tx.Rollback() - _, err = tx.ExecContext(ctx, `DELETE FROM "release"`) + qb := repo.db.squirrel.Delete("release") + + if req.OlderThan > 0 { + if repo.db.Driver == "sqlite" { + qb = qb.Where(fmt.Sprintf("timestamp < strftime('%%Y-%%m-%%dT%%H:00:00', datetime('now','-%d hours'))", req.OlderThan)) + } else { + // postgres compatible + thresholdTime := time.Now().Add(time.Duration(-req.OlderThan) * time.Hour) + qb = qb.Where(sq.Lt{ + //"timestamp": fmt.Sprintf("(now() - interval '%d hours')", req.OlderThan), + "timestamp": thresholdTime, + }) + } + } + + query, args, err := qb.ToSql() if err != nil { return errors.Wrap(err, "error executing query") } - _, err = tx.ExecContext(ctx, `DELETE FROM release_action_status`) - if err != nil { - return errors.Wrap(err, "error executing query") - } + repo.log.Debug().Str("repo", "release").Str("query", query).Msgf("release.delete: args: %v", args) - if err := tx.Commit(); err != nil { - return errors.Wrap(err, "error commit transaction delete") - } - - return nil -} - -func (repo *ReleaseRepo) DeleteOlder(ctx context.Context, duration int) error { - tx, err := repo.db.BeginTx(ctx, nil) - if err != nil { - return err - } - - defer tx.Rollback() - - olderThanTimestamp := time.Now().UTC().Add(-time.Duration(duration) * time.Hour) - log.Debug().Msgf("Deleting releases older than: %v", olderThanTimestamp) - - result, err := tx.ExecContext(ctx, `DELETE FROM "release" WHERE timestamp < $1`, olderThanTimestamp) + result, err := tx.ExecContext(ctx, query, args...) if err != nil { return errors.Wrap(err, "error executing query") } @@ -606,7 +599,6 @@ func (repo *ReleaseRepo) DeleteOlder(ctx context.Context, duration int) error { if err != nil { return errors.Wrap(err, "error fetching rows affected") } - log.Debug().Msgf("Deleted %d rows from release table", deletedRows) _, err = tx.ExecContext(ctx, `DELETE FROM release_action_status WHERE release_id NOT IN (SELECT id FROM "release")`) if err != nil { @@ -617,6 +609,8 @@ func (repo *ReleaseRepo) DeleteOlder(ctx context.Context, duration int) error { return errors.Wrap(err, "error commit transaction delete") } + repo.log.Debug().Msgf("deleted %d rows from release table", deletedRows) + return nil } diff --git a/internal/domain/release.go b/internal/domain/release.go index 582e7b9..615bc68 100644 --- a/internal/domain/release.go +++ b/internal/domain/release.go @@ -35,8 +35,7 @@ type ReleaseRepo interface { Get(ctx context.Context, req *GetReleaseRequest) (*Release, error) GetIndexerOptions(ctx context.Context) ([]string, error) Stats(ctx context.Context) (*ReleaseStats, error) - Delete(ctx context.Context) error - DeleteOlder(ctx context.Context, duration int) error + Delete(ctx context.Context, req *DeleteReleaseRequest) error CanDownloadShow(ctx context.Context, title string, season int, episode int) (bool, error) GetActionStatus(ctx context.Context, req *GetReleaseActionStatusRequest) (*ReleaseActionStatus, error) @@ -115,6 +114,10 @@ type ReleaseActionStatus struct { Timestamp time.Time `json:"timestamp"` } +type DeleteReleaseRequest struct { + OlderThan int +} + func NewReleaseActionStatus(action *Action, release *Release) *ReleaseActionStatus { s := &ReleaseActionStatus{ ID: 0, diff --git a/internal/http/release.go b/internal/http/release.go index aff9247..57e1215 100644 --- a/internal/http/release.go +++ b/internal/http/release.go @@ -19,8 +19,7 @@ type releaseService interface { FindRecent(ctx context.Context) (res []*domain.Release, err error) GetIndexerOptions(ctx context.Context) ([]string, error) Stats(ctx context.Context) (*domain.ReleaseStats, error) - Delete(ctx context.Context) error - DeleteOlder(ctx context.Context, duration int) error + Delete(ctx context.Context, req *domain.DeleteReleaseRequest) error Retry(ctx context.Context, req *domain.ReleaseActionRetryReq) error } @@ -41,8 +40,7 @@ func (h releaseHandler) Routes(r chi.Router) { r.Get("/recent", h.findRecentReleases) r.Get("/stats", h.getStats) r.Get("/indexers", h.getIndexerOptions) - r.Delete("/all", h.deleteReleases) - r.Delete("/older-than/{duration}", h.deleteOlder) + r.Delete("/", h.deleteReleases) r.Route("/{releaseId}", func(r chi.Router) { r.Post("/actions/{actionStatusId}/retry", h.retryAction) @@ -183,34 +181,23 @@ func (h releaseHandler) getStats(w http.ResponseWriter, r *http.Request) { } func (h releaseHandler) deleteReleases(w http.ResponseWriter, r *http.Request) { - err := h.service.Delete(r.Context()) - if err != nil { - h.encoder.StatusResponse(w, http.StatusInternalServerError, map[string]interface{}{ - "code": "INTERNAL_SERVER_ERROR", - "message": err.Error(), - }) - return + req := domain.DeleteReleaseRequest{} + + olderThanParam := r.URL.Query().Get("olderThan") + if olderThanParam != "" { + duration, err := strconv.Atoi(olderThanParam) + if err != nil { + h.encoder.StatusResponse(w, http.StatusBadRequest, map[string]interface{}{ + "code": "BAD_REQUEST_PARAMS", + "message": "olderThan parameter is invalid", + }) + return + } + req.OlderThan = duration } - h.encoder.NoContent(w) -} - -func (h releaseHandler) deleteOlder(w http.ResponseWriter, r *http.Request) { - durationStr := chi.URLParam(r, "duration") - duration, err := strconv.Atoi(durationStr) - if err != nil { - h.encoder.StatusResponse(w, http.StatusBadRequest, map[string]interface{}{ - "code": "BAD_REQUEST_PARAMS", - "message": "Invalid duration", - }) - return - } - - if err := h.service.DeleteOlder(r.Context(), duration); err != nil { - h.encoder.StatusResponse(w, http.StatusInternalServerError, map[string]interface{}{ - "code": "INTERNAL_SERVER_ERROR", - "message": err.Error(), - }) + if err := h.service.Delete(r.Context(), &req); err != nil { + h.encoder.Error(w, err) return } diff --git a/internal/release/service.go b/internal/release/service.go index b6a91df..ffa35ac 100644 --- a/internal/release/service.go +++ b/internal/release/service.go @@ -25,8 +25,7 @@ type Service interface { Stats(ctx context.Context) (*domain.ReleaseStats, error) Store(ctx context.Context, release *domain.Release) error StoreReleaseActionStatus(ctx context.Context, actionStatus *domain.ReleaseActionStatus) error - Delete(ctx context.Context) error - DeleteOlder(ctx context.Context, duration int) error + Delete(ctx context.Context, req *domain.DeleteReleaseRequest) error Process(release *domain.Release) ProcessMultiple(releases []*domain.Release) Retry(ctx context.Context, req *domain.ReleaseActionRetryReq) error @@ -91,12 +90,8 @@ func (s *service) StoreReleaseActionStatus(ctx context.Context, status *domain.R return s.repo.StoreReleaseActionStatus(ctx, status) } -func (s *service) Delete(ctx context.Context) error { - return s.repo.Delete(ctx) -} - -func (s *service) DeleteOlder(ctx context.Context, duration int) error { - return s.repo.DeleteOlder(ctx, duration) +func (s *service) Delete(ctx context.Context, req *domain.DeleteReleaseRequest) error { + return s.repo.Delete(ctx, req) } func (s *service) Process(release *domain.Release) { diff --git a/web/src/api/APIClient.ts b/web/src/api/APIClient.ts index 136b06b..bbbd195 100644 --- a/web/src/api/APIClient.ts +++ b/web/src/api/APIClient.ts @@ -195,8 +195,14 @@ export const APIClient = { }, indexerOptions: () => appClient.Get("api/release/indexers"), stats: () => appClient.Get("api/release/stats"), - delete: () => appClient.Delete("api/release/all"), - deleteOlder: (duration: number) => appClient.Delete(`api/release/older-than/${duration}`), + delete: (olderThan: number) => { + const params = new URLSearchParams(); + if (olderThan !== undefined && olderThan > 0) { + params.append("olderThan", olderThan.toString()); + } + + return appClient.Delete(`api/release?${params.toString()}`) + }, replayAction: (releaseId: number, actionId: number) => appClient.Post(`api/release/${releaseId}/actions/${actionId}/retry`) }, updates: { diff --git a/web/src/screens/settings/Releases.tsx b/web/src/screens/settings/Releases.tsx index 37a77fe..c2d17c9 100644 --- a/web/src/screens/settings/Releases.tsx +++ b/web/src/screens/settings/Releases.tsx @@ -71,7 +71,7 @@ function DeleteReleases() { const [deleteModalIsOpen, toggleDeleteModal] = useToggle(false); const deleteOlderMutation = useMutation({ - mutationFn: (duration: number) => APIClient.release.deleteOlder(duration), + mutationFn: (olderThan: number) => APIClient.release.delete(olderThan), onSuccess: () => { if (parsedDuration === 0) { toast.custom((t) => (