/hg/icedtea-web: CustomPolicyViewer method extraction and tests ...

aazores at icedtea.classpath.org aazores at icedtea.classpath.org
Wed Jun 11 20:00:08 UTC 2014


changeset 48debbfe8672 in /hg/icedtea-web
details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=48debbfe8672
author: Andrew Azores <aazores at redhat.com>
date: Wed Jun 11 15:59:54 2014 -0400

	CustomPolicyViewer method extraction and tests added

	2014-06-11  Andrew Azores  <aazores at redhat.com>

	    CustomPolicyViewer refactored - methods extracted for unit testing and no
	    longer holds PolicyFileModel reference, instead using parent
	    PolicyEditor's interface for interacting with the file model
	    * netx/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewer.java:
	    no longer holds PolicyFileModel reference, interacts through
	    PolicyEditor parent instead
	    (addButtonAction, removeButtonAction, closeButtonAction): use extracted methods
	    (addCustomPermission): extracted method, no longer adds visual duplicates
	    (removeCustomPermission, quit): extracted methods
	    (getCopyOfCustomPermissions): new method
	    (updateCustomPermissions): use parent rather than PolicyFileModel
	    * netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
	    (clearCustomPermissions): new method
	    * tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java
	    (testAddCustomPermission, testClearCustomPermissions): new tests
	    * tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewerTest.java:
	    new test class


diffstat:

 ChangeLog                                                                              |   20 +
 netx/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewer.java                |   65 +++-
 netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java                      |    6 +-
 tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewerTest.java |  132 ++++++++++
 tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java       |   19 +-
 5 files changed, 218 insertions(+), 24 deletions(-)

diffs (370 lines):

diff -r 0e97a940f5c4 -r 48debbfe8672 ChangeLog
--- a/ChangeLog	Fri Jun 06 14:35:41 2014 -0400
+++ b/ChangeLog	Wed Jun 11 15:59:54 2014 -0400
@@ -1,3 +1,23 @@
+2014-06-11  Andrew Azores  <aazores at redhat.com>
+
+	CustomPolicyViewer refactored - methods extracted for unit testing and no
+	longer holds PolicyFileModel reference, instead using parent
+	PolicyEditor's interface for interacting with the file model
+	* netx/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewer.java:
+	no longer holds PolicyFileModel reference, interacts through
+	PolicyEditor parent instead
+	(addButtonAction, removeButtonAction, closeButtonAction): use extracted methods
+	(addCustomPermission): extracted method, no longer adds visual duplicates
+	(removeCustomPermission, quit): extracted methods
+	(getCopyOfCustomPermissions): new method
+	(updateCustomPermissions): use parent rather than PolicyFileModel
+	* netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
+	(clearCustomPermissions): new method
+	* tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java
+	(testAddCustomPermission, testClearCustomPermissions): new tests
+	* tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewerTest.java:
+	new test class
+
 2014-06-06  Andrew Azores  <aazores at redhat.com>
 
 	* netx/net/sourceforge/jnlp/security/policyeditor/CustomPermission.java
diff -r 0e97a940f5c4 -r 48debbfe8672 netx/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewer.java
--- a/netx/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewer.java	Fri Jun 06 14:35:41 2014 -0400
+++ b/netx/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewer.java	Wed Jun 11 15:59:54 2014 -0400
@@ -46,6 +46,7 @@
 import java.awt.event.WindowEvent;
 import java.lang.ref.WeakReference;
 import java.util.Collection;
+import java.util.Objects;
 import java.util.TreeSet;
 
 import javax.swing.DefaultListModel;
@@ -75,7 +76,6 @@
     private final JLabel listLabel = new JLabel();
     private final ActionListener addButtonAction, removeButtonAction, closeButtonAction;
     private final WeakReference<CustomPolicyViewer> weakThis = new WeakReference<>(this);
-    private final PolicyFileModel policyFile;
     private final PolicyEditor parent;
     private final String codebase;
 
@@ -84,14 +84,15 @@
      * @param codebase the codebase for which these custom permissions are enabled
      * @param policyFile the PolicyFileModel
      */
