You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2017/03/02 18:15:06 UTC

svn commit: r1785182 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/spi/security/ test/java/org/apache/jackrabbit/oak/spi/security/

Author: angela
Date: Thu Mar  2 18:15:06 2017
New Revision: 1785182

URL: http://svn.apache.org/viewvc?rev=1785182&view=rev
Log:
OAK-5881 : Reduce code duplication in ConfigurationParameters.Milliseconds.of 
OAK-5793 : Improve coverage for security code in oak-core (wip)

Added:
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/CompositeContextTest.java
Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParameters.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/CompositeConfigurationTest.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParametersTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParameters.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParameters.java?rev=1785182&r1=1785181&r2=1785182&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParameters.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParameters.java Thu Mar  2 18:15:06 2017
@@ -505,28 +505,8 @@ public final class ConfigurationParamete
             if (str == null) {
                 return defaultValue;
             }
-            Matcher m = pattern.matcher(str);
-            long current = -1;
-            while (m.find()) {
-                String number = m.group(1);
-                String decimal = m.group(2);
-                if (decimal != null) {
-                    number += decimal;
-                }
-                String unit = m.group(3);
-                double value = Double.valueOf(number);
-                if ("s".equals(unit)) {
-                    value *= 1000.0;
-                } else if ("m".equals(unit)) {
-                    value *= 60 * 1000.0;
-                } else if ("h".equals(unit)) {
-                    value *= 60 * 60 * 1000.0;
-                } else if ("d".equals(unit)) {
-                    value *= 24 * 60 * 60 * 1000.0;
-                }
-                current += value;
-            }
-            return current < 0 ? defaultValue : new Milliseconds(current + 1);
+            Milliseconds ms = of(str);
+            return (ms == null) ? defaultValue : ms;
         }
 
         @Override

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/CompositeConfigurationTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/CompositeConfigurationTest.java?rev=1785182&r1=1785181&r2=1785182&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/CompositeConfigurationTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/CompositeConfigurationTest.java Thu Mar  2 18:15:06 2017
@@ -16,9 +16,7 @@
  */
 package org.apache.jackrabbit.oak.spi.security;
 
-import java.lang.reflect.Field;
 import java.util.List;
-
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
@@ -32,7 +30,6 @@ import org.osgi.framework.Constants;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
@@ -179,54 +176,6 @@ public class CompositeConfigurationTest
         assertEquals(def, configurations.iterator().next());
     }
 
-    @Test
-    public void testGetContext() throws Exception {
-        Class cls = Class.forName(CompositeConfiguration.class.getName() + "$CompositeContext");
-        Field def = cls.getDeclaredField("defaultCtx");
-        def.setAccessible(true);
-
-        Field delegatees = cls.getDeclaredField("delegatees");
-        delegatees.setAccessible(true);
-
-        Context ctx = compositeConfiguration.getContext();
-        assertSame(cls, ctx.getClass());
-        assertNull(delegatees.get(ctx));
-        assertSame(Context.DEFAULT, def.get(ctx));
-
-        SecurityConfiguration sc = new TestConfiguration();
-        setDefault(sc);
-        ctx = compositeConfiguration.getContext();
-        assertNull(delegatees.get(ctx));
-        assertSame(sc.getContext(), def.get(ctx));
-        assertSame(cls, ctx.getClass());
-
-        addConfiguration(sc);
-        ctx = compositeConfiguration.getContext();
-        assertNotSame(sc.getContext(), ctx);
-        assertEquals(1, ((Context[]) delegatees.get(ctx)).length);
-
-        // add configuration that has DEFAULT ctx -> must not be added
-        SecurityConfiguration defConfig = new SecurityConfiguration.Default();
-        addConfiguration(defConfig);
-        assertEquals(1, ((Context[]) delegatees.get(compositeConfiguration.getContext())).length);
-
-        // add same test configuration again -> no duplicate entries
-        addConfiguration(sc);
-        assertEquals(1, ((Context[]) delegatees.get(compositeConfiguration.getContext())).length);
-
-        SecurityConfiguration sc2 = new TestConfiguration();
-        addConfiguration(sc2);
-        assertEquals(2, ((Context[]) delegatees.get(compositeConfiguration.getContext())).length);
-
-        removeConfiguration(sc2);
-        assertEquals(1, ((Context[]) delegatees.get(compositeConfiguration.getContext())).length);
-
-        removeConfiguration(sc);
-        removeConfiguration(sc);
-        removeConfiguration(defConfig);
-        assertNull(delegatees.get(compositeConfiguration.getContext()));
-    }
-
     @Test(expected = IllegalStateException.class)
     public void testGetSecurityProviderNotInitialized() {
         CompositeConfiguration cc = new CompositeConfiguration("name") {};
@@ -261,18 +210,4 @@ public class CompositeConfigurationTest
 
         assertEquals(1, compositeConfiguration.getProtectedItemImporters().size());
     }
