Mailing-List: contact commits-help@commonsrdf.incubator.apache.org;
 run by ezmlm
Precedence: bulk
Reply-To: dev@commonsrdf.incubator.apache.org
Received-SPF: pass (nike.apache.org: domain of root@apache.org designates
 54.76.25.247 as permitted sender)
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: stain@apache.org
To: commits@commonsrdf.incubator.apache.org
Date: Wed, 29 Apr 2015 10:00:25 -0000
Message-Id: <0313764916064705b1cb61d8a2e9f8e4@git.apache.org>
Subject: [01/13] incubator-commonsrdf git commit: COMMONSRDF-6 Simplify
 internalIdentifier salts

Repository: incubator-commonsrdf
Updated Branches:
  refs/heads/master 299b5d984 -> 48dc06780


COMMONSRDF-6 Simplify internalIdentifier salts

BlankNodeImpl constructor take a UUID as salt, which is created for each
SimpleRDFTermFactory instance.  SimpleRDFTermFactory keeps a random
UUIDv4 per instance for this purpose.

(This may not be effective if loads of factories are made, but this is
the simple implementation which is not required to be pre-emptively
efficient.)

Constructor BlankNodeImpl() without arguments still uses an
internal AtomicLong, now combined with an internal UUID salt.
This could in theory just do

    this.id = UUID.randomUUID().toString()

and not keep any counter, but it is much more conceivable that say
10.000 BlankNodeImpls will be made, than 10.000 SimpleRDFTermFactories.
I did not do any performance testing, but kept the AtomicLong mechanism
as I didn't feel a need to change it now.


Project: http://git-wip-us.apache.org/repos/asf/incubator-commonsrdf/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-commonsrdf/commit/be2e0799
Tree: http://git-wip-us.apache.org/repos/asf/incubator-commonsrdf/tree/be2e0799
Diff: http://git-wip-us.apache.org/repos/asf/incubator-commonsrdf/diff/be2e0799

Branch: refs/heads/master
Commit: be2e079929d8b64ec954b4098c8df63fe7ccc40e
Parents: 5ec5230
Author: Stian Soiland-Reyes <stain@apache.org>
Authored: Thu Apr 23 01:35:01 2015 +0100
Committer: Stian Soiland-Reyes <stain@apache.org>
Committed: Fri Apr 24 09:44:52 2015 +0100

----------------------------------------------------------------------
 .../commons/rdf/simple/BlankNodeImpl.java       | 38 +++++++++++++-------
 .../rdf/simple/SimpleRDFTermFactory.java        | 18 ++++++++--
 .../commons/rdf/simple/BlankNodeImplTest.java   |  9 +++--
 3 files changed, 47 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-commonsrdf/blob/be2e0799/simple/src/main/java/org/apache/commons/rdf/simple/BlankNodeImpl.java
----------------------------------------------------------------------
diff --git a/simple/src/main/java/org/apache/commons/rdf/simple/BlankNodeImpl.java b/simple/src/main/java/org/apache/commons/rdf/simple/BlankNodeImpl.java
index cacfc5a..a1c75a1 100644
--- a/simple/src/main/java/org/apache/commons/rdf/simple/BlankNodeImpl.java
+++ b/simple/src/main/java/org/apache/commons/rdf/simple/BlankNodeImpl.java
@@ -17,39 +17,53 @@
  */
 package org.apache.commons.rdf.simple;
 
-import org.apache.commons.rdf.api.BlankNode;
-
 import java.nio.charset.StandardCharsets;
 import java.util.Objects;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicLong;
 
