Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make behaviour of repeated import configurable #5613

Merged
merged 24 commits into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
81af102
Implement interface and tests
matthias-ronge Mar 30, 2023
76725ee
Get reimport settings from ruleset
matthias-ronge Mar 30, 2023
3dd9ac4
Implement metadata update
matthias-ronge Mar 30, 2023
a04c25d
Implement use
matthias-ronge Mar 30, 2023
d4b2085
Clean-up
matthias-ronge Mar 30, 2023
88c50d1
Organize imports
matthias-ronge Mar 30, 2023
96eb697
Add javadoc
matthias-ronge Mar 30, 2023
7214ff3
Fix checkstyle (kitodo-data-editor)
matthias-ronge Mar 30, 2023
e31c7c2
Fix checkstyle
matthias-ronge Mar 30, 2023
418dc62
Explain use on ruleset subhh.xml
matthias-ronge Mar 31, 2023
0d7371b
Fix Codacy complains
matthias-ronge Mar 31, 2023
92e3938
Use wrong use of Objects.nonNull() to check for null values
matthias-ronge Apr 11, 2023
e66b31e
Use an other error message
matthias-ronge Apr 11, 2023
b83c520
Replace 'var' typing with explicit type naming
matthias-ronge Apr 11, 2023
eb12937
Add an explaination that 'replace' does not delete not-imported metadata
matthias-ronge Apr 19, 2023
6af03c4
Split long method
matthias-ronge May 24, 2023
1979dfe
Implements the reimport behaviour respecting the maxOccurs setting
matthias-ronge May 30, 2023
6183027
Fix import
matthias-ronge May 31, 2023
9b5fb2e
Fix test dummy class
matthias-ronge May 31, 2023
b379c3a
Better explain the use of English
matthias-ronge Jul 13, 2023
370c019
Update Kitodo-DataEditor/src/main/java/org/kitodo/dataeditor/ruleset/…
matthias-ronge Jul 13, 2023
2177480
Update Kitodo-DataEditor/src/main/java/org/kitodo/dataeditor/ruleset/…
matthias-ronge Jul 13, 2023
599eaeb
Update Kitodo-DataEditor/src/main/java/org/kitodo/dataeditor/ruleset/…
matthias-ronge Jul 13, 2023
32b1469
Update Kitodo/src/main/java/org/kitodo/production/forms/createprocess…
matthias-ronge Jul 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import java.util.Map;
import java.util.Optional;

import org.kitodo.api.Metadata;

/**
* Interface for a service that provides access to the ruleset.
*
Expand Down Expand Up @@ -143,4 +145,18 @@ StructuralElementViewInterface getStructuralElementView(String structuralElement
* @return the “always showing” value or its default value
*/
boolean isAlwaysShowingForKey(String keyId);

/**
* Updates metadata during a repeated catalog import, depending on the
* reimport settings specified in the ruleset.
*
* @param metadata
* current metadata
* @param acquisitionStage
* current acquisition stage
* @param updateItems
* items obtained from import
* @return number of added metadata items
*/
int updateMetadata(Collection<Metadata> metadata, String acquisitionStage, Collection<Metadata> updateItems);
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,22 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Locale.LanguageRange;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;

import javax.xml.bind.JAXBException;

import org.apache.commons.lang3.tuple.MutableTriple;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.kitodo.api.Metadata;
import org.kitodo.api.dataeditor.rulesetmanagement.FunctionalDivision;
import org.kitodo.api.dataeditor.rulesetmanagement.FunctionalMetadata;
import org.kitodo.api.dataeditor.rulesetmanagement.RulesetManagementInterface;
Expand All @@ -35,6 +39,7 @@
import org.kitodo.dataeditor.ruleset.xml.Division;
import org.kitodo.dataeditor.ruleset.xml.Key;
import org.kitodo.dataeditor.ruleset.xml.Namespace;
import org.kitodo.dataeditor.ruleset.xml.Reimport;
import org.kitodo.dataeditor.ruleset.xml.Ruleset;
import org.kitodo.dataeditor.ruleset.xml.Setting;
import org.kitodo.utils.JAXBContextCache;
Expand Down Expand Up @@ -321,5 +326,59 @@ public boolean isAlwaysShowingForKey(String keyId) {
return false;
}

