Feature/sshoc 126 merge concepts - [merged]

[dpancic]

In GitLab by @tparkola on Dec 29, 2021, 15:43

_Merges feature/SSHOC-126_mergeconcepts -> develop

The goal was to add endpoint that allows merging concepts within the same vocabulary.

[dpancic]

In GitLab by @tparkola on Dec 29, 2021, 15:43

requested review from @tparkola

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 07:37

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptService.java line 219

I suggest "mergeConcepts" as the method name (consistent with mergeActors previously implemented in AuthorService).

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 07:48

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptService.java line 226

You could also create string message using String.format, e.g. String.format("Unable to find %s with code %s and vocabulary code %s", Concept.class.getName(), code, vocabularyCode) - I like that more as for me it is more readable.

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 07:57

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptService.java line 223

It could be checked at the beginning of the method or at least before dealing with "with" concepts. I suggest that because you cannot do much if the vocabulary is closed (even if the code and vocabularyCode is valid and exists in the repository).

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 08:19

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptService.java line 232

If you would retrieve concepts to a list here, then you could use them below (when dealing with labels and definitions). For instance, you could do something like that: List<Concept> conceptsToMerge = with.stream().map(mergedCode -> conceptRepository.findById(ConceptId.builder().code(mergedCode) .vocabulary(vocabularyCode).build()).orElseThrow(() -> new EntityNotFoundException( String.format("Unable to find %s with code %s and vocabulary code %s", Concept.class.getName(), code, vocabularyCode)))) .collect(Collectors.toList()); - this would give you a list of concepts to be processed further.

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 08:21

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptService.java line 265

Having the list of concepts you could use regular foreach from this section, i.e. for (Concept mergeConcept: conceptsToMerge) { ....

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 08:29

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptService.java line 244

No need to check !mergeConcept.getLabels().isEmpty() condition - if the map is empty then forEach will take no action (no elements to process). Same goes for definitions.

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 08:56

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptService.java line 251

To be removed, or if you wanted to log something - use logger.

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 08:59

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptRelatedConceptService.java line 140

I suggest mergeConceptsRelations or mergeRelations method name here.

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 09:01

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptRelatedConceptService.java line 149

I guess this variable represents relation between concepts, so probably better to name it "newRelation" or similar.

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 09:48

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptService.java line 278

This code itemRepository.findAll().stream().filter(i -> containsConcept(i.getMedia(), mergeConcept)) .collect(Collectors.toList()) should be replaced with a call to newly defined method in itemRepository. Currently you retrieve all items from the repository (lots of data transferred) and you filter them in the code. However, you should retrieve from the repository only the items you are interested in - using proper method in repository (to be created) that will execute proper query on a database level.

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 14:06

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptService.java line 219

changed this line in version 2 of the diff

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 14:06

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptService.java line 226

changed this line in version 2 of the diff

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 14:06

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptService.java line 232

changed this line in version 2 of the diff

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 14:06

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptService.java line 265

changed this line in version 2 of the diff

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 14:06

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptService.java line 244

changed this line in version 2 of the diff

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 14:06

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptService.java line 251

changed this line in version 2 of the diff

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 14:06

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptRelatedConceptService.java line 140

changed this line in version 2 of the diff

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 14:06

Commented on src/main/java/eu/sshopencloud/marketplace/services/vocabularies/ConceptService.java line 278

changed this line in version 2 of the diff

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 14:06

added 2 commits

• 56d3510e - Refactoring code.
• 3d11925f - Refactoring code part 2.

Compare with previous version

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 15:18

Commented on src/test/java/eu/sshopencloud/marketplace/controllers/vocabularies/ConceptControllerITCase.java line 709

Maybe add additional check to verify that the number of concepts after merge is 6 and not 7.

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 15:56

added 1 commit

• 69b09156 - Refactoring tests.

Compare with previous version

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 16:19

Commented on src/main/java/eu/sshopencloud/marketplace/repositories/vocabularies/PropertyRepository.java line 31

I guess @Param("conceptCode") is not really needed (no @Query annotation is present, which uses this param).

[dpancic]

In GitLab by @tparkola on Dec 30, 2021, 16:21

Commented on src/main/java/eu/sshopencloud/marketplace/repositories/items/ItemRepository.java line 79

I think the name could be findAllByConcept - as we retrieve Items based on Concept.

[dpancic]

In GitLab by @tparkola on Jan 3, 2022, 08:21

Commented on src/main/java/eu/sshopencloud/marketplace/repositories/vocabularies/PropertyRepository.java line 31

changed this line in version 4 of the diff

[dpancic]

In GitLab by @tparkola on Jan 3, 2022, 08:21

Commented on src/main/java/eu/sshopencloud/marketplace/repositories/items/ItemRepository.java line 79

changed this line in version 4 of the diff

[dpancic]

In GitLab by @tparkola on Jan 3, 2022, 08:21

added 1 commit

• 420008c4 - Changes in code format.

Compare with previous version

[dpancic]

In GitLab by @tparkola on Jan 3, 2022, 12:13

mentioned in commit 768a7c4d5f7f81bc6aada17ec64245e45dac9af9