-    public CustomPolicyViewer(final PolicyEditor parent, final String codebase, final PolicyFileModel policyFile) {
+    public CustomPolicyViewer(final PolicyEditor parent, final String codebase) {
         super();
+        Objects.requireNonNull(parent);
+        Objects.requireNonNull(codebase);
         this.parent = parent;
         this.codebase = codebase;
-        this.policyFile = policyFile;
         setLayout(new GridBagLayout());
         setTitle(R("PECPTitle"));
-        customPermissions.addAll(policyFile.getCopyOfCustomPermissions().get(codebase));
+        customPermissions.addAll(parent.getCustomPermissions(codebase));
         for (final CustomPermission perm : customPermissions) {
             listModel.addElement(perm);
         }
@@ -100,10 +101,11 @@
             @Override
             public void actionPerformed(final ActionEvent e) {
                 final String prefill = R("PECPType") + " " + R("PECPTarget") + " [" + R("PECPActions") + "]";
-                final String string = JOptionPane.showInputDialog(weakThis.get(), R("PECPPrompt"), prefill);
+                String string = JOptionPane.showInputDialog(weakThis.get(), R("PECPPrompt"), prefill);
                 if (string == null || string.isEmpty()) {
                     return;
                 }
+                string = string.replaceAll("\\s+", " ").trim();
                 final String[] parts = string.split(" ");
                 if (parts.length < 2) {
                     return;
@@ -116,11 +118,7 @@
                     actions = "";
                 }
                 final CustomPermission perm = new CustomPermission(type, target, actions);
-                if (perm != null) {
-                    customPermissions.add(perm);
-                    listModel.addElement(perm);
-                    updateCustomPermissions();
-                }
+                addCustomPermission(perm);
             }
         };
         addButton.setText(R("PECPAddButton"));
@@ -129,12 +127,11 @@
         removeButtonAction = new ActionListener() {
             @Override
             public void actionPerformed(final ActionEvent e) {
-                if (list.getSelectedValue() == null) {
+                final CustomPermission selected = list.getSelectedValue();
+                if (selected == null) {
                     return;
                 }
-                customPermissions.remove(list.getSelectedValue());
-                listModel.removeElement(list.getSelectedValue());
-                updateCustomPermissions();
+                removeCustomPermission(selected);
             }
         };
         removeButton.setText(R("PECPRemoveButton"));
@@ -143,9 +140,7 @@
         closeButtonAction = new ActionListener() {
             @Override
             public void actionPerformed(final ActionEvent e) {
-                weakThis.clear();
-                parent.customPolicyViewerClosing();
-                dispose();
+                quit();
             }
         };
         closeButton.setText(R("PECPCloseButton"));
@@ -168,9 +163,7 @@
         addWindowListener(new WindowAdapter() {
             @Override
             public void windowClosing(final WindowEvent e) {
-                weakThis.clear();
-                parent.customPolicyViewerClosing();
-                dispose();
+                quit();
             }
         });
     }
@@ -216,6 +209,12 @@
         setMinimumSize(getPreferredSize());
     }
 
+    private void quit() {
+        weakThis.clear();
+        parent.customPolicyViewerClosing();
+        dispose();
+    }
+
     /**
      * Update the custom permissions map. Used by the Custom Policy Viewer to update its parent
      * PolicyEditor to changes it has made
@@ -224,8 +223,30 @@
      */
     private void updateCustomPermissions() {
         parent.setChangesMade();
-        policyFile.clearCustomCodebase(codebase);
-        policyFile.addCustomPermissions(codebase, customPermissions);
+        parent.clearCustomPermissions(codebase);
+        for (final CustomPermission permission : customPermissions) {
+            parent.addCustomPermission(codebase, permission);
+        }
+    }
+
+    void addCustomPermission(final CustomPermission permission) {
+        Objects.requireNonNull(permission);
+        if (customPermissions.add(permission)) {
+            listModel.addElement(permission);
+            updateCustomPermissions();
+        }
+        list.setSelectedValue(permission, true);
+    }
+
+    void removeCustomPermission(final CustomPermission permission) {
+        Objects.requireNonNull(permission);
+        customPermissions.remove(permission);
+        listModel.removeElement(permission);
+        updateCustomPermissions();
+    }
+
+    Collection<CustomPermission> getCopyOfCustomPermissions() {
+        return new TreeSet<>(customPermissions);
     }
 
 }
diff -r 0e97a940f5c4 -r 48debbfe8672 netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
--- a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java	Fri Jun 06 14:35:41 2014 -0400
+++ b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java	Wed Jun 11 15:59:54 2014 -0400
@@ -382,7 +382,7 @@
                             return;
                         }
                         if (cpViewer == null) {
-                            cpViewer = new CustomPolicyViewer(PolicyEditor.this, codebase, policyFile);
+                            cpViewer = new CustomPolicyViewer(PolicyEditor.this, codebase);
                             cpViewer.setVisible(true);
                         } else {
                             cpViewer.toFront();
@@ -889,6 +889,10 @@
         return new HashSet<>(policyFile.getCopyOfCustomPermissions().get(codebase));
     }
 