-
-    private static final class TestConfiguration extends SecurityConfiguration.Default {
-
-        private final Context ctx = new TestContext();
-        @Nonnull
-        @Override
-        public Context getContext() {
-            return ctx;
-        }
-    }
-
-    private static final class TestContext extends Context.Default {
-
-    }
 }
\ No newline at end of file

Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/CompositeContextTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/CompositeContextTest.java?rev=1785182&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/CompositeContextTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/CompositeContextTest.java Thu Mar  2 18:15:06 2017
@@ -0,0 +1,258 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.spi.security;
+
+import java.lang.reflect.Field;
+
+import javax.annotation.Nonnull;
+
+import org.apache.jackrabbit.oak.api.PropertyState;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.plugins.tree.TreeLocation;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
+public class CompositeContextTest extends AbstractCompositeConfigurationTest {
+
+    @Before
+    public void before() throws Exception {
+        compositeConfiguration = new CompositeConfiguration("test", Mockito.mock(SecurityProvider.class)) {};
+    }
+
+    @Test
+    public void testGetContext() throws Exception {
+        Class cls = Class.forName(CompositeConfiguration.class.getName() + "$CompositeContext");
+        Field def = cls.getDeclaredField("defaultCtx");
+        def.setAccessible(true);
+
+        Field delegatees = cls.getDeclaredField("delegatees");
+        delegatees.setAccessible(true);
+
+        Context ctx = compositeConfiguration.getContext();
+        assertSame(cls, ctx.getClass());
+        assertNull(delegatees.get(ctx));
+        assertSame(Context.DEFAULT, def.get(ctx));
+
+        SecurityConfiguration sc = new TestConfiguration();
+        setDefault(sc);
+        ctx = compositeConfiguration.getContext();
+        assertNull(delegatees.get(ctx));
+        assertSame(sc.getContext(), def.get(ctx));
+        assertSame(cls, ctx.getClass());
+
+        addConfiguration(sc);
+        ctx = compositeConfiguration.getContext();
+        assertNotSame(sc.getContext(), ctx);
+        assertEquals(1, ((Context[]) delegatees.get(ctx)).length);
+
+        // add configuration that has DEFAULT ctx -> must not be added
+        SecurityConfiguration defConfig = new SecurityConfiguration.Default();
+        addConfiguration(defConfig);
+        assertEquals(1, ((Context[]) delegatees.get(compositeConfiguration.getContext())).length);
+
+        // add same test configuration again -> no duplicate entries
+        addConfiguration(sc);
+        assertEquals(1, ((Context[]) delegatees.get(compositeConfiguration.getContext())).length);
+
+        SecurityConfiguration sc2 = new TestConfiguration();
+        addConfiguration(sc2);
+        assertEquals(2, ((Context[]) delegatees.get(compositeConfiguration.getContext())).length);
+
+        removeConfiguration(sc2);
+        assertEquals(1, ((Context[]) delegatees.get(compositeConfiguration.getContext())).length);
+
+        removeConfiguration(sc);
+        removeConfiguration(sc);
+        removeConfiguration(defConfig);
+        assertNull(delegatees.get(compositeConfiguration.getContext()));
+    }
+
+    @Test
+    public void testEmpty() {
+        Context ctx = compositeConfiguration.getContext();
+
+        assertNotNull(ctx);
+
+        Tree tree = Mockito.mock(Tree.class);
+        assertFalse(ctx.definesContextRoot(tree));
+        assertFalse(ctx.definesInternal(tree));
+        assertFalse(ctx.definesTree(tree));
+        assertFalse(ctx.definesProperty(tree, Mockito.mock(PropertyState.class)));
+        assertFalse(ctx.definesLocation(TreeLocation.create(tree)));
+    }
+
+    @Test
+    public void testDefinesProperty() {
+        TestConfiguration testConfig = new TestConfiguration(true);
+        addConfiguration(testConfig);
+
+        assertTrue(compositeConfiguration.getContext().definesProperty(Mockito.mock(Tree.class), Mockito.mock(PropertyState.class)));
+        assertEquals("definesProperty", testConfig.ctx.method);
+    }
+
+    @Test
+    public void testDefinesProperty2() {
+        TestConfiguration testConfig = new TestConfiguration(false);
+        addConfiguration(testConfig);
+
+        assertFalse(compositeConfiguration.getContext().definesProperty(Mockito.mock(Tree.class), Mockito.mock(PropertyState.class)));
+        assertEquals("definesProperty", testConfig.ctx.method);
+    }
+
+    @Test
+    public void testDefinesContextRoot() {
+        TestConfiguration testConfig = new TestConfiguration(true);
+        addConfiguration(testConfig);
+
+        assertTrue(compositeConfiguration.getContext().definesContextRoot(Mockito.mock(Tree.class)));
+        assertEquals("definesContextRoot", testConfig.ctx.method);
+    }
+
+    @Test
+    public void testDefinesContextRoot2() {
+        TestConfiguration testConfig = new TestConfiguration(false);
+        addConfiguration(testConfig);
+
+        assertFalse(compositeConfiguration.getContext().definesContextRoot(Mockito.mock(Tree.class)));
+        assertEquals("definesContextRoot", testConfig.ctx.method);
+    }
+
+    @Test
+    public void testDefinesTree() {
+        TestConfiguration testConfig = new TestConfiguration(true);
+        addConfiguration(testConfig);
+
+        assertTrue(compositeConfiguration.getContext().definesTree(Mockito.mock(Tree.class)));
+        assertEquals("definesTree", testConfig.ctx.method);
+    }
+
+    @Test
+    public void testDefinesTree2() {
+        TestConfiguration testConfig = new TestConfiguration(false);
+        addConfiguration(testConfig);
+
+        assertFalse(compositeConfiguration.getContext().definesTree(Mockito.mock(Tree.class)));
+        assertEquals("definesTree", testConfig.ctx.method);
+    }
+
+    @Test
+    public void testDefinesLocation() {
+        TestConfiguration testConfig = new TestConfiguration(true);
+        addConfiguration(testConfig);
+
+        assertTrue(compositeConfiguration.getContext().definesLocation(TreeLocation.create(Mockito.mock(Tree.class))));
+        assertEquals("definesLocation", testConfig.ctx.method);
+    }
+
+    @Test
+    public void testDefinesLocation2() {
+        TestConfiguration testConfig = new TestConfiguration(false);
+        addConfiguration(testConfig);
+
+        assertFalse(compositeConfiguration.getContext().definesLocation(TreeLocation.create(Mockito.mock(Tree.class))));
+        assertEquals("definesLocation", testConfig.ctx.method);
+    }
+
+    @Test
+    public void testDefinesInternal() {
+        TestConfiguration testConfig = new TestConfiguration(true);
+        addConfiguration(testConfig);
+
+        assertTrue(compositeConfiguration.getContext().definesInternal(Mockito.mock(Tree.class)));
+        assertEquals("definesInternal", testConfig.ctx.method);
+    }
+
+
+    @Test
+    public void testDefinesInternal2() {
+        TestConfiguration testConfig = new TestConfiguration(false);
+        addConfiguration(testConfig);
+
+        assertFalse(compositeConfiguration.getContext().definesInternal(Mockito.mock(Tree.class)));
+        assertEquals("definesInternal", testConfig.ctx.method);
+    }
+
+
+    private static final class TestConfiguration extends SecurityConfiguration.Default {
+
+        private final TestContext ctx;
+
+        private TestConfiguration() {
+            this(false);
+        }
+
+        private TestConfiguration(boolean returnValue) {
+            this.ctx = new TestContext(returnValue);
+        }
+
+        @Nonnull
+        @Override
+        public Context getContext() {
+            return ctx;
+        }
+    }
+
+    private static final class TestContext extends Context.Default {
+
+        private String method;
+
+        private final boolean returnValue;
+
+        private TestContext(boolean returnValue) {
+            this.returnValue = returnValue;
+        }
+
+        @Override
+        public boolean definesProperty(@Nonnull Tree parent, @Nonnull PropertyState property) {
+            method = "definesProperty";
+            return returnValue;
+        }
+
+        @Override
+        public boolean definesContextRoot(@Nonnull Tree tree) {
+            method = "definesContextRoot";
+            return returnValue;
+        }
+
+        @Override
+        public boolean definesTree(@Nonnull Tree tree) {
+            method = "definesTree";
+            return returnValue;
+        }
+
+        @Override
+        public boolean definesLocation(@Nonnull TreeLocation location) {
+            method = "definesLocation";
+            return returnValue;
+        }
+
+        @Override
+        public boolean definesInternal(@Nonnull Tree tree) {
+            method = "definesInternal";
+            return returnValue;
+        }
+    }
+}
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParametersTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParametersTest.java?rev=1785182&r1=1785181&r2=1785182&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParametersTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/ConfigurationParametersTest.java Thu Mar  2 18:15:06 2017
@@ -20,13 +20,16 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Collections;
+import java.util.Dictionary;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.Properties;
 import java.util.Set;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterators;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -78,6 +81,41 @@ public class ConfigurationParametersTest
     }
 
     @Test
