You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ace.apache.org by ja...@apache.org on 2014/02/28 11:38:02 UTC
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
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("\\/", "/"))
- .append("-")
- .append(value.replaceAll("\\\\", "\\\\\\\\").replaceAll("-", "\\-").replaceAll("\\/", "/"));
+ .append(key.replaceAll("\\\\", "\\\\\\\\").replaceAll("-", "\\-").replaceAll("\\/", "/"))
+ .append("-")
+ .append(value.replaceAll("\\\\", "\\\\\\\\").replaceAll("-", "\\-").replaceAll("\\/", "/"));
// About the /: 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