+import org.apache.commons.rdf.api.BlankNode;
+
 /**
  * A simple implementation of BlankNode.
  */
 final class BlankNodeImpl implements BlankNode {
 
-    private static AtomicLong bnodeCounter = new AtomicLong();
-
-    private static final Object DEFAULT_SALT = new Object();
+    private static final UUID SALT = UUID.randomUUID();
+    private static final AtomicLong COUNTER = new AtomicLong();
 
     private final String id;
 
     public BlankNodeImpl() {
-        this(DEFAULT_SALT, "genid:" + bnodeCounter.incrementAndGet());
+        this(SALT, Long.toString(COUNTER.incrementAndGet()));
     }
 
-    public BlankNodeImpl(Object uuidSalt, String id) {
+    public BlankNodeImpl(UUID uuidSalt, String id) {
         if (Objects.requireNonNull(id).isEmpty()) {
             throw new IllegalArgumentException("Invalid blank node id: " + id);
         }
-        String uuidInput = System.identityHashCode(uuidSalt) + ":" + id;
+
+        // Build a semi-URN - to be hashed for a name-based UUID below
         // Both the scope and the id are used to create the UUID, ensuring that
         // a caller can reliably create the same bnode if necessary by sending
-        // in the same scope.
-        // In addition, it would be very difficult for the default constructor
-        // to interfere with this process since it uses a local object as its
-        // reference.
+        // in the same scope to RDFTermFactory.createBlankNode(String)
+        String uuidInput = "urn:uuid:" + uuidSalt + "#" + id;
+
+        // The above is not a good value for this.id, as the id
+        // needs to be further escaped for
+        // ntriplesString() (there are no restrictions on
+        // RDFTermFactory.createBlankNode(String) ).
+
+
+        // Rather than implement ntriples escaping here, and knowing
+        // the internalIdentifier should contain a UUID anyway, we simply
+        // create another name-based UUID, and use it within both
+        // internalIdentifier() and within ntriplesString().
+        //
+        // A side-effect from this is that the blank node identifier
+        // is not preserved or shown in ntriplesString. In a way
+        // this is a feature, not a bug. as the contract for RDFTermFactory
+        // has no such requirement.
         this.id = UUID.nameUUIDFromBytes(
                 uuidInput.getBytes(StandardCharsets.UTF_8)).toString();
     }

http://git-wip-us.apache.org/repos/asf/incubator-commonsrdf/blob/be2e0799/simple/src/main/java/org/apache/commons/rdf/simple/SimpleRDFTermFactory.java
----------------------------------------------------------------------
diff --git a/simple/src/main/java/org/apache/commons/rdf/simple/SimpleRDFTermFactory.java b/simple/src/main/java/org/apache/commons/rdf/simple/SimpleRDFTermFactory.java
index bdc7b3e..b84fcd5 100644
--- a/simple/src/main/java/org/apache/commons/rdf/simple/SimpleRDFTermFactory.java
+++ b/simple/src/main/java/org/apache/commons/rdf/simple/SimpleRDFTermFactory.java
@@ -17,7 +17,16 @@
  */
 package org.apache.commons.rdf.simple;
 
-import org.apache.commons.rdf.api.*;
+import java.util.UUID;
+
+import org.apache.commons.rdf.api.BlankNode;
+import org.apache.commons.rdf.api.BlankNodeOrIRI;
+import org.apache.commons.rdf.api.Graph;
+import org.apache.commons.rdf.api.IRI;
+import org.apache.commons.rdf.api.Literal;
+import org.apache.commons.rdf.api.RDFTerm;
+import org.apache.commons.rdf.api.RDFTermFactory;
+import org.apache.commons.rdf.api.Triple;
 
 /**
  * A simple implementation of RDFTermFactory.
@@ -28,6 +37,10 @@ import org.apache.commons.rdf.api.*;
  */
 public class SimpleRDFTermFactory implements RDFTermFactory {
 
+    /** Unique salt per instance, for {@link #createBlankNode(String)}
+     */
+    private final UUID SALT = UUID.randomUUID();
+
     @Override
     public BlankNode createBlankNode() {
         return new BlankNodeImpl();
@@ -35,8 +48,7 @@ public class SimpleRDFTermFactory implements RDFTermFactory {
 
     @Override
     public BlankNode createBlankNode(String identifier) {
-        // Creates a BlankNodeImpl object using this object as the salt
-        return new BlankNodeImpl(this, identifier);
+        return new BlankNodeImpl(SALT, identifier);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/incubator-commonsrdf/blob/be2e0799/simple/src/test/java/org/apache/commons/rdf/simple/BlankNodeImplTest.java
----------------------------------------------------------------------
diff --git a/simple/src/test/java/org/apache/commons/rdf/simple/BlankNodeImplTest.java b/simple/src/test/java/org/apache/commons/rdf/simple/BlankNodeImplTest.java
index aa4c868..677fb75 100644
--- a/simple/src/test/java/org/apache/commons/rdf/simple/BlankNodeImplTest.java
+++ b/simple/src/test/java/org/apache/commons/rdf/simple/BlankNodeImplTest.java
@@ -17,16 +17,19 @@
  */
 package org.apache.commons.rdf.simple;
 
+import java.util.UUID;
+
 import org.apache.commons.rdf.api.AbstractBlankNodeTest;
 import org.apache.commons.rdf.api.BlankNode;
 
-import java.util.Optional;
-
 /**
  * Concrete implementation of BlankNodeImpl test.
  */
 public class BlankNodeImplTest extends AbstractBlankNodeTest {
 
+    // Fixed salt just for this test
+    private static UUID SALT = UUID.fromString("35019b59-18b3-4e74-8707-ec55f62a37d6");
+
     @Override
     protected BlankNode getBlankNode() {
         return new BlankNodeImpl();
@@ -34,7 +37,7 @@ public class BlankNodeImplTest extends AbstractBlankNodeTest {
 
     @Override
     protected BlankNode getBlankNode(String identifier) {
-        return new BlankNodeImpl(Optional.empty(), identifier);
+        return new BlankNodeImpl(SALT, identifier);
     }
 
 }