ace-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject svn commit: r1572909 - in /ace/trunk/org.apache.ace.client.repository: src/org/apache/ace/client/repository/impl/RepositoryObjectImpl.java test/org/apache/ace/client/repository/impl/RepositoryObjectImplTest.java
Date Fri, 28 Feb 2014 10:38:02 GMT
Author: jawi
Date: Fri Feb 28 10:38:02 2014
New Revision: 1572909

URL: http://svn.apache.org/r1572909
Log:
ACE-463 - cache the merged keys of all attributes and tags.


Added:
    ace/trunk/org.apache.ace.client.repository/test/org/apache/ace/client/repository/impl/RepositoryObjectImplTest.java
  (with props)
Modified:
    ace/trunk/org.apache.ace.client.repository/src/org/apache/ace/client/repository/impl/RepositoryObjectImpl.java

Modified: ace/trunk/org.apache.ace.client.repository/src/org/apache/ace/client/repository/impl/RepositoryObjectImpl.java
URL: http://svn.apache.org/viewvc/ace/trunk/org.apache.ace.client.repository/src/org/apache/ace/client/repository/impl/RepositoryObjectImpl.java?rev=1572909&r1=1572908&r2=1572909&view=diff
==============================================================================
--- ace/trunk/org.apache.ace.client.repository/src/org/apache/ace/client/repository/impl/RepositoryObjectImpl.java
(original)
+++ ace/trunk/org.apache.ace.client.repository/src/org/apache/ace/client/repository/impl/RepositoryObjectImpl.java
Fri Feb 28 10:38:02 2014
@@ -25,16 +25,19 @@ import com.thoughtworks.xstream.io.Hiera
 
 /**
  * Represents a value-object as is part of the repository.<br>
- * It stores the 'member' values of the repository object, and allows putting tags on this
object by using <code>put()</code>
- * and <code>remove()</code>. It 'looks' like a dictionary to allow filtering
of it, using an ldap filter.
+ * It stores the 'member' values of the repository object, and allows putting tags on this
object by using
+ * <code>put()</code> and <code>remove()</code>. It 'looks' like
a dictionary to allow filtering of it, using an ldap
+ * filter.
  */
 public class RepositoryObjectImpl<T extends RepositoryObject> extends Dictionary<String,
Object> implements RepositoryObject, EventHandler {
     private final Map<String, String> m_attributes = new HashMap<String, String>();
     private final Map<String, String> m_tags = new HashMap<String, String>();
+    /** see ACE-463 */
+    private final Set<String> m_mergedAttrTags = new HashSet<String>();
     private final Map<Class, List<Association>> m_associations = new HashMap<Class,
List<Association>>();
     private final ChangeNotifier m_notifier;
     private final String m_xmlNode;
-    
+
     private volatile boolean m_deleted = false;
     private volatile boolean m_busy = false;
 
@@ -55,16 +58,18 @@ public class RepositoryObjectImpl<T exte
         m_xmlNode = xmlNode;
         if (attributes != null) {
             m_attributes.putAll(attributes);
+            m_mergedAttrTags.addAll(attributes.keySet());
         }
         if (tags != null) {
             m_tags.putAll(tags);
+            m_mergedAttrTags.addAll(tags.keySet());
         }
         if (notifier == null) {
             throw new IllegalArgumentException();
         }
         m_notifier = notifier;
     }
