Bug 581517 - Avoid firing event if already hidden

Signed-off-by: Dirk Fauth <dirk.fauth@googlemail.com>

Change-Id: I0414b3160d440e9fed7191e4dfbb63ea03f4bfe9

4 files changed, 151 insertions, 15 deletions

diff --git a/org.eclipse.nebula.widgets.nattable.core.test/src/org/eclipse/nebula/widgets/nattable/hideshow/command/HideColumnCommandTest.java b/org.eclipse.nebula.widgets.nattable.core.test/src/org/eclipse/nebula/widgets/nattable/hideshow/command/HideColumnCommandTest.java
index 6145bb69..095422d9 100644
--- a/org.eclipse.nebula.widgets.nattable.core.test/src/org/eclipse/nebula/widgets/nattable/hideshow/command/HideColumnCommandTest.java
+++ b/org.eclipse.nebula.widgets.nattable.core.test/src/org/eclipse/nebula/widgets/nattable/hideshow/command/HideColumnCommandTest.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2020 Original authors and others.
+ * Copyright (c) 2012, 2023 Original authors and others.
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -14,10 +14,16 @@ package org.eclipse.nebula.widgets.nattable.hideshow.command;
 
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Iterator;
 
 import org.eclipse.nebula.widgets.nattable.command.ILayerCommand;
+import org.eclipse.nebula.widgets.nattable.coordinate.Range;
 import org.eclipse.nebula.widgets.nattable.hideshow.ColumnHideShowLayer;
+import org.eclipse.nebula.widgets.nattable.hideshow.event.HideColumnPositionsEvent;
 import org.eclipse.nebula.widgets.nattable.test.fixture.layer.DataLayerFixture;
+import org.eclipse.nebula.widgets.nattable.test.fixture.layer.LayerListenerFixture;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -63,4 +69,58 @@ public class HideColumnCommandTest {
                 () -> assertEquals(4, this.columnHideShowLayer.getColumnIndexByPosition(3)));
     }
 
+    @Test
+    public void shouldNotFireEventIfColumnIndexIsAlreadyHidden() {
+        LayerListenerFixture listener = new LayerListenerFixture();
+        this.columnHideShowLayer.addLayerListener(listener);
+
+        shouldHideColumnByIndex();
+
+        assertAll("first event fired",
+                () -> assertEquals(1, listener.getEventsCount()),
+                () -> assertTrue(listener.containsInstanceOf(HideColumnPositionsEvent.class)),
+                () -> {
+                    HideColumnPositionsEvent event = (HideColumnPositionsEvent) listener.getReceivedEvents().get(0);
+                    assertEquals(1, event.getColumnIndexes().length);
+                    assertEquals(2, event.getColumnIndexes()[0]);
+                    assertEquals(1, event.getColumnPositionRanges().size());
+                    assertEquals(new Range(2, 3), event.getColumnPositionRanges().iterator().next());
+                });
+
+        listener.clearReceivedEvents();
+
+        // try to hide the already hidden column again
+        this.columnHideShowLayer.doCommand(new HideColumnByIndexCommand(2));
+
+        assertEquals(0, listener.getEventsCount());
+    }
+
+    @Test
+    public void shouldNotContainAlreadyHiddenColumnIndexInEvent() {
+        LayerListenerFixture listener = new LayerListenerFixture();
+        this.columnHideShowLayer.addLayerListener(listener);
+
+        shouldHideColumnByIndex();
+
+        listener.clearReceivedEvents();
+
+        // try to hide multiple columns with the already hidden column 2
+        this.columnHideShowLayer.doCommand(new HideColumnByIndexCommand(1, 2, 3));
+
+        assertAll("event fired with correct data",
+                () -> assertEquals(1, listener.getEventsCount()),
+                () -> assertTrue(listener.containsInstanceOf(HideColumnPositionsEvent.class)),
+                () -> {
+                    HideColumnPositionsEvent event = (HideColumnPositionsEvent) listener.getReceivedEvents().get(0);
+                    assertEquals(2, event.getColumnIndexes().length);
+                    assertEquals(1, event.getColumnIndexes()[0]);
+                    assertEquals(3, event.getColumnIndexes()[1]);
+                    // there is only one range [1,3] because column 2 is already
+                    // hidden
+                    assertEquals(1, event.getColumnPositionRanges().size());
+                    Iterator<Range> it = event.getColumnPositionRanges().iterator();
+                    assertEquals(new Range(1, 3), it.next());
+                });
+    }
+
 }