@Override
public int updateMetadata(Collection<Metadata> currentMetadata, String acquisitionStage,
Collection<Metadata> updateMetadata) {

HashMap<String, MutableTriple<Collection<Metadata>, Reimport, Collection<Metadata>>> unifying = unifyMetadataByKey(
currentMetadata, acquisitionStage, updateMetadata);
int sizeBefore = currentMetadata.size();
currentMetadata.clear();
for (var entry : unifying.entrySet()) {
var metadata = entry.getValue();
Collection<Metadata> currentEntries = metadata.getLeft();
Collection<Metadata> updateEntries = metadata.getRight();

switch (metadata.getMiddle()) {
case ADD:
currentMetadata.addAll(currentEntries);
currentMetadata.addAll(updateEntries);
break;
case KEEP:
currentMetadata.addAll(currentEntries);
break;
case REPLACE:
currentMetadata.addAll(updateEntries.isEmpty() ? currentEntries : updateEntries);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a conditional replacement. If updateEntry value is empty it will not replaced. I think this should be at least in the description of the ruleset.xsd.

Maybe it makes sense to make two replace variants out of this. e.g. like it is the case with git-reset https://git-scm.com/docs/git-reset. So REPLACE_HARD will be to replace independent of updateEntries value and REPLACE_SOFT checks the updateEntries value before.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. currentEntries is the metadata (for that one key) that is in the user interface. It may be manually entered, or it may have come from the previous import, if that import returned different fields, like first importing a manuscript record from the library catalog, and then importing the record for that manuscript from a secondary database, that holds different information. Without the check, any other metadata, that doesn’t come with the second import, would be deleted during the repeated import, which should not be what the user expects.

break;
default:
throw new IllegalStateException(
"Used not supported reimport case {}".replace("{}", metadata.getMiddle().toString()));
}
}
return currentMetadata.size() - sizeBefore;
}

