Files
pgsql-jellyfin/docs/EF_CORE_REMOVAL_ANALYSIS.md
T
wjones 3e5d29225a Refactor SQLite Database Provider
- Removed unused classes and files related to SQLite database provider, including SqliteDesignTimeJellyfinDbFactory, ModelBuilderExtensions, PragmaConnectionInterceptor, AssemblyInfo, SqliteDatabaseProvider, DateTimeKindValueConverter.
- Updated tests to remove dependencies on removed classes and adjusted mocking for configuration sections.
- Added Microsoft.EntityFrameworkCore.Sqlite package reference to test project.
- Improved string handling in tests for better consistency and clarity.
- Refactored logging methods in JellyfinApplicationFactory for better readability and maintainability.
2026-05-03 09:39:00 -04:00

15 KiB
Raw Blame History

EF Core Removal Analysis — pgsql-jellyfin Fork

Date: 2025
Scope: Full codebase analysis of whether EF Core can and should be removed from this PostgreSQL-only fork.
Verdict: Do not remove EF Core. Address specific friction points instead.


1. Usage Inventory

EF Core (IDbContextFactory<JellyfinDbContext>) is injected into 82 call sites across ~35 source files, permeating every layer:

Layer Files
API DatabaseViewsController
Repository (Item) BaseItemRepository, PeopleRepository, MediaStreamRepository, MediaAttachmentRepository, KeyframeRepository, ChapterRepository
Repository (Other) ActivityManager, DeviceManager, DisplayPreferencesManager, UserManager, TrickplayManager, MediaSegmentManager, AuthenticationManager, AuthorizationContext, LibraryOptionsRepository, BackupService
Scheduled Tasks PeopleValidationTask, CleanupUserDataTask, CleanDatabaseScheduledTask, UserDataManager
Migration Routines 13 routines: MigrateActivityLogDb, MigrateAuthenticationDb, MigrateDisplayPreferencesDb, MigrateKeyframeData, MigrateLibraryDb, MigrateLibraryUserData, MigrateRatingLevels, MoveExtractedFiles, RefreshCleanNames, RefreshInternalDateModified, FixDates, CleanMusicArtist, (+ more)
Infrastructure JellyfinMigrationService, PostgresDatabaseProvider, DbContextFactoryHealthCheck, Program.cs, ServiceCollectionExtensions

The JellyfinDbContext itself defines ~35 DbSets spanning 5 schemas (activitylog, authentication, displaypreferences, library, users).


2. Query Complexity — What EF Core Actually Does

2.1 BaseItemRepository.cs — The Core Case

BaseItemRepository.cs is 3,583 lines / 152 KB, the largest file in the repository. It is the performance-critical code path for all library browsing, search, and playback-related queries. It is built entirely on EF Core LINQ.

Key EF-dependent sub-systems inside it:

Method EF Feature Used Purpose
TranslateQuery() (lines 2548+, ~500 lines) LINQ predicate composition over IQueryable<BaseItemEntity> Translates InternalItemsQuery (60+ filter parameters) to SQL WHERE clauses
ApplyOrder() (~80 lines) OrderBy / ThenBy over EF expression trees, OrderMapper.MapOrderByField() Translates ItemSortBy enum to typed SQL ORDER BY
ApplyNavigations() Conditional Include() / ThenInclude() Eagerly loads TvExtras, AudioExtras, LiveTvExtras, Provider, Images, UserData per DTO field set
ApplyGroupingFilter() DistinctBy() → PostgreSQL DISTINCT ON Deduplication for presentation unique keys
GetNextUpSeriesKeys() Join() + GroupBy() + Max() Complex TV "next up" episode query
GetItemValues() Nested Join() + SelectMany() + correlated subquery pattern Artist/Genre/Studio item-value lookups
UpdateOrInsertItemsAsync() CreateExecutionStrategy().ExecuteAsync(), SaveChangesAsync(), Attach().State = EntityState.Modified, AddRange() Transactional batch upsert
Entity state management context.Entry(placeholder).State = EntityState.Detached Upsert conflict resolution
EF.Constant() PostgreSQL-specific plan hint Prevents parameterization of the placeholder UUID to allow index scans