diff --git a/org.eclipse.nebula.widgets.nattable.core.test/src/org/eclipse/nebula/widgets/nattable/hideshow/command/HideRowCommandTest.java b/org.eclipse.nebula.widgets.nattable.core.test/src/org/eclipse/nebula/widgets/nattable/hideshow/command/HideRowCommandTest.java
index a1822609..99cd6318 100644
--- a/org.eclipse.nebula.widgets.nattable.core.test/src/org/eclipse/nebula/widgets/nattable/hideshow/command/HideRowCommandTest.java
+++ b/org.eclipse.nebula.widgets.nattable.core.test/src/org/eclipse/nebula/widgets/nattable/hideshow/command/HideRowCommandTest.java
@@ -15,11 +15,17 @@ package org.eclipse.nebula.widgets.nattable.hideshow.command;
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Iterator;
 
 import org.eclipse.nebula.widgets.nattable.command.ILayerCommand;
+import org.eclipse.nebula.widgets.nattable.coordinate.Range;
 import org.eclipse.nebula.widgets.nattable.hideshow.RowHideShowLayer;
+import org.eclipse.nebula.widgets.nattable.hideshow.event.HideRowPositionsEvent;
 import org.eclipse.nebula.widgets.nattable.reorder.RowReorderLayer;
 import org.eclipse.nebula.widgets.nattable.test.fixture.layer.DataLayerFixture;
+import org.eclipse.nebula.widgets.nattable.test.fixture.layer.LayerListenerFixture;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -75,4 +81,58 @@ public class HideRowCommandTest {
 
         assertFalse(hideRowCommand.convertToTargetLayer(this.rowHideShowLayer));
     }
+
+    @Test
+    public void shouldNotFireEventIfRowIndexIsAlreadyHidden() {
+        LayerListenerFixture listener = new LayerListenerFixture();
+        this.rowHideShowLayer.addLayerListener(listener);
+
+        shouldHideRowByIndex();
+
+        assertAll("first event fired",
+                () -> assertEquals(1, listener.getEventsCount()),
+                () -> assertTrue(listener.containsInstanceOf(HideRowPositionsEvent.class)),
+                () -> {
+                    HideRowPositionsEvent event = (HideRowPositionsEvent) listener.getReceivedEvents().get(0);
+                    assertEquals(1, event.getRowIndexes().length);
+                    assertEquals(2, event.getRowIndexes()[0]);
+                    assertEquals(1, event.getRowPositionRanges().size());
+                    assertEquals(new Range(2, 3), event.getRowPositionRanges().iterator().next());
+                });
+
+        listener.clearReceivedEvents();
+
+        // try to hide the already hidden row again
+        this.rowHideShowLayer.doCommand(new HideRowByIndexCommand(2));
+
+        assertEquals(0, listener.getEventsCount());
+    }
+
+    @Test
+    public void shouldNotContainAlreadyHiddenRowIndexInEvent() {
+        LayerListenerFixture listener = new LayerListenerFixture();
+        this.rowHideShowLayer.addLayerListener(listener);
+
+        shouldHideRowByIndex();
+
+        listener.clearReceivedEvents();
+
+        // try to hide multiple rows with the already hidden row 2
+        this.rowHideShowLayer.doCommand(new HideRowByIndexCommand(1, 2, 3));
+
+        assertAll("event fired with correct data",
+                () -> assertEquals(1, listener.getEventsCount()),
+                () -> assertTrue(listener.containsInstanceOf(HideRowPositionsEvent.class)),
+                () -> {
+                    HideRowPositionsEvent event = (HideRowPositionsEvent) listener.getReceivedEvents().get(0);
+                    assertEquals(2, event.getRowIndexes().length);
+                    assertEquals(1, event.getRowIndexes()[0]);
+                    assertEquals(3, event.getRowIndexes()[1]);
+                    // there is only one range [1,3] because column 2 is already
+                    // hidden
+                    assertEquals(1, event.getRowPositionRanges().size());
+                    Iterator<Range> it = event.getRowPositionRanges().iterator();
+                    assertEquals(new Range(1, 3), it.next());
+                });
+    }
 }
