changeset 1281:bbd5fd366c96

Sort identifiers list in PolicyEditor * NEWS: add note about sorting * netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java (addNewEntry): clear listModel before adding new identifiers from policyEditorController. Controller handles sorting results for us, so we just clear and repopulate the UI. * netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditorController.java (getIdentifiers): return SortedSet rather than Set * netx/net/sourceforge/jnlp/security/policyeditor/PolicyFileModel.java (getIdentifiers): likewise * netx/net/sourceforge/jnlp/security/policyeditor/PolicyIdentifier.java (compareTo, compareComparable): new methods for implementing Comparable (getPrincipals): return Set, not List * tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyIdentifierTest.java: new tests for PolicyIdentifier
author Andrew Azores <aazores@redhat.com>
date Wed, 19 Aug 2015 10:48:35 -0400
parents 09792ebffb27
children 18e401faa1b9
files ChangeLog NEWS netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditorController.java netx/net/sourceforge/jnlp/security/policyeditor/PolicyFileModel.java netx/net/sourceforge/jnlp/security/policyeditor/PolicyIdentifier.java tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyIdentifierTest.java
diffstat 7 files changed, 217 insertions(+), 10 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Thu Jul 30 13:49:33 2015 -0400
+++ b/ChangeLog	Wed Aug 19 10:48:35 2015 -0400
@@ -1,3 +1,21 @@
+2015-08-19  Andrew Azores  <aazores@redhat.com>
+
+	Sort identifiers list in PolicyEditor
+	* NEWS: add note about sorting
+	* netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
+	(addNewEntry): clear listModel before adding new identifiers from
+	policyEditorController. Controller handles sorting results for us, so we
+	just clear and repopulate the UI.
+	* netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditorController.java
+	(getIdentifiers): return SortedSet rather than Set
+	* netx/net/sourceforge/jnlp/security/policyeditor/PolicyFileModel.java
+	(getIdentifiers): likewise
+	* netx/net/sourceforge/jnlp/security/policyeditor/PolicyIdentifier.java
+	(compareTo, compareComparable): new methods for implementing Comparable
+	(getPrincipals): return Set, not List
+	* tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyIdentifierTest.java:
+	new tests for PolicyIdentifier
+
 2015-07-30  Andrew Azores  <aazores@redhat.com>
 
 	Add tests for PolicyEditor.getFilePathArgument
--- a/NEWS	Thu Jul 30 13:49:33 2015 -0400
+++ b/NEWS	Wed Aug 19 10:48:35 2015 -0400
@@ -29,6 +29,7 @@
   - fixed issues with -html shortcuts
   - fixed issue with -html receiving garbage in width and height
 * PolicyEditor
+  - Entry list is sorted, entries will appear with consistent ordering
   - file flag made to work when used standalone
   - file flag cannot be used in combination with main argument
   - defaultfile flag added