private HashMap<String, MutableTriple<Collection<Metadata>, Reimport, Collection<Metadata>>> unifyMetadataByKey(
Collection<Metadata> currentMetadata, String acquisitionStage, Collection<Metadata> updateMetadata) {

final Function<String, MutableTriple<Collection<Metadata>, Reimport, Collection<Metadata>>> entryProducer = key -> {
MutableTriple<Collection<Metadata>, Reimport, Collection<Metadata>> entry = new MutableTriple<>();
entry.setLeft(new ArrayList<>());
entry.setRight(new ArrayList<>());
return entry;
};

HashMap<String, MutableTriple<Collection<Metadata>, Reimport, Collection<Metadata>>> unifying = new HashMap<>();
for (Metadata metadata : currentMetadata) {
unifying.computeIfAbsent(metadata.getKey(), entryProducer).getLeft().add(metadata);
}
for (Metadata metadata : updateMetadata) {
unifying.computeIfAbsent(metadata.getKey(), entryProducer).getRight().add(metadata);
}
Settings settings = ruleset.getSettings(acquisitionStage);
for (var entry : unifying.entrySet()) {
entry.getValue().setMiddle(settings.getReimport(entry.getKey()));
}
return unifying;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;

import org.kitodo.dataeditor.ruleset.xml.Reimport;
import org.kitodo.dataeditor.ruleset.xml.Setting;

/**
Expand All @@ -44,6 +46,24 @@ public Settings(Collection<Setting> baseSettings) {
.collect(Collectors.toMap(Setting::getKey, Function.identity()));
}

/**
* Returns the reimport setting for a key.
*
* @param keyId
* key for which the query is
* @return reimport setting
*/
Reimport getReimport(String keyId) {
Setting settingForKey = currentSettings.get(keyId);
if (Objects.nonNull(settingForKey)) {
Reimport reimport = settingForKey.getReimport();
if (Objects.nonNull(reimport)) {
return reimport;
}
}
return Reimport.REPLACE;
}

/**
* Returns the settings for a key.
*
Expand Down Expand Up @@ -157,6 +177,7 @@ private List<Setting> merge(Collection<Setting> currentSettings, Collection<Sett
merged.setEditable(other.getEditable() != null ? other.getEditable() : current.getEditable());
merged.setExcluded(other.getExcluded() != null ? other.getExcluded() : current.getExcluded());
merged.setMultiline(other.getMultiline() != null ? other.getMultiline() : current.getMultiline());
merged.setReimport(other.getReimport() != null ? other.getReimport() : current.getReimport());
merged.setSettings(merge(current.getSettings(), other.getSettings()));
mergedSettings.add(merged);
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* (c) Kitodo. Key to digital objects e. V. <contact@kitodo.org>
*
* This file is part of the Kitodo project.
*
* It is licensed under GNU General Public License version 3 or later.
*
* For the full copyright and license information, please read the
* GPL3-License.txt file that was distributed with this source code.
*/

package org.kitodo.dataeditor.ruleset.xml;

import javax.xml.bind.annotation.XmlEnum;
import javax.xml.bind.annotation.XmlEnumValue;

/**
* This class is a backing bean for the XML attribute reimport in the
* ruleset. With it, JAXB can map the attribute to an enum.
*/
@XmlEnum(String.class)
matthias-ronge marked this conversation as resolved.
Show resolved Hide resolved
public enum Reimport {
/**
* The metadata should be added.
*/
@XmlEnumValue("add")
ADD,

/**
* The existing metadata should be kept.
*/
@XmlEnumValue("keep")
KEEP,

/**
* The existing metadata should be replaced.
*/
@XmlEnumValue("replace")
REPLACE
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ public class Setting {
@XmlAttribute
private Boolean multiline;

/**
* This will specify how to update metadata in repeated imports.
*/
@XmlAttribute
private Reimport reimport;

/**
* The settings for sub-keys.
*/
Expand Down Expand Up @@ -136,6 +142,17 @@ public Boolean getMultiline() {
return multiline;
}

/**
* Returns the value “reimport” if one is set. This getter returns
* {@code null} if the attribute was not entered. This is needed, for
* example, when merging attributes.
*
* @return the value “excluded”, if set, else {@code null}
*/
public Reimport getReimport() {
return reimport;
}

/**
* Returns the editor settings.
*
Expand Down Expand Up @@ -239,6 +256,17 @@ public void setMultiline(Boolean multiline) {
this.multiline = multiline;
}

/**
* This sets the “reimport” value. If you set the value to {@code null}, no
* attribute is written.
*
* @param reimport
* “reimport” value to set
*/
public void setReimport(Reimport reimport) {
this.reimport = reimport;
}

/**
* Sets the settings for nested keys.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Locale.LanguageRange;
Expand All @@ -40,6 +41,7 @@

import org.apache.commons.lang3.StringUtils;
import org.junit.Test;
import org.kitodo.api.MdSec;
import org.kitodo.api.Metadata;
import org.kitodo.api.MetadataEntry;
import org.kitodo.api.dataeditor.rulesetmanagement.ComplexMetadataViewInterface;
Expand Down Expand Up @@ -1066,6 +1068,102 @@ public void testValidationByCodomain() throws IOException {
Collections.emptyList()));
}

@Test
public void testReimportOfMetadataModesCreate() throws Exception {
RulesetManagement underTest = new RulesetManagement();
underTest.load(new File("src/test/resources/testReimportOfMetadataModes.xml"));

Collection<Metadata> metadata = new HashSet<>();
metadata.add(newMetadataEntry("metadataToReplaceByDefault", "value to replace"));
metadata.add(newMetadataEntry("metadataToReplaceExplicitly", "value to replace"));
metadata.add(newMetadataEntry("metadataToAdd", "value 1"));
metadata.add(newMetadataEntry("metadataToAddDuringCreationAndKeepLater", "value 1"));
metadata.add(newMetadataEntry("metadataToKeep", "value not to replace"));
metadata.add(newMetadataEntry("metadataToKeepExceptInEditing", "value not to replace"));

Collection<Metadata> imported = new HashSet<>();
imported.add(newMetadataEntry("metadataToReplaceByDefault", "replaced value")); // 0
imported.add(newMetadataEntry("metadataToReplaceExplicitly", "replaced value")); // 0
imported.add(newMetadataEntry("metadataToAdd", "value 2")); // 1
imported.add(newMetadataEntry("metadataToAdd", "value 3")); // 1
imported.add(newMetadataEntry("metadataToAddDuringCreationAndKeepLater", "value 2")); // 1
imported.add(newMetadataEntry("metadataToKeep", "value must not appear in result")); // 0
imported.add(newMetadataEntry("metadataToKeepExceptInEditing", "value not to replace")); // 0
imported.add(newMetadataEntry("metadataThatIsNew", "new value")); // 1

int numAdded = underTest.updateMetadata(metadata, "create", imported);
assertEquals(4, numAdded);

List<Metadata> defaultReplace = metadata.stream()
.filter(element -> element.getKey().equals("metadataToReplaceByDefault")).collect(Collectors.toList());
assertEquals(1, defaultReplace.size());
assertEquals("replaced value", ((MetadataEntry) defaultReplace.get(0)).getValue());

List<Metadata> explicitReplace = metadata.stream()
.filter(element -> element.getKey().equals("metadataToReplaceExplicitly")).collect(Collectors.toList());
assertEquals(1, explicitReplace.size());
assertEquals("replaced value", ((MetadataEntry) explicitReplace.get(0)).getValue());

List<Metadata> add = metadata.stream().filter(element -> element.getKey().equals("metadataToAdd"))
.collect(Collectors.toList());
assertEquals(3, add.size());
assertThat(
add.stream().map(MetadataEntry.class::cast).map(MetadataEntry::getValue).collect(Collectors.toList()),
containsInAnyOrder("value 1", "value 2", "value 3"));

List<Metadata> addCreate = metadata.stream()
.filter(element -> element.getKey().equals("metadataToAddDuringCreationAndKeepLater"))
.collect(Collectors.toList());
assertEquals(2, addCreate.size());
assertThat(
addCreate.stream().map(MetadataEntry.class::cast).map(MetadataEntry::getValue).collect(Collectors.toList()),
containsInAnyOrder("value 1", "value 2"));

List<Metadata> keep = metadata.stream().filter(element -> element.getKey().equals("metadataToKeep"))
.collect(Collectors.toList());
assertEquals(1, keep.size());
assertEquals("value not to replace", ((MetadataEntry) keep.get(0)).getValue());

List<Metadata> keepNoEdit = metadata.stream()
.filter(element -> element.getKey().equals("metadataToKeepExceptInEditing"))
.collect(Collectors.toList());
assertEquals(1, keepNoEdit.size());
assertEquals("value not to replace", ((MetadataEntry) keepNoEdit.get(0)).getValue());
}

@Test
public void testReimportOfMetadataModesEdit() throws Exception {
RulesetManagement underTest = new RulesetManagement();
underTest.load(new File("src/test/resources/testReimportOfMetadataModes.xml"));

Collection<Metadata> metadata = new HashSet<>();
metadata.add(newMetadataEntry("metadataToAddDuringCreationAndKeepLater", "value 1"));
metadata.add(newMetadataEntry("metadataToAddDuringCreationAndKeepLater", "value 2"));
metadata.add(newMetadataEntry("metadataToKeepExceptInEditing", "value 1"));
metadata.add(newMetadataEntry("metadataToKeepExceptInEditing", "value 2"));

Collection<Metadata> imported = new HashSet<>();
imported.add(newMetadataEntry("metadataToAddDuringCreationAndKeepLater", "value not to replace")); // 0
imported.add(newMetadataEntry("metadataToKeepExceptInEditing", "replaced value")); // -1

int numAdded = underTest.updateMetadata(metadata, "edit", imported);
assertEquals(-1, numAdded);

List<Metadata> keepLater = metadata.stream()
.filter(element -> element.getKey().equals("metadataToAddDuringCreationAndKeepLater"))
.collect(Collectors.toList());
assertEquals(2, keepLater.size());
assertThat(
keepLater.stream().map(MetadataEntry.class::cast).map(MetadataEntry::getValue).collect(Collectors.toList()),
containsInAnyOrder("value 1", "value 2"));

List<Metadata> replaceInEdit = metadata.stream()
.filter(element -> element.getKey().equals("metadataToKeepExceptInEditing"))
.collect(Collectors.toList());
assertEquals(1, replaceInEdit.size());
assertEquals("replaced value", ((MetadataEntry) replaceInEdit.get(0)).getValue());
}

/**
* The method provides a simple access to a metadata key in a list of
* MetadataViewWithValuesInterface.
Expand Down Expand Up @@ -1128,4 +1226,12 @@ private List<String> ids(List<MetadataViewWithValuesInterface> metadataViewWithV
.map(metadataViewWithValuesInterface -> metadataViewWithValuesInterface.getMetadata().get().getId())
.collect(Collectors.toList());
}

private MetadataEntry newMetadataEntry(String key, String value) {
MetadataEntry newMetadataEntry = new MetadataEntry();
newMetadataEntry.setKey(key);
newMetadataEntry.setValue(value);
newMetadataEntry.setDomain(MdSec.DMD_SEC);
return newMetadataEntry;
}
}
Loading