+    public void testCreationFromEmptyProperties() {
+        ConfigurationParameters cp = ConfigurationParameters.of(new Properties());
+        assertSame(ConfigurationParameters.EMPTY, cp);
+    }
+
+    @Test
+    public void testCreationFromProperties() {
+        Properties properties = new Properties();
+        properties.put("a", "b");
+
+        ConfigurationParameters cp = ConfigurationParameters.of(properties);
+        assertEquals(ImmutableSet.copyOf(properties.keySet()), ImmutableSet.copyOf(cp.keySet()));
+        assertEquals(ImmutableSet.copyOf(properties.values()), ImmutableSet.copyOf(cp.values()));
+
+    }
+
+    @Test
+    public void testCreationFromEmptyDictionary() {
+        Dictionary dict = new Properties();
+        ConfigurationParameters cp = ConfigurationParameters.of(dict);
+        assertSame(ConfigurationParameters.EMPTY, cp);
+    }
+
+    @Test
+    public void testCreationFromDictionary() {
+        Dictionary dict = new Properties();
+        dict.put("a", "b");
+
+        ConfigurationParameters cp = ConfigurationParameters.of(dict);
+        assertEquals(ImmutableSet.copyOf(Iterators.forEnumeration(dict.keys())), ImmutableSet.copyOf(cp.keySet()));
+        assertEquals(ImmutableSet.copyOf(Iterators.forEnumeration(dict.elements())), ImmutableSet.copyOf(cp.values()));
+
+    }
+
+    @Test
     public void testContains() {
         ConfigurationParameters params = ConfigurationParameters.EMPTY;
         assertFalse(params.contains("some"));
@@ -438,6 +476,41 @@ public class ConfigurationParametersTest
         assertEquals(36 * 60 * 60 * 1000 + 60 * 60 * 1000 + 90 * 1000, ConfigurationParameters.Milliseconds.of("1.5d1h1m30s").value);
     }
 