--- a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java	Thu Jul 30 13:49:33 2015 -0400
+++ b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java	Wed Aug 19 10:48:35 2015 -0400
@@ -840,10 +840,9 @@
         invokeRunnableOrEnqueueLater(new Runnable() {
             @Override
             public void run() {
+                listModel.clear();
                 for (final PolicyIdentifier identifier : policyEditorController.getIdentifiers()) {
-                    if (!listModel.contains(identifier)) {
-                        listModel.addElement(identifier);
-                    }
+                    listModel.addElement(identifier);
                 }
                 list.setSelectedValue(identifier, true);
                 updateCheckboxes(identifier);
--- a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditorController.java	Thu Jul 30 13:49:33 2015 -0400
+++ b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditorController.java	Wed Aug 19 10:48:35 2015 -0400
@@ -48,6 +48,8 @@
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
 
 import net.sourceforge.jnlp.util.logging.OutputController;
 import sun.security.provider.PolicyParser;
@@ -102,8 +104,8 @@
         policyFile.removeIdentifier(identifier);
     }
 
-    public Set<PolicyIdentifier> getIdentifiers() {
-        return new HashSet<>(policyFile.getIdentifiers());
+    public SortedSet<PolicyIdentifier> getIdentifiers() {
+        return new TreeSet<>(policyFile.getIdentifiers());
     }
 
     public Map<PolicyIdentifier, Map<PolicyEditorPermissions, Boolean>> getCopyOfPermissions() {
--- a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyFileModel.java	Thu Jul 30 13:49:33 2015 -0400
+++ b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyFileModel.java	Wed Aug 19 10:48:35 2015 -0400
@@ -48,6 +48,8 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
 
 import net.sourceforge.jnlp.util.FileUtils;
 import net.sourceforge.jnlp.util.MD5SumWatcher;
@@ -201,8 +203,8 @@
         return fileWatcher != null && fileWatcher.update();
     }
 
-    synchronized Set<PolicyIdentifier> getIdentifiers() {
-        return new HashSet<>(permissionsMap.keySet());
+    synchronized SortedSet<PolicyIdentifier> getIdentifiers() {
+        return new TreeSet<>(permissionsMap.keySet());
     }
 
     synchronized KeystoreInfo getKeystoreInfo() {
--- a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyIdentifier.java	Thu Jul 30 13:49:33 2015 -0400
+++ b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyIdentifier.java	Wed Aug 19 10:48:35 2015 -0400
@@ -43,10 +43,12 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Set;
 
 // http://docs.oracle.com/javase/7/docs/technotes/guides/security/PolicyFiles.html
-public class PolicyIdentifier implements Serializable {
+public class PolicyIdentifier implements Comparable<PolicyIdentifier>, Serializable {
 
     public static final PolicyIdentifier ALL_APPLETS_IDENTIFIER = new PolicyIdentifier(null, Collections.<PolicyParser.PrincipalEntry>emptySet(), null) {
         @Override
@@ -56,7 +58,7 @@
     };
 
     private final String signedBy;
-    private final List<PolicyParser.PrincipalEntry> principals = new ArrayList<>();
+    private final LinkedHashSet<PolicyParser.PrincipalEntry> principals = new LinkedHashSet<>();
     private final String codebase;
 
     public PolicyIdentifier(final String signedBy, final Collection<PolicyParser.PrincipalEntry> principals, final String codebase) {
@@ -77,7 +79,7 @@
         return signedBy;
     }
 
-    public List<PolicyParser.PrincipalEntry> getPrincipals() {
+    public Set<PolicyParser.PrincipalEntry> getPrincipals() {
         return principals;
     }
 
@@ -144,4 +146,39 @@
         result = 31 * result + (codebase.hashCode());
         return result;
     }
+
+    @Override
+    public int compareTo(PolicyIdentifier policyIdentifier) {
+        if (this.equals(ALL_APPLETS_IDENTIFIER) && policyIdentifier.equals(ALL_APPLETS_IDENTIFIER)) {
+            return 0;
+        } else if (this.equals(ALL_APPLETS_IDENTIFIER) && !policyIdentifier.equals(ALL_APPLETS_IDENTIFIER)) {
+            return -1;
+        } else if (!this.equals(ALL_APPLETS_IDENTIFIER) && policyIdentifier.equals(ALL_APPLETS_IDENTIFIER)) {
+            return 1;
+        }
+
+        final int codebaseComparison = compareComparable(this.getCodebase(), policyIdentifier.getCodebase());
+        if (codebaseComparison != 0) {
+            return codebaseComparison;
+        }
+
+        final int signedByComparison = compareComparable(this.getSignedBy(), policyIdentifier.getSignedBy());
+        if (signedByComparison != 0) {
+            return signedByComparison;
+        }
+
+        return Integer.compare(this.getPrincipals().hashCode(), policyIdentifier.getPrincipals().hashCode());
+    }
+
+    private static <T extends Comparable<T>> int compareComparable(T a, T b) {
+        if (a == null && b != null) {
+            return 1;
+        } else if (a != null && b == null) {
+            return -1;
+        } else if (a == b) {
+            return 0;
+        } else {
+            return a.compareTo(b);
+        }
+    }
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyIdentifierTest.java	Wed Aug 19 10:48:35 2015 -0400
@@ -0,0 +1,148 @@
+/*Copyright (C) 2015 Red Hat, Inc.
+
+This file is part of IcedTea.
+
+IcedTea is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License as published by
+the Free Software Foundation, version 2.
+
+IcedTea is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with IcedTea; see the file COPYING.  If not, write to
+the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+02110-1301 USA.
+
+Linking this library statically or dynamically with other modules is
+making a combined work based on this library.  Thus, the terms and
+conditions of the GNU General Public License cover the whole
+combination.
+
+As a special exception, the copyright holders of this library give you
+permission to link this library with independent modules to produce an
+executable, regardless of the license terms of these independent
+modules, and to copy and distribute the resulting executable under
+terms of your choice, provided that you also meet, for each linked
+independent module, the terms and conditions of the license of that
+module.  An independent module is a module which is not derived from
+or based on this library.  If you modify this library, you may extend
+this exception to your version of the library, but you are not
+obligated to do so.  If you do not wish to do so, delete this
+exception statement from your version.
+ */
+
+package net.sourceforge.jnlp.security.policyeditor;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import sun.security.provider.PolicyParser;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
+public class PolicyIdentifierTest {
+
+    private static final Set<PolicyParser.PrincipalEntry> principals = new HashSet<>(Arrays.asList(
+            new PolicyParser.PrincipalEntry("aa", "bb"),
+            new PolicyParser.PrincipalEntry("cc", "dd")
+    ));
+    public static final Set<PolicyParser.PrincipalEntry> EMPTY_SET = Collections.emptySet();
+
+    @Test
+    public void testEquals() {
+        PolicyIdentifier exampleIdentifier = new PolicyIdentifier(null, EMPTY_SET, "http://example.com");
+        PolicyIdentifier exampleIdentifier2 = new PolicyIdentifier(null, EMPTY_SET, "http://example.com");
+        assertTrue(exampleIdentifier.equals(exampleIdentifier2));
+        assertTrue(exampleIdentifier2.equals(exampleIdentifier));
+        assertFalse(exampleIdentifier.equals(null));
+        assertFalse(exampleIdentifier.equals(new Object()));
+        assertFalse(exampleIdentifier.equals(PolicyIdentifier.ALL_APPLETS_IDENTIFIER));
+    }
+
+    @Test
+    public void testHashCode() {
+        PolicyIdentifier exampleIdentifier = new PolicyIdentifier(null, EMPTY_SET, "http://example.com");
+        PolicyIdentifier exampleIdentifier2 = new PolicyIdentifier(null, EMPTY_SET, "http://example.com");
+        assertTrue(exampleIdentifier.hashCode() == exampleIdentifier2.hashCode());
+        assertTrue(exampleIdentifier2.hashCode() == exampleIdentifier.hashCode());
+        assertFalse(exampleIdentifier.hashCode() == PolicyIdentifier.ALL_APPLETS_IDENTIFIER.hashCode());
+    }
+
+    @Test
+    public void testCompareTo() {
+        PolicyIdentifier exampleIdentifier1 = createIdentifier("exampleIdentifier1", null, EMPTY_SET, "http://example.com");
+        PolicyIdentifier exampleIdentifier2 = createIdentifier("exampleIdentifier2", null, EMPTY_SET, "http://example.com2");
+        PolicyIdentifier exampleIdentifier3 = createIdentifier("exampleIdentifier3", "signedBy1", EMPTY_SET, "http://example.com");
+        PolicyIdentifier exampleIdentifier4 = createIdentifier("exampleIdentifier4", "signedBy2", EMPTY_SET, "http://example.com");
+        PolicyIdentifier exampleIdentifier5 = createIdentifier("exampleIdentifier5", "signedBy2", principals, "http://example.com");
+        assertLesser(PolicyIdentifier.ALL_APPLETS_IDENTIFIER, exampleIdentifier1);
+        assertLesser(PolicyIdentifier.ALL_APPLETS_IDENTIFIER, exampleIdentifier2);
+        assertLesser(PolicyIdentifier.ALL_APPLETS_IDENTIFIER, exampleIdentifier3);
+        assertLesser(PolicyIdentifier.ALL_APPLETS_IDENTIFIER, exampleIdentifier4);
+        assertTrue("ALL_APPLETS_IDENTIFIER should be equal to itself",
+                PolicyIdentifier.ALL_APPLETS_IDENTIFIER.compareTo(PolicyIdentifier.ALL_APPLETS_IDENTIFIER) == 0);
+
+        assertLesser(exampleIdentifier1, exampleIdentifier2);
+        assertLesser(exampleIdentifier3, exampleIdentifier2);
+        assertLesser(exampleIdentifier3, exampleIdentifier1);
+        assertLesser(exampleIdentifier3, exampleIdentifier4);
+        assertLesser(exampleIdentifier4, exampleIdentifier5);
+    }
+
+    @Test
+    public void testCompareToCodebases() {
+        PolicyIdentifier a = createIdentifier("a", null, EMPTY_SET, "a");
+        PolicyIdentifier aa = createIdentifier("aa", null, EMPTY_SET, "aa");
+        PolicyIdentifier aaa = createIdentifier("aaa", null, EMPTY_SET, "aaa");
+        PolicyIdentifier b = createIdentifier("b", null, EMPTY_SET, "b");
+        assertLesser(a, aa);
+        assertLesser(a, aaa);
+        assertLesser(aa, aaa);
+        assertLesser(a, b);
+        assertLesser(aa, b);
+        assertLesser(aaa, b);
+    }
+
+    @Test
+    public void testCompareToSignedBys() {
+        PolicyIdentifier a = createIdentifier("a", "a", EMPTY_SET, null);
+        PolicyIdentifier aa = createIdentifier("aa", "aa", EMPTY_SET, null);
+        PolicyIdentifier aaa = createIdentifier("aaa", "aaa", EMPTY_SET, null);
+        PolicyIdentifier b = createIdentifier("b", "b", EMPTY_SET, null);
+        assertLesser(a, aa);
+        assertLesser(a, aaa);
+        assertLesser(aa, aaa);
+        assertLesser(a, b);
+        assertLesser(aa, b);
+        assertLesser(aaa, b);
+    }
+
+    //@Test
+    public void testCompareToPrincipals() {
+        // compareTo on principals set depends on implementation of Set.hashCode(), there is no real meaningful ordering on this field
+    }
+
+    static void assertLesser(PolicyIdentifier lesser, PolicyIdentifier greater) {
+        String message = lesser.toString() + " should be less than " + greater.toString();
+        assertTrue(message, lesser.compareTo(greater) < 0);
+    }
+
+    static PolicyIdentifier createIdentifier(final String name, String signedBy, Set<PolicyParser.PrincipalEntry> principals, String codebase) {
+        return new PolicyIdentifier(signedBy, principals, codebase) {
+            @Override
+            public String toString() {
+                return name;
+            }
+        };
+    }
+
+}