From a657a982612fbd7d953d5c214fbe91280b654b1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Ka=C5=88ka?= Date: Thu, 14 Nov 2024 21:40:13 +0100 Subject: [PATCH] [Enhancement kbss-cvut/termit-ui#520] Refactor ChangeRecordDao public interface and add documentation. --- .../termit/persistence/dao/VocabularyDao.java | 13 +----- .../dao/changetracking/ChangeRecordDao.java | 44 ++++++++++++++----- .../persistence/dao/VocabularyDaoTest.java | 9 ++-- .../changetracking/ChangeRecordDaoTest.java | 31 ++++++++----- 4 files changed, 58 insertions(+), 39 deletions(-) diff --git a/src/main/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDao.java b/src/main/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDao.java index cba54669..c8c11eff 100644 --- a/src/main/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDao.java +++ b/src/main/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDao.java @@ -44,7 +44,6 @@ import cz.cvut.kbss.termit.persistence.context.DescriptorFactory; import cz.cvut.kbss.termit.persistence.context.VocabularyContextMapper; import cz.cvut.kbss.termit.persistence.dao.changetracking.ChangeRecordDao; -import cz.cvut.kbss.termit.persistence.dao.changetracking.ChangeTrackingContextResolver; import cz.cvut.kbss.termit.persistence.snapshot.AssetSnapshotLoader; import cz.cvut.kbss.termit.persistence.validation.VocabularyContentValidator; import cz.cvut.kbss.termit.service.snapshot.SnapshotProvider; @@ -90,7 +89,6 @@ public class VocabularyDao extends BaseAssetDao "} GROUP BY ?date HAVING (?cnt > 0) ORDER BY ?date"; private static final String REMOVE_GLOSSARY_TERMS_QUERY_FILE = "remove/removeGlossaryTerms.ru"; - private final ChangeTrackingContextResolver changeTrackingContextResolver; private final ChangeRecordDao changeRecordDao; private volatile long lastModified; @@ -102,12 +100,11 @@ public class VocabularyDao extends BaseAssetDao @Autowired public VocabularyDao(EntityManager em, Configuration config, DescriptorFactory descriptorFactory, VocabularyContextMapper contextMapper, ApplicationContext context, - ChangeTrackingContextResolver changeTrackingContextResolver, ChangeRecordDao changeRecordDao) { + ChangeRecordDao changeRecordDao) { super(Vocabulary.class, em, config.getPersistence(), descriptorFactory); this.contextMapper = contextMapper; refreshLastModified(); this.context = context; - this.changeTrackingContextResolver = changeTrackingContextResolver; this.changeRecordDao = changeRecordDao; } @@ -411,13 +408,7 @@ public List getChangesOfContent(Vocabulary vocabulary) { */ public List getDetailedHistoryOfContent(Vocabulary vocabulary, ChangeRecordFilterDto filter, Pageable pageReq) { Objects.requireNonNull(vocabulary); - return changeRecordDao.findAllFiltered( - changeTrackingContextResolver.resolveChangeTrackingContext(vocabulary), - filter, - Optional.empty(), - Optional.of(URI.create(SKOS.CONCEPT)), // term - pageReq - ); + return changeRecordDao.findAllRelatedToType(vocabulary, filter, URI.create(SKOS.CONCEPT), pageReq); } private Query createContentChangesQuery(Vocabulary vocabulary) { diff --git a/src/main/java/cz/cvut/kbss/termit/persistence/dao/changetracking/ChangeRecordDao.java b/src/main/java/cz/cvut/kbss/termit/persistence/dao/changetracking/ChangeRecordDao.java index 4098d32b..450c5716 100644 --- a/src/main/java/cz/cvut/kbss/termit/persistence/dao/changetracking/ChangeRecordDao.java +++ b/src/main/java/cz/cvut/kbss/termit/persistence/dao/changetracking/ChangeRecordDao.java @@ -83,29 +83,49 @@ public List findAll(Asset asset) { return findAll(asset, new ChangeRecordFilterDto()); } + private Optional resolveChangeTrackingContext(Asset asset) { + try { + return Optional.of(contextResolver.resolveChangeTrackingContext(asset)); + } catch (NoResultException e) { + return Optional.empty(); + } + } + /** + * Finds all change records related to the specified asset matching the filter. * - * @param asset - * @param filterDto - * @return + * @param asset the asset + * @param filterDto filter parameters */ public List findAll(Asset asset, ChangeRecordFilterDto filterDto) { - URI changeTrackingContext = null; - try { - changeTrackingContext = contextResolver.resolveChangeTrackingContext(asset); - } catch (NoResultException e) { - return List.of(); - } - return findAllFiltered(changeTrackingContext, filterDto, Optional.of(asset), Optional.empty(), Pageable.unpaged()); + return resolveChangeTrackingContext(asset).map(context -> + findAllFiltered(context, filterDto, Optional.of(asset), Optional.empty(), Pageable.unpaged())) + .orElseGet(List::of); } /** + * Finds all records from change context resolved from {@code changeContextAsset} + * that are matching the filter and are related to an entity of the type {@code relatedEntityType}. + */ + public List findAllRelatedToType(Asset changeContextAsset, ChangeRecordFilterDto filterDto, URI relatedEntityType, Pageable pageable) { + return resolveChangeTrackingContext(changeContextAsset).map(context -> + findAllFiltered(context, + filterDto, + Optional.empty(), + Optional.ofNullable(relatedEntityType), + pageable + )).orElseGet(List::of); + } + + /** + * Finds all change records matching the filter. + * * @param changeContext the context of change records * @param filter filter parameters * @param asset if present, only changes of the asset will be returned - * @param assetType if present, only changes related to this asset type will be returned. + * @param assetType if present, only changes related to an asset of this type will be returned. */ - public List findAllFiltered(URI changeContext, ChangeRecordFilterDto filter, Optional> asset, Optional assetType, Pageable pageable) { + private List findAllFiltered(URI changeContext, ChangeRecordFilterDto filter, Optional> asset, Optional assetType, Pageable pageable) { TypedQuery query = em.createNativeQuery(""" SELECT DISTINCT ?record WHERE { """ + /* Select anything from change context */ """ diff --git a/src/test/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDaoTest.java b/src/test/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDaoTest.java index 18cddcaa..1656f3b9 100644 --- a/src/test/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDaoTest.java +++ b/src/test/java/cz/cvut/kbss/termit/persistence/dao/VocabularyDaoTest.java @@ -115,9 +115,6 @@ class VocabularyDaoTest extends BaseDaoTestRunner { @Autowired private VocabularyDao sut; - @Autowired - private TermDao termDao; - @SpyBean private ChangeRecordDao changeRecordDao; @@ -952,17 +949,17 @@ void getAnyExternalRelationsReturnsTermsWithBothRelations(URI termRelation) { void getDetailedHistoryOfContentCallsChangeRecordDaoWithFilter() { final Vocabulary vocabulary = Generator.generateVocabularyWithId(); final List records = List.of(); - final Optional skosConcept = Optional.of(URI.create(SKOS.CONCEPT)); + final URI skosConcept = URI.create(SKOS.CONCEPT); final Pageable unpaged = Pageable.unpaged(); final ChangeRecordFilterDto filterDto = new ChangeRecordFilterDto(); filterDto.setAuthorName("Name of the author"); doReturn(vocabulary.getUri()).when(changeTrackingContextResolver).resolveChangeTrackingContext(vocabulary); - doReturn(records).when(changeRecordDao).findAllFiltered(vocabulary.getUri(), filterDto, Optional.empty(), skosConcept, unpaged); + doReturn(records).when(changeRecordDao).findAllRelatedToType(vocabulary, filterDto, skosConcept, unpaged); sut.getDetailedHistoryOfContent(vocabulary, filterDto, unpaged); verify(changeTrackingContextResolver).resolveChangeTrackingContext(vocabulary); - verify(changeRecordDao).findAllFiltered(vocabulary.getUri(), filterDto, Optional.empty(), skosConcept, unpaged); + verify(changeRecordDao).findAllRelatedToType(vocabulary, filterDto, skosConcept, unpaged); } } diff --git a/src/test/java/cz/cvut/kbss/termit/persistence/dao/changetracking/ChangeRecordDaoTest.java b/src/test/java/cz/cvut/kbss/termit/persistence/dao/changetracking/ChangeRecordDaoTest.java index 9fdf3931..2ca822ed 100644 --- a/src/test/java/cz/cvut/kbss/termit/persistence/dao/changetracking/ChangeRecordDaoTest.java +++ b/src/test/java/cz/cvut/kbss/termit/persistence/dao/changetracking/ChangeRecordDaoTest.java @@ -71,6 +71,7 @@ @DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD) class ChangeRecordDaoTest extends BaseDaoTestRunner { + private static final URI SKOS_CONCEPT = URI.create(SKOS.CONCEPT); @Autowired private ChangeTrackingContextResolver contextResolver; @@ -286,6 +287,11 @@ void getAuthorsRetrievesUsersAssociatedWithPersistChangeRecordsOfSpecifiedAsset( assertEquals(Collections.singleton(author), result); } + @Test + void voidFindAllReturnsChangeRecordsWithoutVocabularyChanges() { + + } + @Test void findAllFilteredReturnsRecordsOfExistingTermFilteredByTermName() { enableRdfsInference(em); @@ -306,6 +312,7 @@ void findAllFilteredReturnsRecordsOfExistingTermFilteredByTermName() { final List secondChanges = Generator.generateChangeRecords(secondTerm, author); final List thirdChanges = Generator.generateChangeRecords(thirdTerm, author); + final Descriptor changeContextDescriptor = persistDescriptor(contextResolver.resolveChangeTrackingContext(vocabulary)); final Descriptor vocabularyDescriptor = persistDescriptor(vocabulary.getUri()); transactional(() -> { @@ -319,7 +326,7 @@ void findAllFilteredReturnsRecordsOfExistingTermFilteredByTermName() { Stream.of(firstChanges, secondChanges, thirdChanges) .flatMap(Collection::stream) - .forEach(r -> em.persist(r, vocabularyDescriptor)); + .forEach(r -> em.persist(r, changeContextDescriptor)); }); final ChangeRecordFilterDto filter = new ChangeRecordFilterDto(); @@ -328,7 +335,7 @@ void findAllFilteredReturnsRecordsOfExistingTermFilteredByTermName() { final int recordsCount = firstChanges.size() + secondChanges.size(); final Pageable pageable = Pageable.ofSize(recordsCount * 2); - final List contentChanges = sut.findAllFiltered(vocabulary.getUri(), filter, Optional.empty(), Optional.of(URI.create(SKOS.CONCEPT)), pageable); + final List contentChanges = sut.findAllRelatedToType(vocabulary, filter, SKOS_CONCEPT, pageable); assertEquals(recordsCount, contentChanges.size()); final long persistCount = contentChanges.stream().filter(ch -> ch instanceof PersistChangeRecord).count(); @@ -364,6 +371,7 @@ void findAllFilteredReturnsRecordsOfDeletedTermFilteredByTermName() { deleteChangeRecord.setAuthor(author); deleteChangeRecord.setLabel(termToRemove.getLabel()); + final Descriptor changeContextDescriptor = persistDescriptor(contextResolver.resolveChangeTrackingContext(vocabulary)); final Descriptor vocabularyDescriptor = persistDescriptor(vocabulary.getUri()); transactional(() -> { @@ -374,7 +382,7 @@ void findAllFilteredReturnsRecordsOfDeletedTermFilteredByTermName() { Stream.of(firstChanges, termToRemoveChanges, List.of(deleteChangeRecord)) .flatMap(Collection::stream) - .forEach(r -> em.persist(r, vocabularyDescriptor)); + .forEach(r -> em.persist(r, changeContextDescriptor)); }); final ChangeRecordFilterDto filter = new ChangeRecordFilterDto(); @@ -383,7 +391,7 @@ void findAllFilteredReturnsRecordsOfDeletedTermFilteredByTermName() { final int recordsCount = termToRemoveChanges.size() + 1; // +1 for the delete record final Pageable pageable = Pageable.unpaged(); - final List contentChanges = sut.findAllFiltered(vocabulary.getUri(), filter, Optional.empty(), Optional.of(URI.create(SKOS.CONCEPT)), pageable); + final List contentChanges = sut.findAllRelatedToType(vocabulary, filter, SKOS_CONCEPT, pageable); assertEquals(recordsCount, contentChanges.size()); final long persistCount = contentChanges.stream().filter(ch -> ch instanceof PersistChangeRecord).count(); @@ -413,6 +421,7 @@ void findAllFilteredReturnsRecordsOfExistingTermFilteredByChangedAttributeName() final URI anotherChangedAttribute = URI.create(RDFS.LABEL); final String changedAttributeName = "definition"; + final Descriptor changeContextDescriptor = persistDescriptor(contextResolver.resolveChangeTrackingContext(vocabulary)); final Descriptor vocabularyDescriptor = persistDescriptor(vocabulary.getUri()); Stream.of(firstChanges, secondChanges).flatMap(Collection::stream) @@ -435,7 +444,7 @@ void findAllFilteredReturnsRecordsOfExistingTermFilteredByChangedAttributeName() Stream.of(firstChanges, secondChanges) .flatMap(Collection::stream) - .forEach(r -> em.persist(r, vocabularyDescriptor)); + .forEach(r -> em.persist(r, changeContextDescriptor)); }); final ChangeRecordFilterDto filter = new ChangeRecordFilterDto(); @@ -443,7 +452,7 @@ void findAllFilteredReturnsRecordsOfExistingTermFilteredByChangedAttributeName() final Pageable pageable = Pageable.unpaged(); - final List contentChanges = sut.findAllFiltered(vocabulary.getUri(), filter, Optional.empty(), Optional.of(URI.create(SKOS.CONCEPT)), pageable); + final List contentChanges = sut.findAllRelatedToType(vocabulary, filter, SKOS_CONCEPT, pageable); assertEquals(recordCount.get(), contentChanges.size()); final long persistCount = contentChanges.stream().filter(ch -> ch instanceof PersistChangeRecord).count(); @@ -477,6 +486,7 @@ void findAllFilteredReturnsRecordsOfExistingTermFilteredByAuthorName() { firstChanges.add(Generator.generateUpdateChange(firstTerm)); secondChanges.add(Generator.generateUpdateChange(secondTerm)); + final Descriptor changeContextDescriptor = persistDescriptor(contextResolver.resolveChangeTrackingContext(vocabulary)); final Descriptor vocabularyDescriptor = persistDescriptor(vocabulary.getUri()); transactional(() -> { @@ -487,7 +497,7 @@ void findAllFilteredReturnsRecordsOfExistingTermFilteredByAuthorName() { Stream.of(firstChanges, secondChanges) .flatMap(Collection::stream) - .forEach(r -> em.persist(r, vocabularyDescriptor)); + .forEach(r -> em.persist(r, changeContextDescriptor)); }); final ChangeRecordFilterDto filter = new ChangeRecordFilterDto(); @@ -496,7 +506,7 @@ void findAllFilteredReturnsRecordsOfExistingTermFilteredByAuthorName() { final Pageable pageable = Pageable.unpaged(); - final List contentChanges = sut.findAllFiltered(vocabulary.getUri(), filter, Optional.empty(), Optional.of(URI.create(SKOS.CONCEPT)), pageable); + final List contentChanges = sut.findAllRelatedToType(vocabulary, filter, SKOS_CONCEPT, pageable); assertEquals(recordCount, contentChanges.size()); final long persistCount = contentChanges.stream().filter(ch -> ch instanceof PersistChangeRecord).count(); @@ -531,6 +541,7 @@ void findAllFilteredReturnsRecordsOfExistingTermFilteredByChangeType(Class { @@ -541,7 +552,7 @@ void findAllFilteredReturnsRecordsOfExistingTermFilteredByChangeType(Class em.persist(r, vocabularyDescriptor)); + .forEach(r -> em.persist(r, changeContextDescriptor)); }); final ChangeRecordFilterDto filter = new ChangeRecordFilterDto(); @@ -550,7 +561,7 @@ void findAllFilteredReturnsRecordsOfExistingTermFilteredByChangeType(Class contentChanges = sut.findAllFiltered(vocabulary.getUri(), filter, Optional.empty(), Optional.of(URI.create(SKOS.CONCEPT)), pageable); + final List contentChanges = sut.findAllRelatedToType(vocabulary, filter, SKOS_CONCEPT, pageable); assertEquals(recordCount, contentChanges.size()); assertTrue(contentChanges.stream().allMatch(typeClass::isInstance));