Merge pull request #497 from dedica-team/439_removed_items

439 removed items
dedica-team · Mar 27, 2021 · 70726d5 · 70726d5
2 parents f917972 + a8d40da
commit 70726d5
Show file tree

Hide file tree

Showing 8 changed files with 104 additions and 28 deletions.
diff --git a/docs/source/schema/Models/LandscapeDescription.rst b/docs/source/schema/Models/LandscapeDescription.rst
@@ -85,7 +85,7 @@ LandscapeDescription
      - null
    * - color
      - String
-     - 
+     -
      - optional, defaults to null
      - null
    * - links

diff --git a/src/main/app/src/Components/Landscape/Map/Map.tsx b/src/main/app/src/Components/Landscape/Map/Map.tsx
@@ -46,8 +46,7 @@ const useStyles = makeStyles((theme: Theme) =>
     menuIcon: {
       position: 'absolute',
       cursor: 'pointer',
-      bottom: '10%',
-
+      top: '70px',
       zIndex: 1000,
       backgroundColor: darken(theme.palette.primary.main, 0.2),
     }
@@ -88,7 +87,7 @@ const Map: React.FC<Props> = ({ setSidebarContent, setPageTitle }) => {
   const [assessments, setAssessments] = useState<IAssessment | undefined>(undefined);
 
   const [isFirstRender, setIsFirstRender] = useState(true);
-  const [isZoomed, setISZommed] = useState<Boolean>(false);
+  const [isZoomed, setIsZoomed] = useState<Boolean>(false);
 
   const locateFunctionContext = useContext(LocateFunctionContext);
   const notificationContext = useContext(NotificationContext);
@@ -273,11 +272,11 @@ const Map: React.FC<Props> = ({ setSidebarContent, setPageTitle }) => {
       <div className='landscapeMapContainer'>
         {isZoomed && <IconButton
           className={classes.menuIcon}
-          title={'Click to Reset View'}
+          title={'Click to reset view'}
           onClick={() => {
             // @ts-ignore  
             setValue(fitToViewer(value, 'center', 'center'));
-            setISZommed(false)
+            setIsZoomed(false)
           }}
           size={'small'}
         >
@@ -306,7 +305,7 @@ const Map: React.FC<Props> = ({ setSidebarContent, setPageTitle }) => {
               preventPanOutside={false}
               toolbarProps={{ position: 'none' }}
               detectAutoPan={false}
-              onZoom={() => { setISZommed(true) }}
+              onZoom={() => { setIsZoomed(true) }}
               tool={tool}
               onChangeTool={(newTool) => setTool(newTool)}
               value={value}

diff --git a/src/main/java/de/bonndan/nivio/input/DiffProcessor.java b/src/main/java/de/bonndan/nivio/input/DiffProcessor.java
@@ -4,6 +4,8 @@
 import de.bonndan.nivio.input.dto.LandscapeDescription;
 import de.bonndan.nivio.model.*;
 import de.bonndan.nivio.search.ItemMatcher;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.*;
 import java.util.stream.Collectors;
@@ -15,6 +17,8 @@
  */
 public class DiffProcessor extends Processor {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(DiffProcessor.class);
+
     protected DiffProcessor(ProcessLog processLog) {
         super(processLog);
     }
@@ -61,12 +65,23 @@ public void process(LandscapeDescription input, Landscape landscape) {
                 }
         );
 
-        landscape.setItems(inLandscape);
-        deleteUnreferenced(input, inLandscape, existingItems, processLog)
-                .forEach(item -> landscape.getItems().all().remove(item));
+        //cleanup
+        landscape.setItems(inLandscape); //this already removes the items from the landscape.items
+
+        //remove references left over in groups
+        List<Item> toDelete = getUnreferenced(input, inLandscape, existingItems, processLog);
+        toDelete.forEach(item -> {
+            processLog.info(String.format("Removing item %s from landscape", item));
+            landscape.getGroup(item.getGroup()).ifPresent(group -> {
+                boolean removed = group.removeItem(item);
+                if (!removed) {
+                    LOGGER.warn("Failed to remove item {}", item);
+                }
+            });
+        });
     }
 
-    private List<Item> deleteUnreferenced(
+    private List<Item> getUnreferenced(
             final LandscapeDescription landscapeDescription,
             Set<Item> kept,
             Set<Item> all,
@@ -77,9 +92,7 @@ private List<Item> deleteUnreferenced(
             return new ArrayList<>();
         }
 
-        List<Item> removed = removed(kept, all);
-        logger.info("Removing " + removed.size() + " sources in env " + landscapeDescription.getIdentifier());
-        return removed;
+        return removed(kept, all);
     }
 
     /**
@@ -102,7 +115,7 @@ static List<Item> removed(Collection<Item> items, Collection<Item> itemDescripti
 
     /**
      * Returns all elements which are not in the second list
-     * @return
+     *
      */
     static List<ItemDescription> added(Collection<ItemDescription> itemDescriptions, Collection<Item> existingItems, Landscape landscape) {
         return itemDescriptions.stream()

diff --git a/src/main/java/de/bonndan/nivio/model/Group.java b/src/main/java/de/bonndan/nivio/model/Group.java
@@ -206,6 +206,19 @@ public void addItem(Item item) {
         items.add(item);
     }
 
+    /**
+     * Removes an item from a group.
+     *
+     * @param item the item to remove
+     * @return true if the item could be removed
+     */
+    public boolean removeItem(@Nullable Item item) {
+        if (item == null) {
+            return false;
+        }
+        return items.remove(item);
+    }
+
     public String getLandscapeIdentifier() {
         return landscapeIdentifier;
     }

diff --git a/src/main/java/de/bonndan/nivio/output/map/RenderCache.java b/src/main/java/de/bonndan/nivio/output/map/RenderCache.java
@@ -43,7 +43,7 @@ public RenderCache(SVGRenderer svgRenderer) {
      * Returns an svg.
      *
      * @param landscape the landscape to render
-     * @param debug
+     * @param debug     flag to enable debug messages
      * @return the svg as string, uncached
      */
     @Nullable
@@ -70,8 +70,6 @@ private void createCacheEntry(Landscape landscape, boolean debug) {
     @Override
     public void onApplicationEvent(ProcessingFinishedEvent processingFinishedEvent) {
         Landscape landscape = processingFinishedEvent.getLandscape();
-        if (landscape != null) {
-            createCacheEntry(landscape, false);
-        }
+        createCacheEntry(landscape, false);
     }
 }
diff --git a/src/test/java/de/bonndan/nivio/input/DiffProcessorTest.java b/src/test/java/de/bonndan/nivio/input/DiffProcessorTest.java
@@ -1,36 +1,48 @@
 package de.bonndan.nivio.input;
 
 import de.bonndan.nivio.input.dto.ItemDescription;
+import de.bonndan.nivio.input.dto.LandscapeDescription;
 import de.bonndan.nivio.model.Group;
 import de.bonndan.nivio.model.Item;
 import de.bonndan.nivio.model.LandscapeFactory;
 import de.bonndan.nivio.model.Landscape;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-import java.util.ArrayList;
-import java.util.List;
+import java.util.*;
 
 import static de.bonndan.nivio.model.ItemFactory.getTestItem;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.mock;
 
 public class DiffProcessorTest {
 
-    private ArrayList<Item> items;
+    private Set<Item> items;
     private Landscape landscape;
+    private Group g1;
 
     @BeforeEach
     public void setup() {
 
-        landscape = LandscapeFactory.createForTesting("test", "l1name").build();
+        g1 = new Group("g1", "test");
+        landscape = LandscapeFactory.createForTesting("test", "l1name")
+                .withGroups(Map.of("g1", g1))
+                .build();
 
-        items = new ArrayList<>();
+        items = new HashSet<>();
 
-        Item s1 = getTestItem("g1","s1",landscape);
+        Item s1 = getTestItem("g1", "s1", landscape);
         items.add(s1);
 
-        Item s2 = getTestItem("g1","s2",landscape);
+        Item s2 = getTestItem("g1", "s2", landscape);
         items.add(s2);
+        landscape.setItems(items);
+
+        g1.addItem(s1);
+        g1.addItem(s2);
     }
 
     @Test
@@ -145,4 +157,21 @@ public void keptNone() {
         List<Item> kept = DiffProcessor.kept(items1, items2, landscape);
         assertEquals(0, kept.size());
     }
+
+    @Test
+    void regression439() {
+
+        DiffProcessor diffProcessor = new DiffProcessor(new ProcessLog(LoggerFactory.getLogger(DiffProcessorTest.class)));
+
+        LandscapeDescription input = new LandscapeDescription(landscape.getIdentifier());
+        ItemDescription description = new ItemDescription("s1");
+        description.setGroup("g1");
+        input.setItems(List.of(description));
+
+        //when
+        diffProcessor.process(input, landscape);
+
+        //then
+        assertThat(g1.getItems()).hasSize(1);
+    }
 }
diff --git a/src/test/java/de/bonndan/nivio/model/GroupTest.java b/src/test/java/de/bonndan/nivio/model/GroupTest.java
@@ -3,6 +3,7 @@
 import org.junit.jupiter.api.Test;
 
 import static de.bonndan.nivio.model.ItemFactory.getTestItem;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.*;
 
 class GroupTest {
@@ -32,4 +33,27 @@ void addItemForbidden() {
         Item item = getTestItem("a", "b");
         assertThrows(IllegalArgumentException.class, () -> g.addItem(item));
     }
+
+    @Test
+    void removeItem() {
+        Group g = new Group("foo", null);
+        Item item = getTestItem("foo", "b");
+        g.addItem(item);
+        assertEquals(1, g.getItems().size());
+
+        boolean b = g.removeItem(item);
+        assertThat(b).isTrue();
+        assertThat(g.getItems()).hasSize(0);
+    }
+
+    @Test
+    void removeItemFails() {
+        Group g = new Group("foo", null);
+        Item item = getTestItem("foo", "b");
+        g.addItem(item);
+
+        boolean b = g.removeItem(getTestItem("foo", "c"));
+        assertThat(b).isFalse();
+        assertThat(g.getItems()).hasSize(1);
+    }
 }
diff --git a/src/test/resources/example/example_items_extrafields.yml b/src/test/resources/example/example_items_extrafields.yml
@@ -1,7 +1,7 @@
-identifier: nivio:example
+identifier: nivio:example_extrafields
 name: items with extra fields
 
 items:
   - identifier: one
     foo: bar
-    bar: baz
+    bar: baz