BUG-1665: rework prefix assignment logic 42/10642/2
authorRobert Varga <rovarga@cisco.com>
Tue, 2 Sep 2014 16:08:52 +0000 (18:08 +0200)
committerRobert Varga <rovarga@cisco.com>
Tue, 2 Sep 2014 17:24:15 +0000 (19:24 +0200)
Expand the charset, but do not include xXmMlL in it, preventing a
possible issue. Also speed up the codec process by having things aligned
at 32, allowing us to use shift of 5.

Change-Id: I812b24afc00e3711ae38a69144d4b15fa3232253
Signed-off-by: Robert Varga <rovarga@cisco.com>
yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/xml/RandomPrefix.java
yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/codec/xml/XmlUtils.java
yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/codec/xml/RandomPrefixTest.java

index 42e38104e8dc9b2e8fe68a8bc50d65ef7c19947d..518e796808d9676519e201d89db4dfabc1e90140 100644 (file)
@@ -8,34 +8,37 @@
 package org.opendaylight.yangtools.yang.data.impl.codec.xml;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.BiMap;
 import com.google.common.collect.HashBiMap;
 
 import java.net.URI;
 import java.util.Map;
 
-import org.opendaylight.yangtools.yang.common.QName;
+import javax.xml.namespace.NamespaceContext;
 
