Merge pull request #634 from Arthi-chaud/server/opti-metadata-fetching

Server: Prevent getting the same external id twice
Arthi-chaud · Feb 8, 2024 · 224a0f7 · 224a0f7
2 parents c9f1d7a + 379dad3
commit 224a0f7
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 16 deletions.
diff --git a/front/src/pages/releases/[slugOrId].tsx b/front/src/pages/releases/[slugOrId].tsx
@@ -260,9 +260,11 @@ const ReleasePage = (props: InferSSRProps<typeof getServerSideProps>) => {
 	);
 	const externalIdWithDescription = useMemo(
 		() =>
-			album.data?.externalIds.find(
-				({ description }) => description !== null,
-			),
+			album.data?.externalIds
+				.filter(
+					({ provider }) => provider.name.toLowerCase() !== "discogs",
+				)
+				.find(({ description }) => description !== null),
 		[album.data],
 	);
 	const externalIds = useMemo(() => {

diff --git a/server/src/genre/genre.service.ts b/server/src/genre/genre.service.ts
@@ -240,11 +240,15 @@ export default class GenreService extends RepositoryService<
 				select: {
 					id: true,
 					_count: {
-						select: { songs: true },
+						select: { songs: true, albums: true },
 					},
 				},
 			})
-			.then((genres) => genres.filter((genre) => !genre._count.songs));
+			.then((genres) =>
+				genres.filter(
+					(genre) => !genre._count.songs && !genre._count.albums,
+				),
+			);
 
 		await Promise.all(emptyGenres.map(({ id }) => this.delete({ id })));
 	}

diff --git a/server/src/providers/external-id.provider.ts b/server/src/providers/external-id.provider.ts
@@ -16,6 +16,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+/* eslint-disable max-depth */
 import { Inject, Injectable, forwardRef } from "@nestjs/common";
 import PrismaService from "src/prisma/prisma.service";
 import ProviderService from "./provider.service";
@@ -110,7 +111,11 @@ export default class ExternalIdService {
 			(provider, mbid) => provider.getAlbumMetadataByIdentifier(mbid),
 			(provider, providerId) => {
 				if (!album.artist) {
-					return provider.getAlbumMetadataByName(album.name);
+					return provider.getAlbumMetadataByName(
+						album.name,
+						undefined,
+						album.type,
+					);
 				}
 				const parentArtistIdentifier = album.artist.externalIds.find(
 					(externalId) => externalId.providerId == providerId,
@@ -119,6 +124,7 @@ export default class ExternalIdService {
 				return provider.getAlbumMetadataByName(
 					album.name,
 					parentArtistIdentifier,
+					album.type,
 				);
 			},
 			(provider, mbid) => provider.getAlbumEntry(mbid),
@@ -301,8 +307,17 @@ export default class ExternalIdService {
 					musicbrainzProvider,
 					resourceMBID.value,
 				);
-
-				Array.of(...providersToReach).forEach(async (otherProvider) => {
+				for (const otherProvider of Array.of(...providersToReach)) {
+					const providerId = this.providerService.getProviderId(
+						otherProvider.name,
+					);
+					if (
+						resource.externalIds.find(
+							(id) => id.providerId == providerId,
+						)
+					) {
+						continue;
+					}
 					const relation = resourceEntry.relations?.find(
 						(rel) =>
 							rel.type ==
@@ -311,23 +326,20 @@ export default class ExternalIdService {
 					);
 
 					if (relation?.url === undefined) {
-						return;
+						continue;
 					}
 					const identifier = parseIdentifierFromUrl(
 						relation.url.resource,
 						otherProvider,
 					);
 					if (identifier == null) {
-						return;
+						continue;
 					}
 					try {
 						const metadata = await getResourceMetadataByIdentifier(
 							otherProvider,
 							identifier,
 						);
-						const providerId = this.providerService.getProviderId(
-							otherProvider.name,
-						);
 
 						newIdentifiers.push(
 							formatResourceMetadata(metadata, providerId),
@@ -336,9 +348,9 @@ export default class ExternalIdService {
 							(toReach) => toReach.name !== otherProvider.name,
 						);
 					} catch {
-						return;
+						continue;
 					}
-				});
+				}
 
 				const wikiDataId = resourceEntry.relations
 					?.map(

diff --git a/server/src/providers/iprovider.ts b/server/src/providers/iprovider.ts
@@ -18,6 +18,7 @@
 
 import {
 	AlbumExternalId,
+	AlbumType,
 	ArtistExternalId,
 	ReleaseExternalId,
 	SongExternalId,
@@ -88,6 +89,7 @@ export default abstract class IProvider<SettingsType = unknown> {
 	getAlbumMetadataByName(
 		_albumName: string,
 		_artistName?: string,
+		_albumType?: AlbumType,
 	): Promise<AlbumMetadata> {
 		throw new ProviderMethodNotAvailableError(this.name);
 	}

diff --git a/server/src/providers/musicbrainz/musicbrainz.provider.spec.ts b/server/src/providers/musicbrainz/musicbrainz.provider.spec.ts
@@ -126,6 +126,30 @@ describe("MusicBrainz Provider", () => {
 			expect(id.genres).toContain("Electronic");
 			expect(id.genres).toContain("Dance-pop");
 		});
+		it("should get compilation (and not single) identifier", async () => {
+			const id = await musicBrainzProvider.getAlbumMetadataByName(
+				"Celebration",
+				"79239441-bfd5-4981-a70c-55c3f15c1287",
+				"Compilation",
+			);
+			expect(id.value).toBe("bd252c17-ff32-4369-8e73-4d0a65a316bd");
+		});
+		it("should get Single identifier", async () => {
+			const id = await musicBrainzProvider.getAlbumMetadataByName(
+				"Celebration - Single",
+				"79239441-bfd5-4981-a70c-55c3f15c1287",
+				"Single",
+			);
+			expect(id.value).toBe("8ab9625e-5e09-4e4b-afda-3e64189b624b");
+		});
+		it("should get album (and not single) identifier", async () => {
+			const id = await musicBrainzProvider.getAlbumMetadataByName(
+				"Protection",
+				"10adbe5e-a2c0-4bf3-8249-2b4cbf6e6ca8",
+				"StudioRecording",
+			);
+			expect(id.value).toBe("ded46e46-788d-3c1f-b21b-9f5e9c37b1bc");
+		});
 		it("should get compilation album Identifier", async () => {
 			const id = await musicBrainzProvider.getAlbumMetadataByName(
 				"Nova Tunes 01",

diff --git a/server/src/providers/musicbrainz/musicbrainz.provider.ts b/server/src/providers/musicbrainz/musicbrainz.provider.ts
@@ -33,6 +33,7 @@ import MusicBrainzSettings from "./musicbrainz.settings";
 import { ProviderActionFailedError } from "../provider.exception";
 import levenshtein from "damerau-levenshtein";
 import Slug from "src/slug/slug";
+import { AlbumType } from "@prisma/client";
 
 type MBID = string;
 
@@ -125,13 +126,19 @@ export default class MusicBrainzProvider
 	async getAlbumMetadataByName(
 		albumName: string,
 		artistIdentifier?: string,
+		albumType?: AlbumType,
 	): Promise<AlbumMetadata> {
 		try {
 			const searchResult = await this.mbClient
 				.searchRelease({
-					query: `query="${albumName}" AND arid:${
+					query: `query="${albumName.replace(
+						/\s*-\s*Single$/i,
+						"",
+					)}" AND arid:${
 						artistIdentifier ?? this.compilationArtistID
 					}`,
+					inc: ["release-groups"],
+					limit: 1000,
 				})
 				.then((result) =>
 					result.releases.filter((release) =>
@@ -141,6 +148,21 @@ export default class MusicBrainzProvider
 								(artistIdentifier ?? this.compilationArtistID),
 						),
 					),
+				)
+				.then((releases) =>
+					releases.filter((release) => {
+						const releaseIsSingle =
+							release["release-group"]?.["primary-type"] ==
+							"Single";
+						const albumIsSingle = albumType == "Single";
+						if (albumType === undefined) {
+							return true;
+						}
+						if (releaseIsSingle !== albumIsSingle) {
+							return false;
+						}
+						return true;
+					}),
 				);
 			const releaseGroupId = searchResult.at(0)!["release-group"]!.id;