Skip to content

Create new component (Movie) & fix SonarCloud issues - #2

Merged
devpro merged 6 commits into
masterfrom
feature/create-movie-component
Oct 24, 2019
Merged

Create new component (Movie) & fix SonarCloud issues#2
devpro merged 6 commits into
masterfrom
feature/create-movie-component

Conversation

@devpro

@devpro devpro commented Oct 18, 2019

Copy link
Copy Markdown
Owner
  • Create Movie component
  • Create web app (api)
  • Activate authentication with Firebase in web app
  • Fix SonarCloud issues
  • Add exception filter in web app

@devpro devpro added the enhancement New feature or request label Oct 24, 2019
@devpro devpro self-assigned this Oct 24, 2019
@devpro
devpro merged commit 22c46c3 into master Oct 24, 2019
@devpro
devpro deleted the feature/create-movie-component branch October 24, 2019 15:14
devpro added a commit that referenced this pull request Jul 27, 2026
Covers the admin page's provider-free, deterministic surfaces (recommendation
#2 from the testing assessment): page load via the Admin nav link, the System
status panel resolving, unresolved-queue type switching, and the export -> import
round-trip (idempotent upsert-by-id, so it changes no data).

The provider search/link flow is deliberately left to the per-type detail-page
smoke tests (same endpoints, real providers), and a full sync-now poll is left
out as known-flaky on provider latency.

Passes in self-hosted mutating mode.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PxyaxubjRzm2PSLMz7AE24
devpro added a commit that referenced this pull request Jul 30, 2026
Summary

#1 — Killed the N+1 reference lookup (WatchNextController.cs)
The old code awaited a separate FindByIdAsync per in-progress show (N serial round trips to tvshow_reference). Now it collects the candidate shows' reference ids and does one batched FindByIdsAsync, then maps references back per show — the same batched pattern the movie-poster hydration one line below already used.

#2 (cheap) — Stopped fetching the whole episode history (EpisodeRepository + IEpisodeRepository)
The old code pulled every episode the owner has (int.MaxValue), then WatchNextService discarded every non-Current show's episodes in memory — cost scaled with total lifetime watch history. Added a batched, owner-scoped FindByShowIdsAsync(ownerId, showIds); the controller now fetches episodes only for the Current+linked shows that can actually appear in the result. The owner_id + tv_show_id IN(...) filter rides the leading fields of the existing episode_last_watched index.

Output is behaviorally identical — non-Current and unlinked shows were already excluded downstream.

Tests
- New EpisodeRepositoryTest (integration, real MongoDB): verifies FindByShowIdsAsync returns only the requested shows' episodes, is owner-scoped (a different owner tracking the same show id is excluded), and returns empty for no ids. Both pass.
- Updated the FakeEpisodeRepository in TvTimeImportServiceIdempotencyTest to implement the new interface member.
- Full unit suite (288) green; the two new integration tests green against your local Mongo.

Not done (as agreed): the aggregation version of #2 (server-side max (season,episode) per show) and reference caching — hold those unless this isn't enough. If you want to sanity-check the win, the Watch Next page load should now issue a small constant number of queries regardless of how many finished shows/episodes are in the collection.

One note: ITvShowReferenceRepository.FindByIdAsync is no longer called from this controller but is still used elsewhere, so I left it in place.

the movies tab is already in good shape, unlike the show-reference path we just fixed. I checked all three layers:

What's already correct

1. Server-side filter, with a matching index. WantToWatch = true is pushed into the Mongo query (MovieRepository.GetFilter:34), and there's a dedicated partial index movie_want_to_watch (owner_id, want_to_watch, filtered to want_to_watch: true) backing exactly that shape. So it fetches only the watchlist, never the whole movie collection.
2. Batched poster hydration, no N+1. ReferenceImageHydrator.HydrateAsync collects all reference ids and does a single FindByIdsAsync — the same batched pattern I just applied to the shows. No per-movie round trip.

So the two things that mattered on the shows side (a server-side filter and avoiding N+1) are both already right here.

Two marginal nits (I'd leave both)

- The "already seen" exclusion runs in memory. FilterMoviesToWatch drops movies with FirstSeenAt != null in C#, even though GetFilter already supports pushing it down via IsUnseen = true (line 37). Pushing it to the DB would transfer slightly fewer docs — but the set is already just the watchlist (small), the FirstSeenAt condition wouldn't be index-covered anyway (only want_to_watch is in the partial index), and FilterMoviesToWatch is a documented, unit-tested method that exists specifically because the flag goes stale. Net payoff ≈ zero. Not worth the change.
- A wasted count query — but this is not movie-specific and not really an inefficiency of the movies tab per se: FindAllAsync always runs a CountDocumentsAsync before the Find to populate PagedResult metadata that Watch Next discards. Both the shows fetch and the movies fetch pay it (2 round trips where 1 would do). It's indexed on owner_id and cheap; removing it would mean adding a non-paged read method to the repos — more API surface than the tiny saving justifies.

Bottom line: the movies tab doesn't have the kind of scaling problem the episode/reference path had — its cost is bounded by watchlist size, not by total library size. I wouldn't change anything there. If you want me to push the seen-filter down anyway (turning FilterMoviesToWatch into a DB filter) I can, but I'd consider it cosmetic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant