fix(metadata-refresh): correct provider priority order and enhance REPLACE_ALL handling (#2503)

- Update provider priority logic to ensure P1 > P2 > P3 > P4 for all metadata fields
- Refactor buildFetchMetadata to accept existing metadata and preserve fields when using REPLACE_ALL mode
- Adjust service and test method signatures for new parameter
- Add unit test to verify provider priority order

Signed-off-by: Balázs Szücs <bszucs1209@gmail.com>
This commit is contained in:
Balázs Szücs
2026-01-28 00:35:06 +01:00
committed by GitHub
parent 6c4934b8ba
commit 58bfe2293a
6 changed files with 169 additions and 21 deletions

View File

@@ -78,7 +78,7 @@ public class BookdropMetadataService {
}
Map<MetadataProvider, BookMetadata> metadataMap = metadataRefreshService.fetchMetadataForBook(providers, book);
BookMetadata fetchedMetadata = metadataRefreshService.buildFetchMetadata(book.getId(), refreshOptions, metadataMap);
BookMetadata fetchedMetadata = metadataRefreshService.buildFetchMetadata(initial, book.getId(), refreshOptions, metadataMap);
String fetchedJson = objectMapper.writeValueAsString(fetchedMetadata);
entity.setFetchedMetadata(fetchedJson);

View File

@@ -148,7 +148,7 @@ public class MetadataRefreshService {
BookMetadata fetched = null;
boolean bookReviewMode = false;
if (refreshOptions != null) {
fetched = buildFetchMetadata(book.getId(), refreshOptions, metadataMap);
fetched = buildFetchMetadata(bookMapper.toBook(book).getMetadata(), book.getId(), refreshOptions, metadataMap);
bookReviewMode = Boolean.TRUE.equals(refreshOptions.getReviewBeforeApply());
}
@@ -367,10 +367,10 @@ public class MetadataRefreshService {
protected void addProviderToSet(MetadataRefreshOptions.FieldProvider fieldProvider, Set<MetadataProvider> providerSet, AppSettings appSettings) {
if (fieldProvider != null) {
if (fieldProvider.getP4() != null && isProviderEnabled(fieldProvider.getP4(), appSettings)) providerSet.add(fieldProvider.getP4());
if (fieldProvider.getP3() != null && isProviderEnabled(fieldProvider.getP3(), appSettings)) providerSet.add(fieldProvider.getP3());
if (fieldProvider.getP2() != null && isProviderEnabled(fieldProvider.getP2(), appSettings)) providerSet.add(fieldProvider.getP2());
if (fieldProvider.getP1() != null && isProviderEnabled(fieldProvider.getP1(), appSettings)) providerSet.add(fieldProvider.getP1());
if (fieldProvider.getP2() != null && isProviderEnabled(fieldProvider.getP2(), appSettings)) providerSet.add(fieldProvider.getP2());
if (fieldProvider.getP3() != null && isProviderEnabled(fieldProvider.getP3(), appSettings)) providerSet.add(fieldProvider.getP3());
if (fieldProvider.getP4() != null && isProviderEnabled(fieldProvider.getP4(), appSettings)) providerSet.add(fieldProvider.getP4());
}
}
@@ -416,7 +416,7 @@ public class MetadataRefreshService {
.build();
}
public BookMetadata buildFetchMetadata(Long bookId, MetadataRefreshOptions refreshOptions, Map<MetadataProvider, BookMetadata> metadataMap) {
public BookMetadata buildFetchMetadata(BookMetadata existingMetadata, Long bookId, MetadataRefreshOptions refreshOptions, Map<MetadataProvider, BookMetadata> metadataMap) {
BookMetadata metadata = BookMetadata.builder().bookId(bookId).build();
MetadataRefreshOptions.FieldOptions fieldOptions = refreshOptions.getFieldOptions();
@@ -428,141 +428,238 @@ public class MetadataRefreshService {
if (enabledFields == null) {
enabledFields = new MetadataRefreshOptions.EnabledFields();
}
MetadataReplaceMode replaceMode = refreshOptions.getReplaceMode();
boolean isReplaceAll = replaceMode == MetadataReplaceMode.REPLACE_ALL;
if (enabledFields.isTitle()) {
metadata.setTitle(resolveFieldAsString(metadataMap, fieldOptions.getTitle(), BookMetadata::getTitle));
} else if (isReplaceAll && existingMetadata != null) {
metadata.setTitle(existingMetadata.getTitle());
}
if (enabledFields.isSubtitle()) {
metadata.setSubtitle(resolveFieldAsString(metadataMap, fieldOptions.getSubtitle(), BookMetadata::getSubtitle));
} else if (isReplaceAll && existingMetadata != null) {
metadata.setSubtitle(existingMetadata.getSubtitle());
}
if (enabledFields.isDescription()) {
metadata.setDescription(resolveFieldAsString(metadataMap, fieldOptions.getDescription(), BookMetadata::getDescription));
} else if (isReplaceAll && existingMetadata != null) {
metadata.setDescription(existingMetadata.getDescription());
}
if (enabledFields.isAuthors()) {
metadata.setAuthors(resolveFieldAsList(metadataMap, fieldOptions.getAuthors(), BookMetadata::getAuthors));
} else if (isReplaceAll && existingMetadata != null) {
metadata.setAuthors(existingMetadata.getAuthors());
}
if (enabledFields.isPublisher()) {
metadata.setPublisher(resolveFieldAsString(metadataMap, fieldOptions.getPublisher(), BookMetadata::getPublisher));
} else if (isReplaceAll && existingMetadata != null) {
metadata.setPublisher(existingMetadata.getPublisher());
}
if (enabledFields.isPublishedDate()) {
metadata.setPublishedDate(resolveField(metadataMap, fieldOptions.getPublishedDate(), BookMetadata::getPublishedDate));
} else if (isReplaceAll && existingMetadata != null) {
metadata.setPublishedDate(existingMetadata.getPublishedDate());
}
if (enabledFields.isSeriesName()) {
metadata.setSeriesName(resolveFieldAsString(metadataMap, fieldOptions.getSeriesName(), BookMetadata::getSeriesName));
} else if (isReplaceAll && existingMetadata != null) {
metadata.setSeriesName(existingMetadata.getSeriesName());
}
if (enabledFields.isSeriesNumber()) {
metadata.setSeriesNumber(resolveField(metadataMap, fieldOptions.getSeriesNumber(), BookMetadata::getSeriesNumber));
} else if (isReplaceAll && existingMetadata != null) {
metadata.setSeriesNumber(existingMetadata.getSeriesNumber());
}
if (enabledFields.isSeriesTotal()) {
metadata.setSeriesTotal(resolveFieldAsInteger(metadataMap, fieldOptions.getSeriesTotal(), BookMetadata::getSeriesTotal));
} else if (isReplaceAll && existingMetadata != null) {
metadata.setSeriesTotal(existingMetadata.getSeriesTotal());
}
if (enabledFields.isIsbn13()) {
metadata.setIsbn13(resolveFieldAsString(metadataMap, fieldOptions.getIsbn13(), BookMetadata::getIsbn13));
} else if (isReplaceAll && existingMetadata != null) {
metadata.setIsbn13(existingMetadata.getIsbn13());
}
if (enabledFields.isIsbn10()) {
metadata.setIsbn10(resolveFieldAsString(metadataMap, fieldOptions.getIsbn10(), BookMetadata::getIsbn10));
} else if (isReplaceAll && existingMetadata != null) {
metadata.setIsbn10(existingMetadata.getIsbn10());
}
if (enabledFields.isLanguage()) {
metadata.setLanguage(resolveFieldAsString(metadataMap, fieldOptions.getLanguage(), BookMetadata::getLanguage));
} else if (isReplaceAll && existingMetadata != null) {
metadata.setLanguage(existingMetadata.getLanguage());
}
if (enabledFields.isPageCount()) {
metadata.setPageCount(resolveFieldAsInteger(metadataMap, fieldOptions.getPageCount(), BookMetadata::getPageCount));
} else if (isReplaceAll && existingMetadata != null) {
metadata.setPageCount(existingMetadata.getPageCount());
}
if (enabledFields.isCover()) {
metadata.setThumbnailUrl(resolveFieldAsString(metadataMap, fieldOptions.getCover(), BookMetadata::getThumbnailUrl));
} else if (isReplaceAll && existingMetadata != null) {
metadata.setThumbnailUrl(existingMetadata.getThumbnailUrl());
}
if (enabledFields.isAmazonRating()) {
if (metadataMap.containsKey(Amazon)) {
metadata.setAmazonRating(metadataMap.get(Amazon).getAmazonRating());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setAmazonRating(existingMetadata.getAmazonRating());
}
if (enabledFields.isAmazonReviewCount()) {
if (metadataMap.containsKey(Amazon)) {
metadata.setAmazonReviewCount(metadataMap.get(Amazon).getAmazonReviewCount());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setAmazonReviewCount(existingMetadata.getAmazonReviewCount());
}
if (enabledFields.isGoodreadsRating()) {
if (metadataMap.containsKey(GoodReads)) {
metadata.setGoodreadsRating(metadataMap.get(GoodReads).getGoodreadsRating());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setGoodreadsRating(existingMetadata.getGoodreadsRating());
}
if (enabledFields.isGoodreadsReviewCount()) {
if (metadataMap.containsKey(GoodReads)) {
metadata.setGoodreadsReviewCount(metadataMap.get(GoodReads).getGoodreadsReviewCount());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setGoodreadsReviewCount(existingMetadata.getGoodreadsReviewCount());
}
if (enabledFields.isHardcoverRating()) {
if (metadataMap.containsKey(Hardcover)) {
metadata.setHardcoverRating(metadataMap.get(Hardcover).getHardcoverRating());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setHardcoverRating(existingMetadata.getHardcoverRating());
}
if (enabledFields.isHardcoverReviewCount()) {
if (metadataMap.containsKey(Hardcover)) {
metadata.setHardcoverReviewCount(metadataMap.get(Hardcover).getHardcoverReviewCount());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setHardcoverReviewCount(existingMetadata.getHardcoverReviewCount());
}
if (enabledFields.isAsin()) {
if (metadataMap.containsKey(Amazon)) {
metadata.setAsin(metadataMap.get(Amazon).getAsin());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setAsin(existingMetadata.getAsin());
}
if (enabledFields.isGoodreadsId()) {
if (metadataMap.containsKey(GoodReads)) {
metadata.setGoodreadsId(metadataMap.get(GoodReads).getGoodreadsId());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setGoodreadsId(existingMetadata.getGoodreadsId());
}
if (enabledFields.isHardcoverId()) {
if (metadataMap.containsKey(Hardcover)) {
metadata.setHardcoverId(metadataMap.get(Hardcover).getHardcoverId());
metadata.setHardcoverBookId(metadataMap.get(Hardcover).getHardcoverBookId());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setHardcoverId(existingMetadata.getHardcoverId());
metadata.setHardcoverBookId(existingMetadata.getHardcoverBookId());
}
if (enabledFields.isGoogleId()) {
if (metadataMap.containsKey(Google)) {
metadata.setGoogleId(metadataMap.get(Google).getGoogleId());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setGoogleId(existingMetadata.getGoogleId());
}
if (enabledFields.isComicvineId()) {
if (metadataMap.containsKey(Comicvine)) {
metadata.setComicvineId(metadataMap.get(Comicvine).getComicvineId());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setComicvineId(existingMetadata.getComicvineId());
}
if (enabledFields.isLubimyczytacId()) {
if (metadataMap.containsKey(Lubimyczytac)) {
metadata.setLubimyczytacId(metadataMap.get(Lubimyczytac).getLubimyczytacId());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setLubimyczytacId(existingMetadata.getLubimyczytacId());
}
if (enabledFields.isLubimyczytacRating()) {
if (metadataMap.containsKey(Lubimyczytac)) {
metadata.setLubimyczytacRating(metadataMap.get(Lubimyczytac).getLubimyczytacRating());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setLubimyczytacRating(existingMetadata.getLubimyczytacRating());
}
if (enabledFields.isRanobedbId()) {
if (metadataMap.containsKey(Ranobedb)) {
metadata.setRanobedbId(metadataMap.get(Ranobedb).getRanobedbId());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setRanobedbId(existingMetadata.getRanobedbId());
}
if (enabledFields.isRanobedbRating()) {
if (metadataMap.containsKey(Ranobedb)) {
metadata.setRanobedbRating(metadataMap.get(Ranobedb).getRanobedbRating());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setRanobedbRating(existingMetadata.getRanobedbRating());
}
if (enabledFields.isMoods()) {
if (metadataMap.containsKey(Hardcover)) {
metadata.setMoods(metadataMap.get(Hardcover).getMoods());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setMoods(existingMetadata.getMoods());
}
if (enabledFields.isTags()) {
if (metadataMap.containsKey(Hardcover)) {
metadata.setTags(metadataMap.get(Hardcover).getTags());
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setTags(existingMetadata.getTags());
}
if (enabledFields.isCategories()) {
if (refreshOptions.isMergeCategories()) {
metadata.setCategories(getAllCategories(metadataMap, fieldOptions.getCategories(), BookMetadata::getCategories));
} else {
metadata.setCategories(resolveFieldAsList(metadataMap, fieldOptions.getCategories(), BookMetadata::getCategories));
}
} else if (isReplaceAll && existingMetadata != null) {
metadata.setCategories(existingMetadata.getCategories());
}
List<BookReview> allReviews = metadataMap.values().stream()
@@ -597,10 +694,10 @@ public class MetadataRefreshService {
return null;
}
MetadataProvider[] providers = {
fieldProvider.getP4(),
fieldProvider.getP3(),
fieldProvider.getP1(),
fieldProvider.getP2(),
fieldProvider.getP1()
fieldProvider.getP3(),
fieldProvider.getP4()
};
for (MetadataProvider provider : providers) {
if (provider != null && metadataMap.containsKey(provider)) {
@@ -620,10 +717,10 @@ public class MetadataRefreshService {
}
MetadataProvider[] providers = {
fieldProvider.getP4(),
fieldProvider.getP3(),
fieldProvider.getP1(),
fieldProvider.getP2(),
fieldProvider.getP1()
fieldProvider.getP3(),
fieldProvider.getP4()
};
for (MetadataProvider provider : providers) {

View File

@@ -107,7 +107,7 @@ class BookdropMetadataServiceTest {
when(metadataRefreshService.prepareProviders(any())).thenReturn(List.of());
when(objectMapper.readValue(sampleFile.getOriginalMetadata(), BookMetadata.class)).thenReturn(fetched);
when(metadataRefreshService.fetchMetadataForBook(any(), any(Book.class))).thenReturn(Map.of());
when(metadataRefreshService.buildFetchMetadata(any(), any(), any())).thenReturn(fetched);
when(metadataRefreshService.buildFetchMetadata(any(), any(), any(), any())).thenReturn(fetched);
when(objectMapper.writeValueAsString(fetched)).thenReturn("{\"title\":\"New Title\"}");
BookdropFileEntity result = bookdropMetadataService.attachFetchedMetadata(1L);
@@ -154,7 +154,7 @@ class BookdropMetadataServiceTest {
when(metadataRefreshService.prepareProviders(any())).thenReturn(List.of(MetadataProvider.GoodReads));
when(objectMapper.readValue(anyString(), eq(BookMetadata.class))).thenReturn(fetched);
when(metadataRefreshService.fetchMetadataForBook(any(), any(Book.class))).thenReturn(Map.of());
when(metadataRefreshService.buildFetchMetadata(any(), any(), any())).thenReturn(fetched);
when(metadataRefreshService.buildFetchMetadata(any(), any(), any(), any())).thenReturn(fetched);
when(objectMapper.writeValueAsString(fetched)).thenReturn("{\"title\":\"Fetched Book\"}");
when(bookdropFileRepository.save(any())).thenAnswer(invocation -> invocation.getArgument(0));

View File

@@ -0,0 +1,51 @@
package com.adityachandel.booklore.service.metadata;
import com.adityachandel.booklore.model.dto.BookMetadata;
import com.adityachandel.booklore.model.dto.request.MetadataRefreshOptions;
import com.adityachandel.booklore.model.enums.MetadataProvider;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.InjectMocks;
import org.mockito.junit.jupiter.MockitoExtension;
import java.util.Map;
import static org.assertj.core.api.Assertions.assertThat;
@ExtendWith(MockitoExtension.class)
class MetadataPriorityTest {
@InjectMocks
private MetadataRefreshService metadataRefreshService;
@Test
@DisplayName("P1 should have higher priority than P4")
void buildFetchMetadata_shouldRespectPriority_P1_over_P4() {
Long bookId = 1L;
// Setup Options with P1=Google, P4=Amazon
MetadataRefreshOptions refreshOptions = new MetadataRefreshOptions();
MetadataRefreshOptions.FieldProvider titleProvider = new MetadataRefreshOptions.FieldProvider();
titleProvider.setP1(MetadataProvider.Google);
titleProvider.setP4(MetadataProvider.Amazon);
MetadataRefreshOptions.FieldOptions fieldOptions = new MetadataRefreshOptions.FieldOptions();
fieldOptions.setTitle(titleProvider);
refreshOptions.setFieldOptions(fieldOptions);
BookMetadata googleMetadata = BookMetadata.builder().title("Google Title (P1)").provider(MetadataProvider.Google).build();
BookMetadata amazonMetadata = BookMetadata.builder().title("Amazon Title (P4)").provider(MetadataProvider.Amazon).build();
Map<MetadataProvider, BookMetadata> metadataMap = Map.of(
MetadataProvider.Google, googleMetadata,
MetadataProvider.Amazon, amazonMetadata
);
BookMetadata result = metadataRefreshService.buildFetchMetadata(null, bookId, refreshOptions, metadataMap);
assertThat(result.getTitle())
.as("Should prefer P1 (Google) over P4 (Amazon)")
.isEqualTo("Google Title (P1)");
}
}

View File

@@ -37,7 +37,7 @@ class MetadataRefreshServiceTest {
Map<MetadataProvider, BookMetadata> metadataMap = Collections.emptyMap();
BookMetadata result = assertDoesNotThrow(() ->
metadataRefreshService.buildFetchMetadata(bookId, refreshOptions, metadataMap)
metadataRefreshService.buildFetchMetadata(null, bookId, refreshOptions, metadataMap)
);
assertThat(result).isNotNull();

View File

@@ -46,7 +46,7 @@ class MetadataRefreshServiceTitleUpdateTest {
Map<MetadataProvider, BookMetadata> metadataMap = Map.of(MetadataProvider.Google, googleMetadata);
BookMetadata result = metadataRefreshService.buildFetchMetadata(bookId, refreshOptions, metadataMap);
BookMetadata result = metadataRefreshService.buildFetchMetadata(null, bookId, refreshOptions, metadataMap);
assertThat(result.getTitle())
.as("Title should be fetched when EnabledFields is null (defaults should enable title)")
@@ -74,7 +74,7 @@ class MetadataRefreshServiceTitleUpdateTest {
Map<MetadataProvider, BookMetadata> metadataMap = Map.of(MetadataProvider.Amazon, amazonMetadata);
BookMetadata result = metadataRefreshService.buildFetchMetadata(bookId, refreshOptions, metadataMap);
BookMetadata result = metadataRefreshService.buildFetchMetadata(null, bookId, refreshOptions, metadataMap);
assertThat(result.getTitle())
.as("Title should be fetched when EnabledFields is default-constructed (all fields should be enabled by default)")
@@ -162,7 +162,7 @@ class MetadataRefreshServiceTitleUpdateTest {
Map<MetadataProvider, BookMetadata> metadataMap = Map.of(MetadataProvider.Google, googleMetadata);
BookMetadata result = metadataRefreshService.buildFetchMetadata(bookId, refreshOptions, metadataMap);
BookMetadata result = metadataRefreshService.buildFetchMetadata(null, bookId, refreshOptions, metadataMap);
assertThat(result.getTitle()).isEqualTo("Explicitly Enabled Title");
}
@@ -191,7 +191,7 @@ class MetadataRefreshServiceTitleUpdateTest {
Map<MetadataProvider, BookMetadata> metadataMap = Map.of(MetadataProvider.Google, googleMetadata);
BookMetadata result = metadataRefreshService.buildFetchMetadata(bookId, refreshOptions, metadataMap);
BookMetadata result = metadataRefreshService.buildFetchMetadata(null, bookId, refreshOptions, metadataMap);
assertThat(result.getTitle())
.as("Title should be null when explicitly disabled")
@@ -227,7 +227,7 @@ class MetadataRefreshServiceTitleUpdateTest {
Map<MetadataProvider, BookMetadata> metadataMap = Map.of(MetadataProvider.Google, googleMetadata);
BookMetadata result = metadataRefreshService.buildFetchMetadata(bookId, refreshOptions, metadataMap);
BookMetadata result = metadataRefreshService.buildFetchMetadata(null, bookId, refreshOptions, metadataMap);
assertThat(result.getTitle())
.as("Title should be fetched with default enabled fields")
@@ -264,7 +264,7 @@ class MetadataRefreshServiceTitleUpdateTest {
Map<MetadataProvider, BookMetadata> metadataMap = Map.of(MetadataProvider.Comicvine, comicvineMetadata);
BookMetadata result = metadataRefreshService.buildFetchMetadata(bookId, refreshOptions, metadataMap);
BookMetadata result = metadataRefreshService.buildFetchMetadata(null, bookId, refreshOptions, metadataMap);
assertThat(result.getTitle())
.as("Fetched metadata should include the title from Comicvine")