+    @Test
+    public void testDurationParserWithDefault() {
+        ConfigurationParameters.Milliseconds defValue = ConfigurationParameters.Milliseconds.FOREVER;
+        assertEquals(defValue, ConfigurationParameters.Milliseconds.of("", defValue));
+        assertEquals(defValue, ConfigurationParameters.Milliseconds.of(null, defValue));
+        assertEquals(1, ConfigurationParameters.Milliseconds.of("1", defValue).value);
+        assertEquals(1, ConfigurationParameters.Milliseconds.of("1ms", defValue).value);
+        assertEquals(1, ConfigurationParameters.Milliseconds.of("  1ms", defValue).value);
+        assertEquals(1, ConfigurationParameters.Milliseconds.of("  1ms   ", defValue).value);
+        assertEquals(1, ConfigurationParameters.Milliseconds.of("  1ms  foobar", defValue).value);
+        assertEquals(1000, ConfigurationParameters.Milliseconds.of("1s", defValue).value);
+        assertEquals(1500, ConfigurationParameters.Milliseconds.of("1.5s", defValue).value);
+        assertEquals(1500, ConfigurationParameters.Milliseconds.of("1s 500ms", defValue).value);
+        assertEquals(60 * 1000, ConfigurationParameters.Milliseconds.of("1m", defValue).value);
+        assertEquals(90 * 1000, ConfigurationParameters.Milliseconds.of("1m30s", defValue).value);
+        assertEquals(60 * 60 * 1000 + 90 * 1000, ConfigurationParameters.Milliseconds.of("1h1m30s", defValue).value);
+        assertEquals(36 * 60 * 60 * 1000 + 60 * 60 * 1000 + 90 * 1000, ConfigurationParameters.Milliseconds.of("1.5d1h1m30s", defValue).value);
+    }
+
+    @Test
+    public void testNullMilliseconds() {
+        assertSame(ConfigurationParameters.Milliseconds.NULL, ConfigurationParameters.Milliseconds.of(0));
+    }
+
+    @Test
+    public void testForeverMilliseconds() {
+        assertSame(ConfigurationParameters.Milliseconds.FOREVER, ConfigurationParameters.Milliseconds.of(Long.MAX_VALUE));
+    }
+
+    @Test
+    public void testNeverMilliseconds() {
+        assertSame(ConfigurationParameters.Milliseconds.NEVER, ConfigurationParameters.Milliseconds.of(Long.MIN_VALUE));
+        assertSame(ConfigurationParameters.Milliseconds.NEVER, ConfigurationParameters.Milliseconds.of(-1));
+    }
+
     private class TestObject {
 
         private final String name;