+    public void clearCustomPermissions(final String codebase) {
+        policyFile.clearCustomCodebase(codebase);
+    }
+
     private void updateCheckboxes() {
         for (String codebase : policyFile.getCopyOfPermissions().keySet()) {
             updateCheckboxes(codebase);
diff -r 0e97a940f5c4 -r 48debbfe8672 tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewerTest.java
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/CustomPolicyViewerTest.java	Wed Jun 11 15:59:54 2014 -0400
@@ -0,0 +1,132 @@
+/*Copyright (C) 2014 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.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Collection;
+
+import net.sourceforge.jnlp.security.policyeditor.CustomPermission;
+import net.sourceforge.jnlp.security.policyeditor.CustomPolicyViewer;
+import net.sourceforge.jnlp.security.policyeditor.PolicyEditor;
+
+import org.junit.Before;
+import org.junit.Test;
+
+public class CustomPolicyViewerTest {
+
+    private CustomPolicyViewer viewer;
+    private static final String CODEBASE = "http://example.com";
+    private static final CustomPermission PERMISSION = new CustomPermission("java.lang.RuntimePermission", "createClassLoader", "");
+
+    @Before
+    public void setupViewer() {
+        viewer = new CustomPolicyViewer(new PolicyEditor(null), CODEBASE);
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testConstructorWithNullPolicyEditor() throws Exception {
+        new CustomPolicyViewer(null, CODEBASE);
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testConstructorWithNullCodebase() throws Exception {
+        new CustomPolicyViewer(new PolicyEditor(null), null);
+    }
+
+    @Test
+    public void testPermissionsSetInitiallyEmpty() throws Exception {
+        assertEquals("Permissions set should be empty", 0, viewer.getCopyOfCustomPermissions().size());
+    }
+
+    @Test
+    public void testAddCustomPermission() throws Exception {
+        viewer.addCustomPermission(PERMISSION);
+        assertEquals("Permissions set size mismatch", 1, viewer.getCopyOfCustomPermissions().size());
+        assertTrue("Permissions set should contain " + PERMISSION, viewer.getCopyOfCustomPermissions().contains(PERMISSION));
+    }
+
+    @Test
+    public void testAddCustomPermissionDuplicates() throws Exception {
+        viewer.addCustomPermission(PERMISSION);
+        assertEquals("Permissions set size mismatch", 1, viewer.getCopyOfCustomPermissions().size());
+        assertTrue("Permissions set should contain " + PERMISSION, viewer.getCopyOfCustomPermissions().contains(PERMISSION));
+        viewer.addCustomPermission(PERMISSION);
+        assertEquals("Permissions set size should not have changed", 1, viewer.getCopyOfCustomPermissions().size());
+        assertTrue("Permissions set should still contain " + PERMISSION, viewer.getCopyOfCustomPermissions().contains(PERMISSION));
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testAddCustomPermissionNull() throws Exception {
+        viewer.addCustomPermission(null);
+    }
+
+    @Test
+    public void testRemoveCustomPermission() throws Exception {
+        viewer.addCustomPermission(PERMISSION);
+        assertEquals("Permissions set size mismatch", 1, viewer.getCopyOfCustomPermissions().size());
+        assertTrue("Permissions set should contain " + PERMISSION, viewer.getCopyOfCustomPermissions().contains(PERMISSION));
+        viewer.removeCustomPermission(PERMISSION);
+        assertEquals("Permissions set should be empty", 0, viewer.getCopyOfCustomPermissions().size());
+        assertFalse("Permissions set should not contain " + PERMISSION, viewer.getCopyOfCustomPermissions().contains(PERMISSION));
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testRemoveCustomPermissionNull() throws Exception {
+        viewer.removeCustomPermission(null);
+    }
+
+    @Test
+    public void testGetCopyOfCustomPermissionsNotNull() throws Exception {
+        assertNotNull(viewer.getCopyOfCustomPermissions());
+    }
+
+    @Test
+    public void testGetCopyOfCustomPermissionsReturnsCopy() throws Exception {
+        final Collection<CustomPermission> permissions = viewer.getCopyOfCustomPermissions();
+        permissions.add(PERMISSION);
+        assertNotEquals("Sets should be distinct", viewer.getCopyOfCustomPermissions(), permissions);
+        assertNotEquals("Sizes should not match", viewer.getCopyOfCustomPermissions().size(), permissions.size());
+        assertTrue("Copy should contain " + PERMISSION, permissions.contains(PERMISSION));
+        assertFalse("Viewer should not contain " + PERMISSION, viewer.getCopyOfCustomPermissions().contains(PERMISSION));
+    }
+
+}
diff -r 0e97a940f5c4 -r 48debbfe8672 tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java
--- a/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java	Fri Jun 06 14:35:41 2014 -0400
+++ b/tests/netx/unit/net/sourceforge/jnlp/security/policyeditor/PolicyEditorTest.java	Wed Jun 11 15:59:54 2014 -0400
@@ -38,7 +38,6 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
@@ -142,6 +141,24 @@
     }
 
     @Test
+    public void testAddCustomPermission() throws Exception {
+        final String codebase = "http://example.com";
+        final CustomPermission customPermission = new CustomPermission("java.lang.RuntimePermission", "createClassLoader", "");
+        editor.addCustomPermission(codebase, customPermission);
+        assertTrue(editor.getCustomPermissions(codebase).contains(customPermission));
+    }
+
+    @Test
+    public void testClearCustomPermissions() throws Exception {
+        final String codebase = "http://example.com";
+        final CustomPermission customPermission = new CustomPermission("java.lang.RuntimePermission", "createClassLoader", "");
+        editor.addCustomPermission(codebase, customPermission);
+        assertTrue(editor.getCustomPermissions(codebase).contains(customPermission));
+        editor.clearCustomPermissions(codebase);
+        assertEquals(0, editor.getCustomPermissions(codebase).size());
+    }
+
+    @Test
     public void testReturnedCodebasesIsCopy() throws Exception {
         final Collection<String> original = editor.getCodebases();
         original.add("some invalid value");


More information about the distro-pkg-dev mailing list