LINQ operator counts in BaseItemRepository.cs (approximate from code review):

  • Where(), Select(), Join(), GroupBy(), Include(), ThenInclude(), OrderBy(): 200+ call sites
  • ExecuteDelete() / ExecuteUpdate() bulk ops: ~25 call sites
  • FromSqlRaw / FromSqlInterpolated: 0 — reads are pure LINQ
  • ExecuteSqlAsync() raw SQL upserts: ~8 (ON CONFLICT upserts for extension tables)

The LINQ query composition is not superficial mapping sugar — TranslateQuery conditionally applies 30+ predicate branches (user data filters, rating filters, type filters, ancestor traversal, search term scoring, tag inheritance, etc.) based on InternalItemsQuery flags. This is the equivalent of a 500-line dynamic SQL builder, expressed in type-safe C#.

2.2 Other Repositories

The other repositories (ActivityManager, UserManager, DeviceManager, etc.) use EF more simply — primarily Where() + Select() + SaveChanges() patterns — but collectively represent hundreds more LINQ call sites.

2.3 Hybrid Usage Already Present

BaseItemRepository.UpdateOrInsertItemsAsync() already uses a hybrid approach: EF Core for reads and change-tracking, raw ExecuteSqlAsync() for ON CONFLICT upserts. This tells us the team is already reaching EF's limits on writes.


3. Migration Infrastructure

3.1 EF Schema Migrations Are DISABLED

Per JellyfinMigrationService.cs (line ~275):

// DISABLED for .NET 11 preview: EF Core database migrations don't work with preview SDK
// Database schema is initialized from SQL scripts in PostgresDatabaseProvider instead
/*
if (stage is JellyfinMigrationStageTypes.CoreInitialisation)
{
    pendingDatabaseMigrations = migrationsAssembly.Migrations...
}
*/

EF's primary justification — schema migration management — is already bypassed. Schema is initialized via sql/schema_init/create_database_schema.sql + psql.

3.2 EF Is Still Used for Code Migration Tracking

JellyfinMigrationService still uses EF to track which code routines have run via __EFMigrationsHistory:

var historyRepository = dbContext.GetService<IHistoryRepository>();
await historyRepository.CreateIfNotExistsAsync();
var appliedMigrations = await dbContext.Database.GetAppliedMigrationsAsync();
await dbContext.Database.ExecuteSqlRawAsync(
    historyRepository.GetInsertScript(new HistoryRow(migrationId, version)));