-final class RandomPrefix {
-
-    public static final char STARTING_CHAR = 'a';
-    public static final int CHARACTER_RANGE = 26;
-    public static final int PREFIX_MAX_LENGTH = 4;
-
-    public static final int MAX_COUNTER_VALUE = (int) Math.pow(CHARACTER_RANGE, PREFIX_MAX_LENGTH);
-    private static final int STARTING_WITH_XML = decode("xml");
+class RandomPrefix {
+    // 32 characters, carefully chosen
+    private static final String LOOKUP = "abcdefghiknoprstABCDEFGHIKNOPRST";
+    private static final int MASK = 0x1f;
+    private static final int SHIFT = 5;
 
     private int counter = 0;
 
     // BiMap to make values lookup faster
     private final BiMap<URI, String> prefixes = HashBiMap.create();
+    private final NamespaceContext context;
 
-    Iterable<Map.Entry<URI, String>> getPrefixes() {
-        return prefixes.entrySet();
+    RandomPrefix() {
+        this.context = null;
     }
 
-    String encodeQName(final QName qname) {
-        return encodePrefix(qname.getNamespace()) + ':' + qname.getLocalName();
+    RandomPrefix(final NamespaceContext context) {
+        this.context = Preconditions.checkNotNull(context);
+    }
+
+    Iterable<Map.Entry<URI, String>> getPrefixes() {
+        return prefixes.entrySet();
     }
 
     String encodePrefix(final URI namespace) {
@@ -45,18 +48,6 @@ final class RandomPrefix {
         }
 
         do {
-            // Skip values starting with xml (Expecting only 4 chars max since division is calculated only once)
-            while (counter == STARTING_WITH_XML
-                    || counter / CHARACTER_RANGE == STARTING_WITH_XML) {
-                counter++;
-            }
-
-            // Reset in case of max prefix generated
-            if (counter >= MAX_COUNTER_VALUE) {
-                counter = 0;
-                prefixes.clear();
-            }
-
             prefix = encode(counter);
             counter++;
         } while (alreadyUsedPrefix(prefix));
@@ -66,30 +57,29 @@ final class RandomPrefix {
     }
 
     private boolean alreadyUsedPrefix(final String prefix) {
-        return prefixes.values().contains(prefix);
+        return context != null && context.getNamespaceURI(prefix) != null;
     }
 
     @VisibleForTesting
-    static int decode(final String s) {
-        int num = 0;
-        for (final char ch : s.toCharArray()) {
-            num *= CHARACTER_RANGE;
-            num += (ch - STARTING_CHAR);
+    static int decode(final String str) {
+        int ret = 0;
+        for (char c : str.toCharArray()) {
+            int idx = LOOKUP.indexOf(c);
+            Preconditions.checkArgument(idx != -1, "Invalid string %s", str);
+            ret = (ret << SHIFT) + idx;
         }
-        return num;
+
+        return ret;
     }
 
     @VisibleForTesting
     static String encode(int num) {
-        if (num == 0) {
-            return "a";
-        }
-
         final StringBuilder sb = new StringBuilder();
-        while (num != 0) {
-            sb.append(((char) (num % CHARACTER_RANGE + STARTING_CHAR)));
-            num /= CHARACTER_RANGE;
-        }
+
+        do {
+            sb.append(LOOKUP.charAt(num & MASK));
+            num >>>= SHIFT;
+        } while (num != 0);
 
         return sb.reverse().toString();
     }
index 7312c363829832466c593d488f6bac31f7356621..e6cfd58e7ddb7b4b43105190646fbcf62c2dd321 100644 (file)
@@ -8,7 +8,9 @@
 package org.opendaylight.yangtools.yang.data.impl.codec.xml;
 
 import java.util.Map;
+
 import javax.annotation.Nonnull;
+
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
@@ -44,16 +46,23 @@ public final class XmlUtils {
         StringBuilder textContent = new StringBuilder();
         for (PathArgument pathArgument : id.getPathArguments()) {
             textContent.append('/');
-            textContent.append(prefixes.encodeQName(pathArgument.getNodeType()));
+
+            final QName nt = pathArgument.getNodeType();
+            textContent.append(prefixes.encodePrefix(nt.getNamespace()));
+            textContent.append(':');
+            textContent.append(nt.getLocalName());
+
             if (pathArgument instanceof NodeIdentifierWithPredicates) {
                 Map<QName, Object> predicates = ((NodeIdentifierWithPredicates) pathArgument).getKeyValues();
 
                 for (Map.Entry<QName, Object> entry : predicates.entrySet()) {
-                    String predicateValue = String.valueOf(entry.getValue());
+                    final QName key = entry.getKey();
                     textContent.append('[');
-                    textContent.append(prefixes.encodeQName(entry.getKey()));
+                    textContent.append(prefixes.encodePrefix(key.getNamespace()));
+                    textContent.append(':');
+                    textContent.append(key.getLocalName());
                     textContent.append("='");
-                    textContent.append(predicateValue);
+                    textContent.append(String.valueOf(entry.getValue()));
                     textContent.append("']");
                 }
             } else if (pathArgument instanceof NodeWithValue) {
index e7b7f3015b4b4bdcf72b8a8f94ceaa9f601c55db..73e3ad6f25c686006a5e24d05cb4e3e310ca5c2c 100644 (file)
@@ -24,20 +24,21 @@ import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.common.QNameModule;
 
 public class RandomPrefixTest {
+    static final int MAX_COUNTER = 4000;
 
     @Test
     public void testEncodeDecode() throws Exception {
+
         final List<String> allGenerated = Lists.newArrayList();
-        for (int i = 0; i < RandomPrefix.MAX_COUNTER_VALUE; i++) {
+        for (int i = 0; i < MAX_COUNTER; i++) {
             final String encoded = RandomPrefix.encode(i);
             assertEquals(RandomPrefix.decode(encoded), i);
             allGenerated.add(encoded);
         }
 
-        assertEquals(allGenerated.size(), RandomPrefix.MAX_COUNTER_VALUE);
-        assertEquals("zzzz", allGenerated.get(RandomPrefix.MAX_COUNTER_VALUE - 1));
+        assertEquals(allGenerated.size(), MAX_COUNTER);
+        assertEquals("dPT", allGenerated.get(MAX_COUNTER - 1));
         assertEquals("a", allGenerated.get(0));
-        assertEquals("xml", allGenerated.get(RandomPrefix.decode("xml")));
         assertEquals(allGenerated.size(), Sets.newHashSet(allGenerated).size());
     }
 
@@ -46,22 +47,22 @@ public class RandomPrefixTest {
         final RandomPrefix a = new RandomPrefix();
 
         final List<String> allGenerated = Lists.newArrayList();
-        for (int i = 0; i < RandomPrefix.MAX_COUNTER_VALUE; i++) {
+        for (int i = 0; i < MAX_COUNTER; i++) {
             final String prefix = RandomPrefix.encode(i);
             final URI uri = new URI("localhost:" + prefix);
-            final QName qName = QName.create(QNameModule.create(uri, new Date()), prefix, "local-name");
+            final QName qName = QName.create(QNameModule.create(uri, new Date()), "local-name");
             allGenerated.add(a.encodePrefix(qName.getNamespace()));
         }
 
-        assertEquals(RandomPrefix.MAX_COUNTER_VALUE, allGenerated.size());
+        assertEquals(MAX_COUNTER, allGenerated.size());
         // We are generating MAX_COUNTER_VALUE + 27 prefixes total, so we should encounter a reset in prefix a start from 0 at some point
         // At the end, there should be only 27 values in RandomPrefix cache
-        assertEquals(27, Iterables.size(a.getPrefixes()));
+        assertEquals(MAX_COUNTER, Iterables.size(a.getPrefixes()));
         assertThat(allGenerated, CoreMatchers.not(CoreMatchers.hasItem("xml")));
         assertThat(allGenerated, CoreMatchers.not(CoreMatchers.hasItem("xmla")));
         assertThat(allGenerated, CoreMatchers.not(CoreMatchers.hasItem("xmlz")));
 
-        assertEquals(2, Iterables.frequency(allGenerated, "a"));
+        assertEquals(1, Iterables.frequency(allGenerated, "a"));
     }
 
     @Test