-    
+
     @Override
     public void notifyChanged() {
         notifyChanged(null);
@@ -89,8 +94,8 @@ public class RepositoryObjectImpl<T exte
     }
 
     /**
-     * Gets the object associated with this key. Will return null when the key is not used;
if the key is available for both the
-     * tags and the object's basic information, an array of two Strings will be returned.
+     * Gets the object associated with this key. Will return null when the key is not used;
if the key is available for
+     * both the tags and the object's basic information, an array of two Strings will be
returned.
      */
     @Override
     public Object get(Object key) {
@@ -116,7 +121,7 @@ public class RepositoryObjectImpl<T exte
     @Override
     public boolean isEmpty() {
         synchronized (m_attributes) {
-            return m_attributes.isEmpty() && m_tags.isEmpty();
+            return m_mergedAttrTags.isEmpty();
         }
     }
 
@@ -126,17 +131,15 @@ public class RepositoryObjectImpl<T exte
     @Override
     public Enumeration<String> keys() {
         synchronized (m_attributes) {
-            Set<String> keys = new HashSet<String>();
-            keys.addAll(m_attributes.keySet());
-            keys.addAll(m_tags.keySet());
-            return new KeysEnumeration(keys.iterator());
+            return new KeysEnumeration(m_mergedAttrTags.iterator());
         }
     }
 
     /**
      * Unsupported operation.
      * 
-     * @throws UnsupportedOperationException always
+     * @throws UnsupportedOperationException
+     *             always
      */
     @Override
     public Object put(String key, Object value) {
@@ -146,7 +149,8 @@ public class RepositoryObjectImpl<T exte
     /**
      * Unsupported operation.
      * 
-     * @throws UnsupportedOperationException always
+     * @throws UnsupportedOperationException
+     *             always
      */
     @Override
     public Object remove(Object key) {
@@ -159,10 +163,7 @@ public class RepositoryObjectImpl<T exte
     @Override
     public int size() {
         synchronized (m_attributes) {
-            Set<String> keys = new HashSet<String>();
-            keys.addAll(m_attributes.keySet());
-            keys.addAll(m_tags.keySet());
-            return keys.size();
+            return m_mergedAttrTags.size();
         }
     }
 
@@ -179,6 +180,8 @@ public class RepositoryObjectImpl<T exte
         synchronized (m_attributes) {
             ensureCurrent();
             result = m_attributes.get(key);
+            // ACE-463: keep a cached version of the merged attribute/tag keys...
+            m_mergedAttrTags.add(key);
             if (!value.equals(result)) {
                 result = m_attributes.put(key, value);
                 notifyChanged(null);
@@ -186,7 +189,7 @@ public class RepositoryObjectImpl<T exte
         }
         return result;
     }
-    
+
     @Override
     public String removeAttribute(String key) {
         for (String s : getDefiningKeys()) {
@@ -198,6 +201,10 @@ public class RepositoryObjectImpl<T exte
         synchronized (m_attributes) {
             ensureCurrent();
             result = m_attributes.get(key);
+            // ACE-463: keep a cached version of the merged attribute/tag keys...
+            if (!m_tags.containsKey(key)) {
+                m_mergedAttrTags.remove(key);
+            }
             if (result != null) {
                 result = m_attributes.remove(key);
                 notifyChanged(null);
@@ -214,6 +221,8 @@ public class RepositoryObjectImpl<T exte
         synchronized (m_attributes) {
             ensureCurrent();
             result = m_tags.get(key);
+            // ACE-463: keep a cached version of the merged attribute/tag keys...
+            m_mergedAttrTags.add(key);
             if (!value.equals(result)) {
                 result = m_tags.put(key, value);
                 notifyChanged(null);
@@ -221,13 +230,17 @@ public class RepositoryObjectImpl<T exte
         }
         return result;
     }
-    
+
     @Override
     public String removeTag(String key) {
         String result = null;
         synchronized (m_attributes) {
             ensureCurrent();
             result = m_tags.get(key);
+            // ACE-463: keep a cached version of the merged attribute/tag keys...
+            if (!m_attributes.containsKey(key)) {
+                m_mergedAttrTags.remove(key);
+            }
             if (result != null) {
                 result = m_tags.remove(key);
                 notifyChanged(null);
@@ -340,7 +353,7 @@ public class RepositoryObjectImpl<T exte
 
     @Override
     public boolean equals(Object o) {
-        synchronized(m_attributes) {
+        synchronized (m_attributes) {
             if ((o == null) || !(getClass().isInstance(o))) {
                 return false;
             }
@@ -368,13 +381,12 @@ public class RepositoryObjectImpl<T exte
     }
 
     /**
-     * Returns an array of keys which are considered to be defining for this object's
-     * attributes. The basic implementation just uses every key that is used; this
-     * function is intended to be overridden by deriving classes, providing a real
-     * set.
+     * Returns an array of keys which are considered to be defining for this object's attributes.
The basic
+     * implementation just uses every key that is used; this function is intended to be overridden
by deriving classes,
+     * providing a real set.
      *
-     * Note that the array returned from this function should be read from only;
-     * writing to it will change the state of the object.
+     * Note that the array returned from this function should be read from only; writing
to it will change the state of
+     * the object.
      */
     String[] getDefiningKeys() {
         return m_attributes.keySet().toArray(new String[m_attributes.size()]);
@@ -382,7 +394,7 @@ public class RepositoryObjectImpl<T exte
 
     @Override
     public int hashCode() {
-        synchronized(m_attributes) {
+        synchronized (m_attributes) {
             return m_attributes.hashCode();
         }
     }
@@ -398,20 +410,24 @@ public class RepositoryObjectImpl<T exte
     }
 
     /**
-     * This method is intended to be overridden by deriving classes, to read custom information
-     * from the XML representation of this object. This method should end with the writer
at
-     * the same 'level' as before, that is, using equally many moveDown() and moveUp() calls.
-     * @param reader A reader to read from the XML stream.
+     * This method is intended to be overridden by deriving classes, to read custom information
from the XML
+     * representation of this object. This method should end with the writer at the same
'level' as before, that is,
+     * using equally many moveDown() and moveUp() calls.
+     * 
+     * @param reader
+     *            A reader to read from the XML stream.
      */
     protected void readCustom(HierarchicalStreamReader reader) {
         // do nothing
     }
 
     /**
-     * This method is intended to be overridden by deriving classes, to write custom information
-     * to the XML representation of this object. This method should end with the writer at
-     * the same 'level' as before, that is, using equally many moveDown() and moveUp() calls.
-     * @param writer A writer to write to the XML stream.
+     * This method is intended to be overridden by deriving classes, to write custom information
to the XML
+     * representation of this object. This method should end with the writer at the same
'level' as before, that is,
+     * using equally many moveDown() and moveUp() calls.
+     * 
+     * @param writer
+     *            A writer to write to the XML stream.
      */
     protected void writeCustom(HierarchicalStreamWriter writer) {
         // do nothing
@@ -443,12 +459,14 @@ public class RepositoryObjectImpl<T exte
     }
 
     /**
-     * Helper function to check the existence of keys in a map. Each attribute is required
-     * to be non-empty.
-     * @param attributes A map of attribute-value combinations.
-     * @param mandatory An array of attributes which have to be present in the map.
+     * Helper function to check the existence of keys in a map. Each attribute is required
to be non-empty.
+     * 
+     * @param attributes
+     *            A map of attribute-value combinations.
+     * @param mandatory
+     *            An array of attributes which have to be present in the map.
      * @return <code>attributes</code> if this map meets the requirements. If
not, <code>IllegalArgumentException</code>
-     * will be thrown.
+     *         will be thrown.
      */
     static Map<String, String> checkAttributes(Map<String, String> attributes,
String... mandatory) {
         boolean[] booleans = new boolean[mandatory.length];
@@ -458,13 +476,16 @@ public class RepositoryObjectImpl<T exte
 
     /**
      * Helper function to check the existence of keys in a map.
-     * @param attributes A map of attribute-value combinations.
-     * @param mandatory An array of attributes which have to be present in the map.
-     * @param emptyAttributeAllowed An array of booleans, indicating which of the attributes
is allowed
-     * to be equal. Items in this array are matched by index on the elements in <code>mandatory</code>,
so
-     * the length should be equal.
+     * 
+     * @param attributes
+     *            A map of attribute-value combinations.
+     * @param mandatory
+     *            An array of attributes which have to be present in the map.
+     * @param emptyAttributeAllowed
+     *            An array of booleans, indicating which of the attributes is allowed to
be equal. Items in this array
+     *            are matched by index on the elements in <code>mandatory</code>,
so the length should be equal.
      * @return <code>attributes</code> if this map meets the requirements. If
not, <code>IllegalArgumentException</code>
-     * will be thrown.
+     *         will be thrown.
      */
     static Map<String, String> checkAttributes(Map<String, String> attributes,
String[] mandatory, boolean[] emptyAttributeAllowed) {
         if (!(mandatory.length == emptyAttributeAllowed.length)) {
@@ -508,8 +529,8 @@ public class RepositoryObjectImpl<T exte
         // setBusy should 'wait' until all altering operations have passed. To do so,
         // it gets the locks for the other 'set' objects. Once it has all these locks,
         // we are sure no thread is performing a set-action.
-        synchronized(m_associations) {
-            synchronized(m_attributes) {
+        synchronized (m_associations) {
+            synchronized (m_attributes) {
                 if (m_busy && !busy) {
                     m_associations.notifyAll();
                     m_attributes.notifyAll();
@@ -540,7 +561,7 @@ public class RepositoryObjectImpl<T exte
     }
 
     void ensureCurrent() {
-    	ensureNotDeleted();
+        ensureNotDeleted();
         ensureNotBusy();
     }
 
@@ -553,9 +574,9 @@ public class RepositoryObjectImpl<T exte
             String value = getAttribute(key);
             if (value != null) {
                 result.append("-")
-                .append(key.replaceAll("\\\\", "\\\\\\\\").replaceAll("-", "\\-").replaceAll("\\/",
"&#47"))
-                .append("-")
-                .append(value.replaceAll("\\\\", "\\\\\\\\").replaceAll("-", "\\-").replaceAll("\\/",
"&#47"));
+                    .append(key.replaceAll("\\\\", "\\\\\\\\").replaceAll("-", "\\-").replaceAll("\\/",
"&#47"))
+                    .append("-")
+                    .append(value.replaceAll("\\\\", "\\\\\\\\").replaceAll("-", "\\-").replaceAll("\\/",
"&#47"));
                 // About the &#47: the forward slash will be used by the preference admin,
but we don't want that.
             }
         }
@@ -564,9 +585,11 @@ public class RepositoryObjectImpl<T exte
     }
 
     /**
-     * Creates a filter string for use in associations, optionally with some
-     * additional properties. The basic implementation will use all <code>getDefiningKeys</code>.
-     * @param properties Properties indicating specifics of the filter to be created.
+     * Creates a filter string for use in associations, optionally with some additional properties.
The basic
+     * implementation will use all <code>getDefiningKeys</code>.
+     * 
+     * @param properties
+     *            Properties indicating specifics of the filter to be created.
      * @return A string representation of a filter, for use in <code>Association</code>s.
      */
     public String getAssociationFilter(Map<String, String> properties) {
@@ -582,9 +605,10 @@ public class RepositoryObjectImpl<T exte
     }
 
     /**
-     * Determines the cardinality of this endpoint of an association, given
-     * the passed properties.
-     * @param properties Properties indicating specifics of this endpoint.
+     * Determines the cardinality of this endpoint of an association, given the passed properties.
+     * 
+     * @param properties
+     *            Properties indicating specifics of this endpoint.
      * @return The necessary cardinality.
      */
     public int getCardinality(Map<String, String> properties) {
@@ -592,8 +616,8 @@ public class RepositoryObjectImpl<T exte
     }
 
     /**
-     * Returns a <code>Comparator</code> for this type of object, suitable
-     * for the endpoint properties that are passed.
+     * Returns a <code>Comparator</code> for this type of object, suitable for
the endpoint properties that are passed.
+     * 
      * @return A <code>Comparator</code> for this type of object
      */
     public Comparator<T> getComparator() {
@@ -633,7 +657,7 @@ public class RepositoryObjectImpl<T exte
             return get(m_iter.nextElement());
         }
     }
-    
+
     @Override
     public String toString() {
         StringBuilder attrs = new StringBuilder();

Added: ace/trunk/org.apache.ace.client.repository/test/org/apache/ace/client/repository/impl/RepositoryObjectImplTest.java
URL: http://svn.apache.org/viewvc/ace/trunk/org.apache.ace.client.repository/test/org/apache/ace/client/repository/impl/RepositoryObjectImplTest.java?rev=1572909&view=auto
==============================================================================
--- ace/trunk/org.apache.ace.client.repository/test/org/apache/ace/client/repository/impl/RepositoryObjectImplTest.java
(added)
+++ ace/trunk/org.apache.ace.client.repository/test/org/apache/ace/client/repository/impl/RepositoryObjectImplTest.java
Fri Feb 28 10:38:02 2014
@@ -0,0 +1,115 @@
+/*
+ * 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.ace.client.repository.impl;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.ace.client.repository.RepositoryObject;
+import org.mockito.Mockito;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+/**
+ * Test cases for {@link RepositoryObjectImpl}.
+ */
+public class RepositoryObjectImplTest {
+    private static final String XML_NODE = "dummy";
+
+    private ChangeNotifier m_notifier;
+
+    @BeforeMethod(alwaysRun = true)
+    public void setUp() {
+        m_notifier = Mockito.mock(ChangeNotifier.class);
+    }
+
+    /**
+     * Test case for ACE-463.
+     */
+    @Test
+    public void testKeysOk() throws Exception {
+        Map<String, String> attrs = createMap("common", "value1", "attr1", "value2",
"attr2", "value3");
+        Map<String, String> tags = createMap("common", "value1", "tag1", "value2",
"tag2", "value3");
+
+        RepositoryObjectImpl<RepositoryObject> obj = createRepoObject(attrs, tags);
+        // Initially, all keys should be present...
+        assertKeysContain(obj.keys(), "common", "attr1", "attr2", "tag1", "tag2");
+        assertEquals(5, obj.size());
+
+        // Remove a tag...
+        obj.removeTag("tag1");
+        // Check remaining keys...
+        assertKeysContain(obj.keys(), "common", "attr1", "attr2", "tag2");
+        assertEquals(4, obj.size());
+
+        // Remove an attribute...
+        obj.removeAttribute("attr2");
+        // Check remaining keys...
+        assertKeysContain(obj.keys(), "common", "attr1", "tag2");
+        assertEquals(3, obj.size());
+
+        // Remove a "shared" attribute, should cause it to remain...
+        obj.removeAttribute("common");
+        // Check remaining keys...
+        assertKeysContain(obj.keys(), "common", "attr1", "tag2");
+        assertEquals(3, obj.size());
+
+        // Remove a "shared" tag, should cause it to be removed...
+        obj.removeTag("common");
+        // Check remaining keys...
+        assertKeysContain(obj.keys(), "attr1", "tag2");
+        assertEquals(2, obj.size());
+    }
+
+    private void assertKeysContain(Enumeration<String> enumerator, String... expectedKeys)
{
+        List<String> expected = new ArrayList<String>(Arrays.asList(expectedKeys));
+
+        while (enumerator.hasMoreElements()) {
+            String key = enumerator.nextElement();
+            assertTrue(expected.remove(key), "Key " + key + " not removed!");
+        }
+
+        assertTrue(expected.isEmpty(), "Not all keys were seen in enumerator: " + expected);
+    }
+
+    private Map<String, String> createMap(String... entries) {
+        Map<String, String> result = new HashMap<String, String>();
+        for (int i = 0; i < entries.length; i += 2) {
+            result.put(entries[i], entries[i + 1]);
+        }
+        return result;
+    }
+
+    private RepositoryObjectImpl<RepositoryObject> createRepoObject(Map<String,
String> attrs, Map<String, String> tags) {
+        return new RepositoryObjectImpl<RepositoryObject>(attrs, tags, m_notifier,
XML_NODE) {
+            @Override
+            String[] getDefiningKeys() {
+                // Causes that we always can remove any attribute...
+                return new String[0];
+            }
+        };
+    }
+}

Propchange: ace/trunk/org.apache.ace.client.repository/test/org/apache/ace/client/repository/impl/RepositoryObjectImplTest.java
------------------------------------------------------------------------------
    svn:eol-style = native



Mime
View raw message