This usage of IHistoryRepository is what required the SnakeCaseHistoryRepository workaround (custom class overriding MigrationIdColumnName and ProductVersionColumnName, using #pragma warning disable EF1001 because the base class is internal EF infrastructure).


4. Known Friction Points

These are the specific problems that prompted this analysis:

4.1 Snake_case Naming Impedance (FIXED, but fragile)

EF Core defaults to PascalCase. This fork uses snake_case PostgreSQL conventions. Three workarounds are required:

  1. SnakeCaseNamingConventionIPropertyAddedConvention that converts all property names to snake_case at model build time
  2. SnakeCaseHistoryRepository — custom NpgsqlHistoryRepository subclass overriding column name properties; requires #pragma warning disable EF1001 (breaking internal EF API)
  3. Manual table names in model configurations — 31 IEntityTypeConfiguration files; two (LibraryOptionsEntityConfiguration, HomeSectionConfiguration) had PascalCase table names that overrode the convention and caused relation "library.LibraryOptions" does not exist errors at runtime

Risk: Every future upstream EF Core upgrade may break SnakeCaseHistoryRepository since it depends on internal EF infrastructure.

4.2 Execution Strategy Transaction Friction (FIXED, but imposed on all writers)

EnableRetryOnFailure in the PostgreSQL retry strategy makes EF Core incompatible with user-initiated BeginTransaction(). All transactional code must wrap in context.Database.CreateExecutionStrategy().Execute(...). This has already been fixed in MigrateRatingLevels and MigrateKeyframeData but is an ongoing constraint.

4.3 EF-PostgreSQL Translation Gaps

Multiple code comments document EF Core translation failures:

  • DistinctBy() on complex types sometimes cannot be translated → fallback to ApplyGroupingInMemory() using reflection on anonymous types
  • GetItemValues() in some paths falls back to .AsEnumerable() for in-memory filtering
  • GetLatestItemList() had to be split into two queries to avoid untranslatable nested Min() subqueries
  • A documented 165s query regression was caused by EF generating correlated subqueries — fixed by switching to DistinctBy()DISTINCT ON
  • AsSingleQuery() required explicitly to prevent N+1 for multi-table includes

4.4 Inconsistent Table Naming in Raw SQL

UpdateOrInsertItemsAsync() uses raw ExecuteSqlAsync() with PascalCase quoted names:

$@"INSERT INTO library.""BaseItemProviders"" (""ItemId"", ""ProviderId"", ""ProviderValue"")..."
$@"INSERT INTO library.""BaseItemTvExtras"" (""ItemId"", ..."

These PascalCase quoted names ("BaseItemProviders", "BaseItemTvExtras") are inconsistent with the snake_case goal of the fork. The actual table names in PostgreSQL must match these quoted strings exactly — implying the schema init SQL creates them with PascalCase quoted names, not the snake_case convention applied to EF properties.

4.5 Known Performance Anti-Pattern in GetItemValues

// TODO: This is bad refactor!
itemCount = new ItemCounts() {
    SeriesCount = itemCountQuery!.Count(f => f.Type == seriesTypeName),
    EpisodeCount = itemCountQuery!.Count(f => f.Type == episodeTypeName),
    // ... 5 more COUNT queries
}

This executes 7 separate COUNT queries per item in a result set — a classic N+1 problem embedded in an EF projection. This is an EF translation limitation (aggregate counts in a nested SELECT project).


5. Can EF Core Be Removed?

Technically: Yes. Practically: The cost is prohibitive.

A full removal would require:

  1. Rewriting BaseItemRepository.cs (3,583 lines) as raw SQL — either Dapper or direct NpgsqlCommand. The TranslateQuery method alone is a ~500-line dynamic query builder that would need to be rewritten as a SQL string builder. All Include() chains would become explicit JOINs. All ExecuteUpdate/ExecuteDelete calls would become parameterized SQL.

  2. Replacing 31 ModelConfiguration files with SQL schema definitions (partially done via sql/schema_init/).

  3. Replacing the migration tracking system (IHistoryRepository) with a simple SQL table + custom tracker.

  4. Rewriting all other repositories (~15 files) as Dapper or raw Npgsql.

  5. Replacing all IDbContextFactory injection points (~35 files, 82 sites) with new connection/command factories.

Estimated effort: 36 person-months of careful rewrite, plus extensive testing coverage.

Alternatives assessed:

Alternative Pros Cons
Full removal (Dapper + Npgsql) Full control of SQL; no EF friction; better PostgreSQL-native features 3-6 month rewrite; no dynamic LINQ query composition; lose ExecuteUpdate/ExecuteDelete; high regression risk
Hybrid (EF reads, Dapper writes) Eliminates write friction; keeps LINQ query composition Two persistence paradigms; still need EF for reads; does not eliminate snake_case friction
Keep EF, address friction points Minimal disruption; preserves LINQ query composition Ongoing EF+PostgreSQL compatibility work; preview SDK risk
Replace only migration tracking Eliminates SnakeCaseHistoryRepository hack Small gain; does not address execution strategy or translation friction

6. Should EF Core Be Removed?

No — but specific friction points should be addressed deliberately.

6.1 Why EF Core Should Stay

The LINQ query composition in TranslateQuery / ApplyOrder is EF Core's core contribution to this codebase. It provides:

  • Type safety across 60+ filter parameters in InternalItemsQuery
  • Composability — predicates are built incrementally, each branch independently testable
  • Maintainability — 500 lines of C# expression trees is significantly more maintainable than equivalent raw SQL string building
  • ExecuteUpdate/ExecuteDelete — these EF 7+ bulk operations map directly to UPDATE ... WHERE and DELETE ... WHERE without loading entities; replacing these with raw SQL would produce functionally identical code with more boilerplate
  • Navigation property loading (Include) — the conditional navigation loading in ApplyNavigations maps cleanly to the DTO field selection pattern used by the API layer

EF Core's value in this codebase is almost entirely in the read path (LINQ-to-SQL translation) and bulk operations (ExecuteUpdate/ExecuteDelete). Its weaknesses are in the write path and infrastructure (migration history, naming conventions, execution strategy).

In priority order:

Priority 1 — Replace Migration History Tracking (Low effort, high safety gain)

The SnakeCaseHistoryRepository workaround depends on EF internal APIs (#pragma warning disable EF1001). Replace the code migration tracking with a simple dedicated table and Dapper/Npgsql:

CREATE TABLE IF NOT EXISTS public.jellyfin_applied_migrations (
    migration_id VARCHAR(150) PRIMARY KEY,
    applied_at TIMESTAMPTZ NOT NULL DEFAULT now(),
    product_version VARCHAR(32) NOT NULL
);

Remove JellyfinMigrationService's dependency on IHistoryRepository and GetAppliedMigrationsAsync(). This eliminates the most fragile EF workaround.

Priority 2 — Fix Raw SQL Table Names to Match snake_case Schema (Low effort, correctness)

UpdateOrInsertItemsAsync uses library."BaseItemProviders", library."BaseItemTvExtras", etc. If the fork's goal is snake_case naming, verify whether these tables exist as quoted PascalCase or as snake_case in PostgreSQL, and make the raw SQL consistent. Quoted PascalCase in PostgreSQL is case-sensitive and is a footgun.

Priority 3 — Replace Write Path with Raw SQL/Dapper (Medium effort, eliminates execution strategy friction)

The UpdateOrInsertItemsAsync method is already a hybrid (raw SQL for ON CONFLICT upserts + EF for the surrounding logic). Converting the remaining EF write operations (SaveChangesAsync, Attach().State = EntityState.Modified, AddRange()) in this method to direct Npgsql/Dapper calls would:

  • Eliminate the CreateExecutionStrategy().Execute() wrappers
  • Enable direct PostgreSQL ON CONFLICT DO UPDATE throughout
  • Remove the impedance between EF change tracking and PostgreSQL's native upsert semantics

Do not extend this to reads — the LINQ query composition is worth keeping.

Priority 4 — Fix the N+1 in GetItemValues (Medium effort, performance)

The 7 separate COUNT queries per item should be replaced with a single GROUP BY type, COUNT(*) query executed once and pivoted in memory. This is a correctness/performance issue independent of whether EF is removed.


7. Summary

Question Answer
Can EF Core be removed? Yes, technically. Estimated 36 person-months of work.
Should EF Core be removed? No. The LINQ query composition in BaseItemRepository is EF's core value and cost-prohibitive to replace.
Is EF Core being used optimally? No. The write path, migration tracking, and naming convention workarounds are sources of ongoing friction.
What should be done instead? Replace migration tracking with a native SQL table; convert the write path in BaseItemRepository to raw SQL; keep EF for the LINQ read path.
Biggest ongoing risk? SnakeCaseHistoryRepository uses internal EF APIs (EF1001) that may break on EF Core version bumps. Replacing migration tracking eliminates this.
Second biggest risk? EF Core + Npgsql preview packages (11.0.0-preview.1) on net11.0. Schema migrations are already disabled because of this. Pin to stable as soon as net11.0 releases.