Vojtech Szocs has uploaded a new change for review.
Change subject: webadmin,userportal: Consolidate SortedListModel vs.
SearchableListModel
......................................................................
webadmin,userportal: Consolidate SortedListModel vs. SearchableListModel
This patch removes SearchableListModel#setComparator override,
improves code in SortedListModel and reflects these changes into
associated unit tests.
Change-Id: Id10b84c2fb476ed2240c33d72f6432335650df33
Signed-off-by: Vojtech Szocs <vsz...@redhat.com>
---
M
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
M
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModel.java
M
frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModelTest.java
3 files changed, 176 insertions(+), 69 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/01/31101/1
diff --git
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
index 270a163..4cb6441 100644
---
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
+++
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SearchableListModel.java
@@ -3,12 +3,10 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
-import java.util.Comparator;
import java.util.Date;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
-import java.util.SortedSet;
import java.util.logging.Logger;
import org.ovirt.engine.core.common.businessentities.BusinessEntity;
@@ -825,17 +823,6 @@
}
@Override
- public void setComparator(Comparator<? super T> comparator, boolean
sortAscending) {
- super.setComparator(comparator, sortAscending);
-
- Collection<T> items = getItems();
- if (items != null) {
- Collection<T> maybeSortedItems = (comparator != null) ?
sortItems(items) : new ArrayList<T>(items);
- setItems(maybeSortedItems);
- }
- }
-
- @Override
public void setItems(Collection<T> value)
{
if (items != value)
@@ -851,14 +838,13 @@
}
}
- if (comparator == null || ((value instanceof SortedSet)
- && ObjectUtils.objectsEqual(((SortedSet<?>)
value).comparator(), comparator))) {
- itemsChanging(value, items);
- items = value;
- } else {
+ if (useComparator(value)) {
Collection<T> sortedItems = sortItems(value);
itemsChanging(sortedItems, items);
items = sortedItems;
+ } else {
+ itemsChanging(value, items);
+ items = value;
}
updatePagingAvailability();
diff --git
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModel.java
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModel.java
index fc38f8b..0055b86 100644
---
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModel.java
+++
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModel.java
@@ -21,6 +21,10 @@
this.sortAscending = sortAscending;
}
+ SortSensitiveComparator(Comparator<? super T> comparator) {
+ this(comparator, DEFAULT_SORT_ASCENDING);
+ }
+
@Override
public int compare(T a, T b) {
return sortAscending ? comparator.compare(a, b) :
comparator.compare(b, a);
@@ -55,7 +59,9 @@
}
- protected Comparator<T> comparator;
+ private static final boolean DEFAULT_SORT_ASCENDING = true;
+
+ private Comparator<T> comparator;
public SortedListModel(Comparator<? super T> comparator) {
super();
@@ -66,28 +72,38 @@
this(null);
}
- public final void setComparator(Comparator<? super T> comparator) {
- setComparator(comparator, true);
+ public void setComparator(Comparator<? super T> comparator) {
+ setComparator(comparator, DEFAULT_SORT_ASCENDING);
}
public void setComparator(Comparator<? super T> comparator, boolean
sortAscending) {
this.comparator = (comparator != null) ? new
SortSensitiveComparator<T>(comparator, sortAscending) : null;
+
+ // Sort items after updating comparator
+ Collection<T> items = getItems();
+ if (items != null) {
+ Collection<T> maybeSortedItems = useComparator(items) ?
sortItems(items) : new ArrayList<T>(items);
+ super.setItems(maybeSortedItems);
+ }
}
@Override
- public void setItems(Collection<T> value) {
- Collection<T> sortedItems = sortItems(value);
- super.setItems(sortedItems);
+ public void setItems(Collection<T> items) {
+ Collection<T> maybeSortedItems = useComparator(items) ?
sortItems(items) : items;
+ super.setItems(maybeSortedItems);
}
- protected final Collection<T> sortItems(Collection<T> items) {
- if (items == null || comparator == null) {
- return items;
+ protected boolean useComparator(Collection<T> items) {
+ return items != null && comparator != null;
+ }
+
+ protected Collection<T> sortItems(Collection<T> items) {
+ if (comparator == null) {
+ throw new IllegalStateException("comparator cannot be null");
//$NON-NLS-1$
}
List<T> sortedList = new ArrayList<T>(items);
Collections.sort(sortedList, comparator);
-
return sortedList;
}
diff --git
a/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModelTest.java
b/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModelTest.java
index 008f933..de02e9f 100644
---
a/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModelTest.java
+++
b/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModelTest.java
@@ -1,18 +1,20 @@
package org.ovirt.engine.ui.uicommonweb.models;
+import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThat;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.Comparator;
-import java.util.List;
-import org.junit.Before;
+import org.junit.ClassRule;
import org.junit.Test;
-import org.ovirt.engine.ui.uicommonweb.Configurator;
-import org.ovirt.engine.ui.uicommonweb.ILogger;
+import org.ovirt.engine.ui.uicommonweb.junit.UiCommonSetup;
public class SortedListModelTest {
@@ -56,6 +58,11 @@
return true;
}
+ @Override
+ public String toString() {
+ return "ITEM_" + value + "_" + salt; //$NON-NLS-1$ //$NON-NLS-2$
+ }
+
}
static final TestItem ITEM_1_1 = new TestItem(1, 1);
@@ -63,7 +70,10 @@
static final TestItem ITEM_2_1 = new TestItem(2, 1);
static final TestItem ITEM_2_2 = new TestItem(2, 2);
- SortedListModel<TestItem> testedModel;
+ static final Collection<TestItem> SOME_ITEMS = Arrays.asList(ITEM_1_2,
ITEM_1_1);
+
+ @ClassRule
+ public static UiCommonSetup setup = new UiCommonSetup();
Comparator<TestItem> makeValueComparator() {
return new Comparator<TestItem>() {
@@ -83,60 +93,155 @@
};
}
- @Before
- public void setUp() {
- testedModel = new SortedListModel<TestItem>() {
- @Override
- protected Configurator lookupConfigurator() {
- return null;
- }
-
- @Override
- protected ILogger lookupLogger() {
- return null;
- }
- };
+ @Test
+ public void testUseComparator_nullItems() {
+ SortedListModel<TestItem> testedModel = new
SortedListModel<TestItem>(makeValueComparator());
+ assertThat(testedModel.useComparator(null), is(false));
}
@Test
- public void testSortItems_nullComparator() {
- List<TestItem> initial = Arrays.asList(ITEM_1_2, ITEM_1_1);
-
- testedModel.setComparator(null);
- Collection<TestItem> sorted = testedModel.sortItems(initial);
- assertEquals(sorted, initial);
+ public void testUseComparator_nullComparator() {
+ SortedListModel<TestItem> testedModel = new
SortedListModel<TestItem>();
+ assertThat(testedModel.useComparator(SOME_ITEMS), is(false));
}
@Test
+ public void testUseComparator_happyPath() {
+ SortedListModel<TestItem> testedModel = new
SortedListModel<TestItem>(makeValueComparator());
+ assertThat(testedModel.useComparator(SOME_ITEMS), is(true));
+ }
+
+ @Test(expected = NullPointerException.class)
public void testSortItems_nullItems() {
- testedModel.setComparator(makeValueComparator());
- Collection<TestItem> sorted = testedModel.sortItems(null);
- assertNull(sorted);
+ SortedListModel<TestItem> testedModel = new
SortedListModel<TestItem>(makeValueComparator());
+ testedModel.sortItems(null);
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void testSortItems_nullComparator() {
+ SortedListModel<TestItem> testedModel = new
SortedListModel<TestItem>();
+ testedModel.sortItems(SOME_ITEMS);
}
@Test
- public void testSortItems_retainOrder() {
- List<TestItem> initial = Arrays.asList(ITEM_2_1, ITEM_2_2, ITEM_1_2,
ITEM_1_1);
- List<TestItem> expected = Arrays.asList(ITEM_1_2, ITEM_1_1, ITEM_2_1,
ITEM_2_2);
-
- testedModel.setComparator(makeValueComparator());
+ public void testSortItems_noItems() {
+ SortedListModel<TestItem> testedModel = new
SortedListModel<TestItem>(makeValueComparator());
+ Collection<TestItem> initial = Collections.emptyList();
Collection<TestItem> sorted = testedModel.sortItems(initial);
+ assertNotSame(sorted, initial);
+ assertThat(sorted.isEmpty(), is(true));
+ }
+
+ @Test
+ public void testSortItems_valueComparator() {
+ SortedListModel<TestItem> testedModel = new
SortedListModel<TestItem>(makeValueComparator());
+ Collection<TestItem> initial = Arrays.asList(ITEM_2_1, ITEM_2_2,
ITEM_1_2, ITEM_1_1);
+ Collection<TestItem> expected = Arrays.asList(ITEM_1_2, ITEM_1_1,
ITEM_2_1, ITEM_2_2);
+ Collection<TestItem> sorted = testedModel.sortItems(initial);
+ assertNotSame(sorted, initial);
+ assertThat(sorted.isEmpty(), is(false));
+ assertThat(sorted.size(), is(initial.size()));
assertArrayEquals(sorted.toArray(), expected.toArray());
}
@Test
- public void testSortItems_switchComparator() {
- List<TestItem> initial = Arrays.asList(ITEM_2_1, ITEM_2_2, ITEM_1_2,
ITEM_1_1);
- List<TestItem> expected = Arrays.asList(ITEM_2_1, ITEM_1_1, ITEM_2_2,
ITEM_1_2);
+ public void testSortItems_saltComparator() {
+ SortedListModel<TestItem> testedModel = new
SortedListModel<TestItem>(makeSaltComparator());
+ Collection<TestItem> initial = Arrays.asList(ITEM_2_1, ITEM_2_2,
ITEM_1_2, ITEM_1_1);
+ Collection<TestItem> expected = Arrays.asList(ITEM_2_1, ITEM_1_1,
ITEM_2_2, ITEM_1_2);
+ Collection<TestItem> sorted = testedModel.sortItems(initial);
+ assertNotSame(sorted, initial);
+ assertThat(sorted.isEmpty(), is(false));
+ assertThat(sorted.size(), is(initial.size()));
+ assertArrayEquals(sorted.toArray(), expected.toArray());
+ }
+
+ @Test
+ public void testSetItems_nullItems() {
+ SortedListModel<TestItem> testedModel = new
SortedListModel<TestItem>(makeValueComparator());
+ testedModel.setItems(null);
+ assertNull(testedModel.getItems());
+ }
+
+ @Test
+ public void testSetItems_nullComparator() {
+ SortedListModel<TestItem> testedModel = new
SortedListModel<TestItem>();
+ Collection<TestItem> initial = Arrays.asList(ITEM_2_1, ITEM_2_2,
ITEM_1_2, ITEM_1_1);
+ testedModel.setItems(initial);
+ Collection<TestItem> sorted = testedModel.getItems();
+ assertSame(sorted, initial);
+ }
+
+ @Test
+ public void testSetItems_noItems() {
+ SortedListModel<TestItem> testedModel = new
SortedListModel<TestItem>(makeValueComparator());
+ Collection<TestItem> initial = Collections.emptyList();
+ testedModel.setItems(initial);
+ Collection<TestItem> sorted = testedModel.getItems();
+ assertNotSame(sorted, initial);
+ assertThat(sorted.isEmpty(), is(true));
+ }
+
+ @Test
+ public void testSetItems_valueThenSaltComparator() {
+ SortedListModel<TestItem> testedModel = new
SortedListModel<TestItem>(makeValueComparator());
+ Collection<TestItem> initial = Arrays.asList(ITEM_2_1, ITEM_2_2,
ITEM_1_2, ITEM_1_1);
+ Collection<TestItem> expected = Arrays.asList(ITEM_1_2, ITEM_1_1,
ITEM_2_1, ITEM_2_2);
+ testedModel.setItems(initial);
+ Collection<TestItem> sorted = testedModel.getItems();
+ assertNotSame(sorted, initial);
+ assertThat(sorted.isEmpty(), is(false));
+ assertThat(sorted.size(), is(initial.size()));
+ assertArrayEquals(sorted.toArray(), expected.toArray());
testedModel.setComparator(makeSaltComparator());
- Collection<TestItem> sorted = testedModel.sortItems(initial);
- assertArrayEquals(sorted.toArray(), expected.toArray());
+ Collection<TestItem> expected2 = Arrays.asList(ITEM_1_1, ITEM_2_1,
ITEM_1_2, ITEM_2_2);
+ Collection<TestItem> sorted2 = testedModel.getItems();
+ assertNotSame(sorted2, sorted);
+ assertThat(sorted2.isEmpty(), is(false));
+ assertThat(sorted2.size(), is(sorted.size()));
+ assertArrayEquals(sorted2.toArray(), expected2.toArray());
+ }
- expected = Arrays.asList(ITEM_1_1, ITEM_1_2, ITEM_2_1, ITEM_2_2);
-
+ @Test
+ public void testSetComparator_nullToValueComparator() {
+ SortedListModel<TestItem> testedModel = new
SortedListModel<TestItem>();
+ Collection<TestItem> initial = Arrays.asList(ITEM_2_1, ITEM_2_2,
ITEM_1_2, ITEM_1_1);
+ Collection<TestItem> expected = Arrays.asList(ITEM_1_2, ITEM_1_1,
ITEM_2_1, ITEM_2_2);
+ testedModel.setItems(initial);
testedModel.setComparator(makeValueComparator());
- sorted = testedModel.sortItems(sorted);
+ Collection<TestItem> sorted = testedModel.getItems();
+ assertNotSame(sorted, initial);
+ assertThat(sorted.isEmpty(), is(false));
+ assertThat(sorted.size(), is(initial.size()));
+ assertArrayEquals(sorted.toArray(), expected.toArray());
+ }
+
+ @Test
+ public void testSetComparator_valueToNullComparator() {
+ SortedListModel<TestItem> testedModel = new
SortedListModel<TestItem>(makeValueComparator());
+ Collection<TestItem> initial = Arrays.asList(ITEM_2_1, ITEM_2_2,
ITEM_1_2, ITEM_1_1);
+ Collection<TestItem> expected = Arrays.asList(ITEM_1_2, ITEM_1_1,
ITEM_2_1, ITEM_2_2);
+ testedModel.setItems(initial);
+ testedModel.setComparator(null);
+ Collection<TestItem> sorted = testedModel.getItems();
+ assertNotSame(sorted, initial);
+ assertThat(sorted.isEmpty(), is(false));
+ assertThat(sorted.size(), is(initial.size()));
+ assertArrayEquals(sorted.toArray(), expected.toArray());
+ }
+
+ @Test
+ public void testSetComparator_valueToSaltComparator() {
+ SortedListModel<TestItem> testedModel = new
SortedListModel<TestItem>(makeValueComparator());
+ Collection<TestItem> initial = Arrays.asList(ITEM_2_1, ITEM_2_2,
ITEM_1_2, ITEM_1_1);
+ Collection<TestItem> expected = Arrays.asList(ITEM_1_1, ITEM_2_1,
ITEM_1_2, ITEM_2_2);
+ testedModel.setItems(initial);
+ testedModel.setComparator(makeSaltComparator());
+ Collection<TestItem> sorted = testedModel.getItems();
+ assertNotSame(sorted, initial);
+ assertThat(sorted.isEmpty(), is(false));
+ assertThat(sorted.size(), is(initial.size()));
assertArrayEquals(sorted.toArray(), expected.toArray());
}
--
To view, visit http://gerrit.ovirt.org/31101
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id10b84c2fb476ed2240c33d72f6432335650df33
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vojtech Szocs <vsz...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
