From 83ad453b4087d27de4758e74b5df12dc1b4377dd Mon Sep 17 00:00:00 2001 From: Oliver Salzburg Date: Tue, 10 Mar 2026 12:01:30 +0100 Subject: [PATCH 1/3] feat: Rewrite module loader The previous approach suffers from provider instances losing their relation to the actual source class. This is caused by abstract base class methods that reduce instances to their base type in unexpected ways. This leads to type assumptions that ultimately don't hold up during runtime, and cause hard-to-analyze issues. This change moves abstract methodology into the concrete types, composing merged types, and then using those composed types instead of base class references. While this change generally seems to perform as expected and correctly (1 test is failing locally), it should likely be cleaned up to integrate better with the rest of the code structure and style. The change is primarily motivated to help illustrate the solution. --- cli.ts | 2 +- lookup.ts | 12 ++++++------ providers/Bandcamp/mod.ts | 8 ++++++-- providers/Beatport/mod.ts | 14 ++++++++++++-- providers/Deezer/mod.ts | 4 +++- providers/Mora/mod.ts | 6 +++++- providers/MusicBrainz/mod.ts | 6 +++++- providers/Ototoy/mod.ts | 8 ++++++-- providers/Spotify/mod.ts | 6 +++++- providers/Tidal/mod.ts | 15 +++++++-------- providers/base.ts | 24 ++---------------------- providers/iTunes/mod.ts | 6 +++++- providers/registry.ts | 36 ++++++++++++++++++++++++++++-------- providers/test_spec.ts | 7 ++++--- server/fresh.gen.ts | 2 +- 15 files changed, 96 insertions(+), 60 deletions(-) diff --git a/cli.ts b/cli.ts index 5dd9b2df..bdad073c 100644 --- a/cli.ts +++ b/cli.ts @@ -13,7 +13,7 @@ if (args._.length === 1) { let specifier: GTIN | URL = args._[0]; try { - specifier = new URL(specifier); + specifier = new URL(String(specifier)); } catch { // not a valid URL, treat specifier as GTIN } diff --git a/lookup.ts b/lookup.ts index b033c768..b03ac50c 100644 --- a/lookup.ts +++ b/lookup.ts @@ -76,7 +76,7 @@ export class CombinedReleaseLookup { }); return false; } else { - this.queuedReleases.push(provider.getRelease(id, this.options)); + this.queuedReleases.push(() => provider.releaseLookup(id, this.options).getRelease()); this.queuedProviderNames.add(displayName); this.gtinLookupProviders.delete(provider.internalName); return true; @@ -102,7 +102,7 @@ export class CombinedReleaseLookup { }); return false; } else { - this.queuedReleases.push(provider.getRelease(url, this.options)); + this.queuedReleases.push(() => provider.releaseLookup(url, this.options).getRelease()); this.queuedProviderNames.add(displayName); this.gtinLookupProviders.delete(provider.internalName); return true; @@ -143,7 +143,7 @@ export class CombinedReleaseLookup { const provider = providers.findByName(providerName); if (provider) { if (provider.getQuality('GTIN lookup') != FeatureQuality.MISSING) { - this.queuedReleases.push(provider.getRelease(['gtin', this.gtin], this.options)); + this.queuedReleases.push(() => provider.releaseLookup(['gtin', gtin.toString()], this.options).getRelease()); this.queuedProviderNames.add(provider.name); } else { this.messages.push({ @@ -183,7 +183,7 @@ export class CombinedReleaseLookup { return this.cachedReleaseMap; } - const releaseResults = await Promise.allSettled(this.queuedReleases); + const releaseResults = await Promise.allSettled(this.queuedReleases.map((_) => _())); const releasesOrErrors: Array = await Promise.all(releaseResults.map(async (result) => { if (result.status === 'fulfilled') { return result.value; @@ -321,7 +321,7 @@ export class CombinedReleaseLookup { /** Internal names of providers which will be used for GTIN lookups. */ private gtinLookupProviders: Set; - private queuedReleases: Promise[] = []; + private queuedReleases: Array<() => Promise> = []; /** Display names of all queued providers. */ private queuedProviderNames = new Set(); @@ -349,7 +349,7 @@ export function getReleaseByUrl(url: URL, options?: ReleaseOptions): Promise { readonly name = 'Bandcamp'; readonly supportedUrls = new URLPattern({ @@ -52,7 +54,9 @@ export default class BandcampProvider extends MetadataProvider { month: 9, }; - readonly releaseLookup = BandcampReleaseLookup; + releaseLookup(specifier: ReleaseSpecifier, options: ReleaseOptions = {}) { + return new BandcampReleaseLookup(this, specifier, options); + } override extractEntityFromUrl(url: URL): EntityId | undefined { const albumResult = this.supportedUrls.exec(url); diff --git a/providers/Beatport/mod.ts b/providers/Beatport/mod.ts index e23c44d5..a0e59bcd 100644 --- a/providers/Beatport/mod.ts +++ b/providers/Beatport/mod.ts @@ -1,5 +1,13 @@ import type { Artist, BeatportNextData, BeatportRelease, Release, Track } from './json_types.ts'; -import type { ArtistCreditName, EntityId, HarmonyRelease, HarmonyTrack, LinkType } from '@/harmonizer/types.ts'; +import type { + ArtistCreditName, + EntityId, + HarmonyRelease, + HarmonyTrack, + LinkType, + ReleaseOptions, + ReleaseSpecifier, +} from '@/harmonizer/types.ts'; import { variousArtists } from '@/musicbrainz/special_entities.ts'; import { CacheEntry, MetadataProvider, ReleaseLookup } from '@/providers/base.ts'; import { DurationPrecision, FeatureQuality, FeatureQualityMap } from '@/providers/features.ts'; @@ -30,7 +38,9 @@ export default class BeatportProvider extends MetadataProvider { recording: 'track', }; - readonly releaseLookup = BeatportReleaseLookup; + releaseLookup(specifier: ReleaseSpecifier, options: ReleaseOptions = {}) { + return new BeatportReleaseLookup(this, specifier, options); + } override readonly launchDate: PartialDate = { year: 2005, diff --git a/providers/Deezer/mod.ts b/providers/Deezer/mod.ts index 93ef3987..55eda176 100644 --- a/providers/Deezer/mod.ts +++ b/providers/Deezer/mod.ts @@ -59,7 +59,9 @@ export default class DeezerProvider extends MetadataApiProvider { override readonly availableRegions = new Set(availableRegions); - readonly releaseLookup = DeezerReleaseLookup; + releaseLookup(specifier: ReleaseSpecifier, options: ReleaseOptions = {}) { + return new DeezerReleaseLookup(this, specifier, options); + } override readonly launchDate: PartialDate = { year: 2007, diff --git a/providers/Mora/mod.ts b/providers/Mora/mod.ts index c7bab6a4..6c99140b 100644 --- a/providers/Mora/mod.ts +++ b/providers/Mora/mod.ts @@ -7,6 +7,8 @@ import type { HarmonyRelease, HarmonyTrack, LinkType, + ReleaseOptions, + ReleaseSpecifier, } from '@/harmonizer/types.ts'; import { type CacheEntry, MetadataProvider, ReleaseLookup } from '@/providers/base.ts'; import { DurationPrecision, FeatureQuality, FeatureQualityMap } from '@/providers/features.ts'; @@ -49,7 +51,9 @@ export default class MoraProvider extends MetadataProvider { month: 4, }; - readonly releaseLookup = MoraReleaseLookup; + releaseLookup(specifier: ReleaseSpecifier, options: ReleaseOptions = {}) { + return new MoraReleaseLookup(this, specifier, options); + } constructUrl(entity: EntityId): URL { return new URL(`https://mora.jp/${entity.type}/${entity.id}/`); diff --git a/providers/MusicBrainz/mod.ts b/providers/MusicBrainz/mod.ts index 738a3b2c..90b4ceb5 100644 --- a/providers/MusicBrainz/mod.ts +++ b/providers/MusicBrainz/mod.ts @@ -6,6 +6,8 @@ import type { HarmonyTrack, MediumFormat, ReleaseGroupType, + ReleaseOptions, + ReleaseSpecifier, } from '@/harmonizer/types.ts'; import { type ApiQueryOptions, @@ -60,7 +62,9 @@ export default class MusicBrainzProvider extends MetadataApiProvider { release: 'release', }; - readonly releaseLookup = MusicBrainzReleaseLookup; + releaseLookup(specifier: ReleaseSpecifier, options: ReleaseOptions = {}) { + return new MusicBrainzReleaseLookup(this, specifier, options); + } readonly apiBaseUrl = musicbrainzApiBaseUrl; diff --git a/providers/Ototoy/mod.ts b/providers/Ototoy/mod.ts index 250626d7..26e0513a 100644 --- a/providers/Ototoy/mod.ts +++ b/providers/Ototoy/mod.ts @@ -8,6 +8,8 @@ import type { HarmonyTrack, Label, LinkType, + ReleaseOptions, + ReleaseSpecifier, } from '@/harmonizer/types.ts'; import { type CacheEntry, MetadataProvider, ReleaseLookup } from '@/providers/base.ts'; import { DurationPrecision, FeatureQuality, FeatureQualityMap } from '@/providers/features.ts'; @@ -16,7 +18,7 @@ import { ProviderError, ResponseError } from '@/utils/errors.ts'; import { DOMParser, HTMLDocument } from '@b-fuze/deno-dom'; import { parseDuration } from '../../utils/time.ts'; -export default class OtotoyProvider extends MetadataProvider { +export default class OtotoyProvider extends MetadataProvider { readonly name = 'OTOTOY'; readonly supportedUrls = new URLPattern({ @@ -50,7 +52,9 @@ export default class OtotoyProvider extends MetadataProvider { month: 8, }; - readonly releaseLookup = OtotoyReleaseLookup; + releaseLookup(specifier: ReleaseSpecifier, options: ReleaseOptions = {}) { + return new OtotoyReleaseLookup(this, specifier, options); + } override extractEntityFromUrl(url: URL): EntityId | undefined { const packageResult = this.supportedUrls.exec(url); diff --git a/providers/Spotify/mod.ts b/providers/Spotify/mod.ts index 891ee83f..9542dbb6 100644 --- a/providers/Spotify/mod.ts +++ b/providers/Spotify/mod.ts @@ -35,6 +35,8 @@ import type { HarmonyRelease, HarmonyTrack, LinkType, + ReleaseOptions, + ReleaseSpecifier, } from '@/harmonizer/types.ts'; // See https://developer.spotify.com/documentation/web-api @@ -66,7 +68,9 @@ export default class SpotifyProvider extends MetadataApiProvider { override readonly availableRegions = new Set(availableRegions); - readonly releaseLookup = SpotifyReleaseLookup; + releaseLookup(specifier: ReleaseSpecifier, options: ReleaseOptions = {}) { + return new SpotifyReleaseLookup(this, specifier, options); + } override readonly launchDate: PartialDate = { year: 2008, diff --git a/providers/Tidal/mod.ts b/providers/Tidal/mod.ts index b2dd6829..a39bf6b6 100644 --- a/providers/Tidal/mod.ts +++ b/providers/Tidal/mod.ts @@ -70,7 +70,9 @@ export default class TidalProvider extends MetadataApiProvider { override readonly availableRegions = new Set(availableRegions); - protected releaseLookup: typeof TidalV1ReleaseLookup | typeof TidalV2ReleaseLookup = TidalV2ReleaseLookup; + releaseLookup(specifier: ReleaseSpecifier, options: ReleaseOptions = {}) { + return new TidalV2ReleaseLookup(this, specifier, options); + } override readonly launchDate: PartialDate = { year: 2014, @@ -78,14 +80,11 @@ export default class TidalProvider extends MetadataApiProvider { day: 28, }; - override getRelease(specifier: ReleaseSpecifier, options: ReleaseOptions = {}): Promise { + getRelease(specifier: ReleaseSpecifier, options: ReleaseOptions = {}): Promise { if (!options.snapshotMaxTimestamp || options.snapshotMaxTimestamp > tidalV1MaxTimestamp) { - this.releaseLookup = TidalV2ReleaseLookup; - } else { - this.releaseLookup = TidalV1ReleaseLookup; - } - - return super.getRelease(specifier, options); + return new TidalV2ReleaseLookup(this,specifier,options).getRelease(); + } + return new TidalV1ReleaseLookup(this,specifier,options).getRelease(); } constructUrl(entity: EntityId): URL { diff --git a/providers/base.ts b/providers/base.ts index 718b790c..739940d8 100644 --- a/providers/base.ts +++ b/providers/base.ts @@ -58,7 +58,7 @@ export type MetadataProviderConstructor = new ( * Abstract metadata provider which looks up releases from a specific source. * Converts the raw metadata into a common representation. */ -export abstract class MetadataProvider { +export abstract class MetadataProvider | unknown = unknown> { constructor({ rateLimitInterval = null, concurrentRequests = 1, @@ -104,23 +104,11 @@ export abstract class MetadataProvider { /** Maps MusicBrainz entity types to the corresponding entity types of the provider. */ abstract readonly entityTypeMap: Record; - protected abstract releaseLookup: ReleaseLookupConstructor; - /** Country codes of regions in which the provider offers its services (optional). */ readonly availableRegions?: Set; readonly launchDate: PartialDate = {}; - /** Looks up the release which is identified by the given specifier (URL, GTIN/barcode or provider ID). */ - getRelease(specifier: ReleaseSpecifier, options: ReleaseOptions = {}): Promise { - try { - const lookup = new this.releaseLookup(this, specifier, options); - return lookup.getRelease(); - } catch (error) { - return Promise.reject(error); - } - } - /** Checks whether the provider supports the domain of the given URL. */ supportsDomain(url: URL | string): boolean { return new URLPattern({ hostname: this.supportedUrls.hostname }).test(url); @@ -322,15 +310,7 @@ export abstract class MetadataProvider { } } -type ReleaseLookupConstructor = new ( - // It is probably impossible to specify the correct provider subclass here. - // deno-lint-ignore no-explicit-any - provider: any, - specifier: ReleaseSpecifier, - options: ReleaseOptions, -) => ReleaseLookup; - -export abstract class ReleaseLookup { +export abstract class ReleaseLookup, RawRelease> { /** Initializes the release lookup for the given release specifier. */ constructor( protected provider: Provider, diff --git a/providers/iTunes/mod.ts b/providers/iTunes/mod.ts index 17541ffe..d6466092 100644 --- a/providers/iTunes/mod.ts +++ b/providers/iTunes/mod.ts @@ -17,6 +17,8 @@ import type { HarmonyRelease, LinkType, ReleaseGroupType, + ReleaseOptions, + ReleaseSpecifier, } from '@/harmonizer/types.ts'; // See https://developer.apple.com/library/archive/documentation/AudioVideo/Conceptual/iTuneSearchAPI @@ -44,7 +46,9 @@ export default class iTunesProvider extends MetadataApiProvider { override readonly availableRegions = new Set(availableRegions); - readonly releaseLookup = iTunesReleaseLookup; + releaseLookup(specifier: ReleaseSpecifier, options: ReleaseOptions = {}) { + return new iTunesReleaseLookup(this, specifier, options); + } override readonly launchDate: PartialDate = { year: 2003, diff --git a/providers/registry.ts b/providers/registry.ts index 3369c071..e17c7f6e 100644 --- a/providers/registry.ts +++ b/providers/registry.ts @@ -1,8 +1,28 @@ -import type { MetadataProvider, MetadataProviderConstructor } from './base.ts'; +import type { MetadataProvider } from './base.ts'; import { FeatureQuality, type ProviderFeature } from './features.ts'; import type { AppInfo } from '@/app.ts'; import type { ExternalEntityId } from '@/harmonizer/types.ts'; import { SnapStorage } from 'snap-storage'; +import type BandcampProvider from './Bandcamp/mod.ts'; +import type BeatportProvider from './Beatport/mod.ts'; +import type DeezerProvider from './Deezer/mod.ts'; +import type iTunesProvider from './iTunes/mod.ts'; +import type MoraProvider from './Mora/mod.ts'; +import type MusicBrainzProvider from './MusicBrainz/mod.ts'; +import type OtotoyProvider from './Ototoy/mod.ts'; +import type SpotifyProvider from './Spotify/mod.ts'; +import type TidalProvider from './Tidal/mod.ts'; + +export type MetadataProviders = + | BandcampProvider + | BeatportProvider + | DeezerProvider + | iTunesProvider + | MoraProvider + | MusicBrainzProvider + | OtotoyProvider + | SpotifyProvider + | TidalProvider; export interface ProviderRegistryOptions { /** Information about the application which is passed to each provider. */ @@ -19,8 +39,8 @@ export class ProviderRegistry { } /** Adds an instance of the given provider to the registry. */ - add(Provider: MetadataProviderConstructor) { - const provider = new Provider({ snaps: this.#snaps, appInfo: this.#appInfo }); + add MetadataProviders)>(ProviderClass: Provider) { + const provider = new ProviderClass({ snaps: this.#snaps, appInfo: this.#appInfo }); const { name, internalName } = provider; if (this.#displayNames.has(name)) { @@ -39,7 +59,7 @@ export class ProviderRegistry { } /** Adds an instance for each of the given providers to the registry. */ - addMultiple(...providers: MetadataProviderConstructor[]) { + addMultiple(...providers: Array MetadataProviders)>) { for (const Provider of providers) { this.add(Provider); } @@ -100,13 +120,13 @@ export class ProviderRegistry { } /** Finds a registered provider by name (internal name or display name). */ - findByName(name: string): MetadataProvider | undefined { + findByName(name: string): MetadataProviders | undefined { const internalName = this.toInternalName(name); return internalName ? this.#providerMap[internalName] : undefined; } /** Finds a registered provider which supports the domain of the given URL. */ - findByUrl(url: URL | string): MetadataProvider | undefined { + findByUrl(url: URL | string): MetadataProviders | undefined { return this.#providerList.find((provider) => provider.supportsDomain(url)); } @@ -142,8 +162,8 @@ export class ProviderRegistry { } #appInfo: AppInfo | undefined; - #providerList: MetadataProvider[] = []; - #providerMap: Record = {}; + #providerList: MetadataProviders[] = []; + #providerMap: Record = {}; #displayNames = new Set(); #internalNames = new Set(); #displayToInternal: Record = {}; diff --git a/providers/test_spec.ts b/providers/test_spec.ts index f88556d1..a8e458c6 100644 --- a/providers/test_spec.ts +++ b/providers/test_spec.ts @@ -15,6 +15,7 @@ import { assertThrows } from 'std/assert/assert_throws.ts'; import { filterValues } from '@std/collections/filter-values'; import { describe, it } from '@std/testing/bdd'; import { preferArray } from 'utils/array/scalar.js'; +import type { MetadataProviders } from './registry.ts'; /** Specification which describes the expected behavior of a {@linkcode MetadataProvider}. */ export interface ProviderSpecification { @@ -36,7 +37,7 @@ export interface ProviderSpecification { } /** Registers test suites to compare the given provider against its specification. */ -export function describeProvider(provider: MetadataProvider, spec: ProviderSpecification) { +export function describeProvider(provider: MetadataProviders, spec: ProviderSpecification) { describeEntityUrlExtraction(provider, spec.urls); describeEntityUrlConstruction(provider, spec.urls); it('rejects invalid release IDs', () => { @@ -140,7 +141,7 @@ export interface ReleaseLookupTest { assert: (actualRelease: HarmonyRelease, context: Deno.TestContext) => void | Promise; } -function describeReleaseLookups(provider: MetadataProvider, tests: ReleaseLookupTest[]) { +function describeReleaseLookups(provider: MetadataProviders, tests: ReleaseLookupTest[]) { describe('release lookup', () => { for (const test of tests) { let { description, release } = test; @@ -159,7 +160,7 @@ function describeReleaseLookups(provider: MetadataProvider, tests: ReleaseLookup it(description, { only: test.only, }, async (context) => { - const actualRelease = await provider.getRelease(release, test.options); + const actualRelease = await provider.releaseLookup(release, test.options).getRelease(); // Remove properties which are not stable across multiple runs. actualRelease.info.providers.forEach((providerInfo) => { diff --git a/server/fresh.gen.ts b/server/fresh.gen.ts index 9cb5b4cf..53d5b713 100644 --- a/server/fresh.gen.ts +++ b/server/fresh.gen.ts @@ -15,7 +15,7 @@ import * as $OpenAllLinks from './islands/OpenAllLinks.tsx'; import * as $PersistentInput from './islands/PersistentInput.tsx'; import * as $RegionList from './islands/RegionList.tsx'; import * as $ReleaseSeeder from './islands/ReleaseSeeder.tsx'; -import { type Manifest } from '$fresh/server.ts'; +import type { Manifest } from '$fresh/server.ts'; const manifest = { routes: { From ecf5a36c1ca513ee2f9a4f19c6b4ab9b7077d426 Mon Sep 17 00:00:00 2001 From: Oliver Salzburg Date: Wed, 11 Mar 2026 14:33:24 +0100 Subject: [PATCH 2/3] style: Fix trailing whitespace --- providers/Tidal/mod.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/providers/Tidal/mod.ts b/providers/Tidal/mod.ts index a39bf6b6..004c17c8 100644 --- a/providers/Tidal/mod.ts +++ b/providers/Tidal/mod.ts @@ -82,9 +82,9 @@ export default class TidalProvider extends MetadataApiProvider { getRelease(specifier: ReleaseSpecifier, options: ReleaseOptions = {}): Promise { if (!options.snapshotMaxTimestamp || options.snapshotMaxTimestamp > tidalV1MaxTimestamp) { - return new TidalV2ReleaseLookup(this,specifier,options).getRelease(); - } - return new TidalV1ReleaseLookup(this,specifier,options).getRelease(); + return new TidalV2ReleaseLookup(this, specifier, options).getRelease(); + } + return new TidalV1ReleaseLookup(this, specifier, options).getRelease(); } constructUrl(entity: EntityId): URL { From 795596e4e6006fcdf5050085b70b5b9461f12e65 Mon Sep 17 00:00:00 2001 From: Oliver Salzburg Date: Fri, 13 Mar 2026 07:38:10 +0100 Subject: [PATCH 3/3] fix: class/instance mixups --- providers/registry.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/providers/registry.ts b/providers/registry.ts index e17c7f6e..fc741f7e 100644 --- a/providers/registry.ts +++ b/providers/registry.ts @@ -13,6 +13,8 @@ import type OtotoyProvider from './Ototoy/mod.ts'; import type SpotifyProvider from './Spotify/mod.ts'; import type TidalProvider from './Tidal/mod.ts'; +// deno-lint-ignore no-explicit-any +export type ConstructorOf = new (...args: Array) => TConstructed; export type MetadataProviders = | BandcampProvider | BeatportProvider @@ -23,6 +25,16 @@ export type MetadataProviders = | OtotoyProvider | SpotifyProvider | TidalProvider; +export type MetadataProviderClasses = + | ConstructorOf + | ConstructorOf + | ConstructorOf + | ConstructorOf + | ConstructorOf + | ConstructorOf + | ConstructorOf + | ConstructorOf + | ConstructorOf; export interface ProviderRegistryOptions { /** Information about the application which is passed to each provider. */ @@ -39,7 +51,7 @@ export class ProviderRegistry { } /** Adds an instance of the given provider to the registry. */ - add MetadataProviders)>(ProviderClass: Provider) { + add(ProviderClass: MetadataProviderClasses) { const provider = new ProviderClass({ snaps: this.#snaps, appInfo: this.#appInfo }); const { name, internalName } = provider; @@ -59,7 +71,7 @@ export class ProviderRegistry { } /** Adds an instance for each of the given providers to the registry. */ - addMultiple(...providers: Array MetadataProviders)>) { + addMultiple(...providers: Array) { for (const Provider of providers) { this.add(Provider); }