diff --git a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/hideshow/ColumnHideShowLayer.java b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/hideshow/ColumnHideShowLayer.java
index 8869950c..bc8c3e40 100644
--- a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/hideshow/ColumnHideShowLayer.java
+++ b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/hideshow/ColumnHideShowLayer.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2020 Original authors and others.
+ * Copyright (c) 2012, 2023 Original authors and others.
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -188,13 +188,22 @@ public class ColumnHideShowLayer extends AbstractColumnHideShowLayer implements
 
     @Override
     public void hideColumnIndexes(int... columnIndexes) {
-        int[] columnPositions = Arrays.stream(columnIndexes)
-                .map(this::getColumnPositionByIndex)
-                .sorted()
+        int[] filteredIndexes = Arrays.stream(columnIndexes)
+                .filter(index -> !this.hiddenColumnIndexes.contains(index))
                 .toArray();
-        this.hiddenColumnIndexes.addAll(columnIndexes);
-        invalidateCache();
-        fireLayerEvent(new HideColumnPositionsEvent(this, columnPositions, columnIndexes));
+
+        // only fire an update if something will change
+        if (filteredIndexes.length > 0) {
+            int[] columnPositions = Arrays.stream(filteredIndexes)
+                    .filter(index -> !this.hiddenColumnIndexes.contains(index))
+                    .map(this::getColumnPositionByIndex)
+                    .sorted()
+                    .toArray();
+
+            this.hiddenColumnIndexes.addAll(filteredIndexes);
+            invalidateCache();
+            fireLayerEvent(new HideColumnPositionsEvent(this, columnPositions, filteredIndexes));
+        }
     }
 
     @Override
diff --git a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/hideshow/RowHideShowLayer.java b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/hideshow/RowHideShowLayer.java
index dec153d8..a6d9a5eb 100644
--- a/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/hideshow/RowHideShowLayer.java
+++ b/org.eclipse.nebula.widgets.nattable.core/src/org/eclipse/nebula/widgets/nattable/hideshow/RowHideShowLayer.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2020 Original authors and others.
+ * Copyright (c) 2012, 2023 Original authors and others.
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -171,13 +171,20 @@ public class RowHideShowLayer extends AbstractRowHideShowLayer implements IRowHi
 
     @Override
     public void hideRowIndexes(int... rowIndexes) {
-        int[] rowPositions = Arrays.stream(rowIndexes)
-                .map(this::getRowPositionByIndex)
-                .sorted()
+        int[] filteredIndexes = Arrays.stream(rowIndexes)
+                .filter(index -> !this.hiddenRowIndexes.contains(index))
                 .toArray();
-        this.hiddenRowIndexes.addAll(rowIndexes);
-        invalidateCache();
-        fireLayerEvent(new HideRowPositionsEvent(this, rowPositions, rowIndexes));
+
+        // only fire an update if something will change
+        if (filteredIndexes.length > 0) {
+            int[] rowPositions = Arrays.stream(filteredIndexes)
+                    .map(this::getRowPositionByIndex)
+                    .sorted()
+                    .toArray();
+            this.hiddenRowIndexes.addAll(filteredIndexes);
+            invalidateCache();
+            fireLayerEvent(new HideRowPositionsEvent(this, rowPositions, filteredIndexes));
+        }
     }
 
     @Override