[#436] relation changelog

src/main/java/de/bonndan/nivio/input/ItemRelationProcessor.java

@@ -4,9 +4,9 @@
 import de.bonndan.nivio.input.dto.RelationDescription;
 import de.bonndan.nivio.model.*;
 import de.bonndan.nivio.search.ItemMatcher;
+import org.apache.commons.collections.CollectionUtils;
 
-import java.util.Iterator;
-import java.util.Optional;
+import java.util.*;
 
 /**
  * Creates {@link Relation}s between {@link Item}s.
@@ -19,70 +19,114 @@ protected ItemRelationProcessor(ProcessLog processLog) {
 
     @Override
     public ProcessingChangelog process(LandscapeDescription input, Landscape landscape) {
-        input.getItemDescriptions().all().forEach(serviceDescription -> {
-            Item origin = landscape.getItems().pick(serviceDescription);
-            if (!input.isPartial()) {
-                processLog.debug(String.format("Clearing relations of %s", origin));
-                origin.getRelations().clear(); //delete all relations on full update
-            }
-        });
+
+        ProcessingChangelog changelog = new ProcessingChangelog();
 
         input.getItemDescriptions().all().forEach(itemDescription -> {
             Item origin = landscape.getItems().pick(itemDescription);
+            List<Relation> affected = new ArrayList<>();
 
             for (RelationDescription relationDescription : itemDescription.getRelations()) {
+
+                if (!isValid(relationDescription, landscape)) {
+                    continue;
+                }
+
                 Optional<Relation> update = update(relationDescription, landscape, origin);
-                update.ifPresent(relation -> {
-                    origin.getRelations().remove(relation);
-                    origin.getRelations().add(relation);
-                    if (relation.getSource() == origin) {
-                        relation.getTarget().getRelations().remove(relation);
-                        relation.getTarget().getRelations().add(relation);
-                    } else {
-                        relation.getSource().getRelations().remove(relation);
-                        relation.getSource().getRelations().add(relation);
-                    }
-                });
+                if (update.isPresent()) {
+                    Relation relation = update.get();
+                    affected.add(relation);
+                    processLog.info(String.format("Updating relation between %s and %s", relation.getSource(), relation.getTarget()));
+                    changelog.addEntry(relation, ProcessingChangelog.ChangeType.UPDATED);
+                    continue;
+                }
+
+                Relation created = create(relationDescription, landscape);
+                affected.add(created);
+                processLog.info(String.format("Adding relation between %s and %s", created.getSource(), created.getTarget()));
+                changelog.addEntry(created, ProcessingChangelog.ChangeType.CREATED);
             }
+
+            affected.forEach(relation -> assignToBothEnds(origin, relation));
+            Collection<Relation> toDelete = CollectionUtils.disjunction(origin.getRelations(), affected);
+            toDelete.forEach(relation -> {
+                if (!origin.equals(relation.getSource()))
+                    return;
+                removefromBothEnds(origin, relation);
+                processLog.info(String.format("Removing relation between %s and %s", relation.getSource(), relation.getTarget()));
+                changelog.addEntry(relation, ProcessingChangelog.ChangeType.DELETED);
+            });
         });
 
-        return new ProcessingChangelog();
+        return changelog;
     }
 
-    private Optional<Relation> update(RelationDescription relationDescription, Landscape landscape, Item origin) {
+    private boolean isValid(RelationDescription relationDescription, Landscape landscape) {
 
         Optional<Item> source = findBy(relationDescription.getSource(), landscape);
         if (source.isEmpty()) {
             processLog.warn(String.format("Relation source %s not found", relationDescription.getSource()));
-            return Optional.empty();
+            return false;
         }
 
         Optional<Item> target = findBy(relationDescription.getTarget(), landscape);
         if (target.isEmpty()) {
             processLog.warn(String.format("Relation target %s not found", relationDescription.getTarget()));
-            return Optional.empty();
+            return false;
         }
 
+        return true;
+    }
+
+    private void assignToBothEnds(Item origin, Relation relation) {
+        removefromBothEnds(origin,relation);
+
+        origin.getRelations().add(relation);
+        if (relation.getSource() == origin) {
+            relation.getTarget().getRelations().add(relation);
+        } else {
+            relation.getSource().getRelations().add(relation);
+        }
+    }
+    private void removefromBothEnds(Item origin, Relation relation) {
+        origin.getRelations().remove(relation);
+        if (relation.getSource() == origin) {
+            relation.getTarget().getRelations().remove(relation);
+        } else {
+            relation.getSource().getRelations().remove(relation);
+        }
+    }
+
+
+    private Optional<Relation> update(RelationDescription relationDescription,
+                                      Landscape landscape,
+                                      Item origin
+    ) {
+        Item source = findBy(relationDescription.getSource(), landscape).orElseThrow();
+        Item target = findBy(relationDescription.getTarget(), landscape).orElseThrow();
+
         Iterator<Relation> iterator = origin.getRelations().iterator();
-        Relation existing = null;
-        Relation created = new Relation(source.get(), target.get());
+        Relation created = new Relation(source, target);
+        Relation existing;
         while (iterator.hasNext()) {
             existing = iterator.next();
             if (existing.equals(created)) {
-                processLog.info(String.format("Updating relation between %s and %s", existing.getSource(), existing.getTarget()));
                 return Optional.of(RelationBuilder.update(existing, relationDescription));
             }
         }
 
-        created = new Relation(created.getSource(),
-                created.getTarget(),
+        return Optional.empty();
+    }
+
+    private Relation create(RelationDescription relationDescription, Landscape landscape) {
+
+        return new Relation(
+                findBy(relationDescription.getSource(), landscape).orElseThrow(),
+                findBy(relationDescription.getTarget(), landscape).orElseThrow(),
                 relationDescription.getDescription(),
                 relationDescription.getFormat(),
                 relationDescription.getType()
         );
-
-        processLog.info(String.format("Adding relation from %s to %s", created.getSource(), created.getTarget()));
-        return Optional.of(created);
     }
 
     private Optional<Item> findBy(String term, Landscape landscape) {

src/main/java/de/bonndan/nivio/input/ProcessingChangelog.java

@@ -5,6 +5,7 @@
 import io.swagger.v3.oas.annotations.media.Schema;
 import org.springframework.lang.NonNull;
 import org.springframework.lang.Nullable;
+import org.springframework.util.StringUtils;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -28,7 +29,10 @@ public void addEntry(
             @NonNull final ChangeType changeType,
             @Nullable final String message
     ) {
-        String id = Objects.requireNonNull(component).getFullyQualifiedIdentifier().jsonValue();
+        String id = Objects.requireNonNull(component).getFullyQualifiedIdentifier().toString();
+        if (StringUtils.isEmpty(id)) {
+            throw new RuntimeException("Could not create a changelog entry id for " + component);
+        }
         Entry entry = new Entry(
                 component.getClass().getSimpleName(),
                 Objects.requireNonNull(changeType),
@@ -56,6 +60,9 @@ public void addEntry(
     ) {
         Objects.requireNonNull(relation);
         final String id = getRelationKey(relation);
+        if (StringUtils.isEmpty(id)) {
+            throw new RuntimeException("Could not create a changelog entry id for " + relation);
+        }
         Entry entry = new Entry(
                 relation.getClass().getSimpleName(),
                 Objects.requireNonNull(changeType),
@@ -119,6 +126,15 @@ public String getChangeType() {
         public String getMessage() {
             return message;
         }
+
+        @Override
+        public String toString() {
+            return "Entry{" +
+                    "componentType='" + componentType + '\'' +
+                    ", changeType='" + changeType + '\'' +
+                    ", message='" + message + '\'' +
+                    '}';
+        }
     }
 
     enum ChangeType {

src/main/java/de/bonndan/nivio/model/RelationBuilder.java

@@ -50,6 +50,14 @@ public static RelationDescription provides(String source, ItemDescription target
         return relationDescription;
     }
 
+    /**
+     * Returns a new relation with values updated by the description.
+     *
+     * @param existing    existing relation
+     * @param description incoming data
+     * @return new copy
+     */
+    @NonNull
     public static Relation update(@NonNull final Relation existing, @NonNull final RelationDescription description) {
         Objects.requireNonNull(existing);
         Objects.requireNonNull(description);

src/test/java/de/bonndan/nivio/input/ItemRelationProcessorTest.java

@@ -0,0 +1,83 @@
+package de.bonndan.nivio.input;
+
+import de.bonndan.nivio.input.dto.ItemDescription;
+import de.bonndan.nivio.input.dto.LandscapeDescription;
+import de.bonndan.nivio.input.dto.RelationDescription;
+import de.bonndan.nivio.model.*;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.slf4j.LoggerFactory;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+
+class ItemRelationProcessorTest {
+
+    private LandscapeDescription input;
+    private Landscape landscape;
+    private ItemRelationProcessor processor;
+
+    @BeforeEach
+    void setUp() {
+        input = new LandscapeDescription("test");
+        Set<Item> items = new HashSet<>();
+        Item foo = ItemFactory.getTestItem("a", "foo");
+        items.add(foo);
+        Item bar = ItemFactory.getTestItem("a", "bar");
+        items.add(bar);
+        Item baz = ItemFactory.getTestItem("a", "baz");
+        items.add(baz);
+
+        foo.getRelations().add(new Relation(foo, bar));
+        bar.getRelations().add(new Relation(foo, bar));
+
+        foo.getRelations().add(new Relation(foo, baz));
+        baz.getRelations().add(new Relation(foo, baz));
+
+        landscape = LandscapeFactory.createForTesting("test", "test").withItems(items).build();
+
+        processor = new ItemRelationProcessor(new ProcessLog(LoggerFactory.getLogger(ItemRelationProcessorTest.class)));
+    }
+
+    @Test
+    void processAddsRelation() {
+
+        ItemDescription description = new ItemDescription("foo");
+        description.setGroup("a");
+        description.addRelation(new RelationDescription("foo", "bar"));
+        description.addRelation(new RelationDescription("foo", "baz"));
+        input.addItems(List.of(description));
+        //new
+        ItemDescription bar = new ItemDescription("bar");
+        bar.setGroup("a");
+        description.addRelation(new RelationDescription("bar", "baz"));
+        input.addItems(List.of(bar));
+
+        //when
+        ProcessingChangelog process = processor.process(input, landscape);
+
+        //then
+        assertThat(process.changes).hasSize(3); //two updates, one created
+    }
+
+    @Test
+    void processRemovesRelation() {
+
+        ItemDescription description = new ItemDescription("foo");
+        description.setGroup("a");
+        description.addRelation(new RelationDescription("foo", "bar"));
+        input.addItems(List.of(description));
+
+        //when
+        ProcessingChangelog process = processor.process(input, landscape);
+
+        //then
+        assertThat(process.changes).hasSize(2); //one update, one delete
+        assertThat(process.changes).containsKey("test/a/foo;test/a/baz");
+        assertThat(process.changes.get("test/a/foo;test/a/baz").getChangeType()).isEqualTo(ProcessingChangelog.ChangeType.DELETED.name());
+    }
+}