You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jena.apache.org by an...@apache.org on 2024/04/05 08:37:33 UTC

(jena) 02/05: GH-2385: Use PrefixMappingBase for BufferingPrefixMapping

This is an automated email from the ASF dual-hosted git repository.

andy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/jena.git

commit 796d49b2a4e5aa38895c334fc431bec025bdca7a
Author: Andy Seaborne <an...@apache.org>
AuthorDate: Sun Mar 31 12:16:13 2024 +0100

    GH-2385: Use PrefixMappingBase for BufferingPrefixMapping
---
 .../jena/sparql/graph/PrefixMappingBase.java       | 110 +++++++++++----------
 .../system/buffering/BufferingPrefixMapping.java   |  66 +++++++++++--
 .../buffering/TestBufferingPrefixMapping.java      |  20 ++--
 .../java/org/apache/jena/rdf/model/impl/Util.java  |   4 +-
 .../main/java/org/apache/jena/util/SplitIRI.java   |  10 +-
 5 files changed, 139 insertions(+), 71 deletions(-)

diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/graph/PrefixMappingBase.java b/jena-arq/src/main/java/org/apache/jena/sparql/graph/PrefixMappingBase.java
index cdf19403e1..d52b18f496 100644
--- a/jena-arq/src/main/java/org/apache/jena/sparql/graph/PrefixMappingBase.java
+++ b/jena-arq/src/main/java/org/apache/jena/sparql/graph/PrefixMappingBase.java
@@ -39,55 +39,60 @@ import org.apache.jena.util.XMLChar;
  */
 public abstract class PrefixMappingBase implements PrefixMapping {
     /* Reverse mappings.
-     * The strict contract of PrefixMapping requires a separate reverse mapping to be stored and manipulated,
-     * which in turn adds complexity to the storage implementations.
-     * However, applications removing prefixes is unusual so we end up with a lot of complexity with little value.
-     * 
+     *
+     * The strict contract of PrefixMapping requires a separate reverse mapping to be
+     * stored and manipulated, which in turn adds complexity to the storage
+     * implementations. However, applications removing prefixes is unusual so we end
+     * up with a lot of complexity with little value.
+     *
      * Beware of the details of removing a mapping when there is another to the same URI.
      * If we had:
-     * 
-     *  Add (pref1, U)
-     *  Add (pref2, U)
-     * 
+     *
+     *     Add (pref1, U)
+     *     Add (pref2, U)
+     *
      * so that {@code U} reverse maps ({@link #getNsURIPrefix}) to {@code pref2} (it was
      * done second) then
-     * 
-     *  Remove (pref2)
-     * 
-     * it causes {@code U} to reverse map to {@code pref1}.
-     * 
+     *
+     *     Remove (pref2)
+     *
+     * causes {@code U} to reverse map to {@code pref1}.
+     *
      * This feature is quite a burden on implementations and should be regarded as "legacy" -
      * an implementation may not support this complex effect.
-     * 
+     *
      * PrefixMappingMem does.
      * Database backed ones typically don't.
      */
 
+    /** Whether to apply XML 1.0 prefix restrictions. */
+    private static final boolean XML_PREFIX_RULES = true;
+
     protected PrefixMappingBase() {}
-    
+
     // The storage operations of an implementation.
-    
+
     /** Add prefix */
     abstract protected void add(String prefix, String uri);
-    
+
     /** Remove prefix. */
     abstract protected void remove(String prefix);
-    
+
     /** Clear all mappings */
     abstract protected void clear();
 
     abstract protected boolean isEmpty();
-    
+
     abstract protected int size();
 
-    /** Return the URI that the prefix maps to. */ 
+    /** Return the URI that the prefix maps to. */
     abstract protected String prefixToUri(String prefix);
-    
+
     /** Return a prefix that maps to the URI.
-     * There may be several; the answer is any one of them. 
-     */ 
+     * There may be several; the answer is any one of them.
+     */
     abstract protected String uriToPrefix(String uri);
-    
+
     /** Return as a map. This map is only used within this class.
      * It can be as efficient as possible.
      * It will not be modified.
@@ -95,15 +100,15 @@ public abstract class PrefixMappingBase implements PrefixMapping {
      */
     abstract protected Map<String, String> asMap();
 
-    /** 
+    /**
      * Return as a map. The map return is not connected to the prefix mapping implementation,
-     * does not reflect subsequent prefix mapping changes.
+     * and does not reflect subsequent prefix mapping changes.
      */
     abstract protected Map<String, String> asMapCopy();
 
     /** Apply the {@link BiConsumer} to each (prefix, uri) pair. */
     abstract protected void apply(BiConsumer<String, String> action);
-    
+
     /**
      * This part of the subclass API and may be overridden if an implementation can do
      * better This general implementation is based on asMap() which may be a copy or may
@@ -112,7 +117,7 @@ public abstract class PrefixMappingBase implements PrefixMapping {
     protected Optional<Entry<String, String>> findMapping( String uri, boolean partial ) {
         return asMap().entrySet().stream().sequential().filter(e->{
             String ss = e.getValue();
-            if (uri.startsWith( ss ) && (partial || ss.length() == uri.length())) 
+            if (uri.startsWith( ss ) && (partial || ss.length() == uri.length()))
                 return true;
             return false;
         }).findFirst();
@@ -120,17 +125,17 @@ public abstract class PrefixMappingBase implements PrefixMapping {
 
     @Override
     public PrefixMapping setNsPrefix(String prefix, String uri) {
-        checkLegalPrefix(prefix); 
+        checkLegalPrefix(prefix);
         add(prefix, uri);
         return this;
     }
-    
+
     @Override
     public PrefixMapping removeNsPrefix(String prefix) {
         remove(prefix);
         return this;
     }
-    
+
     @Override
     public PrefixMapping clearNsPrefixMap() {
         clear();
@@ -138,21 +143,18 @@ public abstract class PrefixMappingBase implements PrefixMapping {
     }
 
     /**
-     * Checks that a prefix is "legal" - it must be a valid XML NCName or "". XML rules
-     * for RDF/XML output.
-     * <p>
-     * This is a recurring user question - why does {@code Resource.getNamespace},
-     * {@code Resource.getLocalname} not abbreviate when it is legal Turtle.
-     * <p>
-     * Answer - legacy for RDF/XML.
+     *
      * <p>
      * See also {@link #qnameFor}.
      */
     public static void checkLegalPrefix(String prefix) {
         if ( prefix == null )
             throw new PrefixMapping.IllegalPrefixException("null for prefix");
-        if ( prefix.length() > 0 && !XMLChar.isValidNCName(prefix) )
-            throw new PrefixMapping.IllegalPrefixException(prefix);
+        if ( XML_PREFIX_RULES ) {
+            // For Full compatibility with PrefixMappingImpl.
+            if ( prefix.length() > 0 && !XMLChar.isValidNCName(prefix) )
+                throw new PrefixMapping.IllegalPrefixException(prefix);
+        }
     }
 
     @Override
@@ -162,7 +164,7 @@ public abstract class PrefixMappingBase implements PrefixMapping {
             pmap2.apply((p,u)->setNsPrefix(p, u));
             return this;
         }
-        // Need to create as a map (a copy) and then add. 
+        // Need to create as a map (a copy) and then add.
         setNsPrefixes(pmap.getNsPrefixMap());
         return this;
     }
@@ -174,9 +176,9 @@ public abstract class PrefixMappingBase implements PrefixMapping {
     }
 
     /* Not javadoc.
-        This is the unusual contract as defined by the interface:
-      Update this PrefixMapping with the bindings in <code>map</code>, only
-      adding those (p, u) pairs for which neither p nor u appears in this mapping.
+     * This is the unusual contract as defined by the interface: update
+     * this PrefixMapping with the bindings in <code>map</code>, only adding those
+     * (p, u) pairs for which neither p nor u appears in this mapping.
      */
     @Override
     public PrefixMapping withDefaultMappings(PrefixMapping pmap) {
@@ -191,9 +193,9 @@ public abstract class PrefixMappingBase implements PrefixMapping {
         map.forEach(this::addWith);
         return this;
     }
-    
+
     private void addWith(String p, String u) {
-        if ( prefixToUri(p) == null && uriToPrefix(u) == null ) 
+        if ( prefixToUri(p) == null && uriToPrefix(u) == null )
             add(p,u);
     }
 
@@ -226,8 +228,12 @@ public abstract class PrefixMappingBase implements PrefixMapping {
 
     @Override
     public String qnameFor(String uri) {
-        // Turtle.  SplitIRI.splitpoint(uri);
-        int split = SplitIRI.splitXML(uri);
+        // This operation applies a fixed splitting rule and
+        // does not search for other possibilities.
+        // See findMapping.
+        int split = XML_PREFIX_RULES
+                ? SplitIRI.splitXML(uri)     // The longest XML NCName at the end of the URI
+                : SplitIRI.splitpoint(uri);  // Split at last "#", "/" (or ":" for urn's).
         String ns = uri.substring(0, split);
         String local = uri.substring(split);
         if ( local.equals("") )
@@ -235,7 +241,7 @@ public abstract class PrefixMappingBase implements PrefixMapping {
         String prefix = getNsURIPrefix(ns);
         return prefix == null ? null : prefix + ":" + local;
     }
-    
+
     @Override
     public String shortForm(String uri) {
         Optional<Entry<String, String>> e = findMapping(uri, true);
@@ -256,11 +262,11 @@ public abstract class PrefixMappingBase implements PrefixMapping {
         return this;
     }
 
-    @Override 
+    @Override
     public boolean hasNoMappings() {
         return isEmpty();
     }
-    
+
     @Override
     public int numPrefixes() {
         return size();
@@ -268,7 +274,7 @@ public abstract class PrefixMappingBase implements PrefixMapping {
 
     @Override
     public String toString() {
-        StringJoiner sj = new StringJoiner(", "); 
+        StringJoiner sj = new StringJoiner(", ");
         apply((p,u)->sj.add(p+"->"+u));
         return "pm:["+numPrefixes()+"]{"+sj.toString()+"}";
     }
diff --git a/jena-arq/src/main/java/org/apache/jena/system/buffering/BufferingPrefixMapping.java b/jena-arq/src/main/java/org/apache/jena/system/buffering/BufferingPrefixMapping.java
index 1d9e37ec17..7f29bb13e9 100644
--- a/jena-arq/src/main/java/org/apache/jena/system/buffering/BufferingPrefixMapping.java
+++ b/jena-arq/src/main/java/org/apache/jena/system/buffering/BufferingPrefixMapping.java
@@ -19,18 +19,24 @@
 package org.apache.jena.system.buffering;
 
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
+import java.util.function.BiConsumer;
 
 import org.apache.jena.shared.PrefixMapping;
-import org.apache.jena.shared.impl.PrefixMappingImpl;
+import org.apache.jena.sparql.graph.PrefixMappingBase;
+import org.apache.jena.sparql.graph.PrefixMappingMem;
 
-/** A {@link PrefixMapping} that buffers changes until {@link #flush()} is called. */
-public class BufferingPrefixMapping extends PrefixMappingImpl implements BufferingCtl {
+/**
+ * A {@link PrefixMapping} that buffers changes until {@link #flush()} is called.
+ */
+public class BufferingPrefixMapping extends PrefixMappingBase implements BufferingCtl {
 
     private static final boolean CHECK = false;
-    // Depends on the fact that PrefixMappingImpl maps everything get/set/remove.
-    private final PrefixMapping added = new PrefixMappingImpl();
+
+    private final PrefixMapping added = new PrefixMappingMem();
     private final Set<String> deleted = new HashSet<>();
+    // The underlying PrefixMapping
     private final PrefixMapping other;
 
     public BufferingPrefixMapping(PrefixMapping other) {
@@ -57,7 +63,7 @@ public class BufferingPrefixMapping extends PrefixMappingImpl implements Bufferi
     }
 
     @Override
-    protected void set(String prefix, String uri) {
+    protected void add(String prefix, String uri) {
         if ( CHECK ) {
             String u = other.getNsPrefixURI(prefix);
             if ( uri.equals(u) )
@@ -67,7 +73,7 @@ public class BufferingPrefixMapping extends PrefixMappingImpl implements Bufferi
     }
 
     @Override
-    protected String get(String prefix) {
+    protected String prefixToUri(String prefix) {
         if ( deleted.contains(prefix) )
             return null;
         String uri = added.getNsPrefixURI(prefix);
@@ -76,6 +82,19 @@ public class BufferingPrefixMapping extends PrefixMappingImpl implements Bufferi
         return other.getNsPrefixURI(prefix);
     }
 
+    @Override
+    protected String uriToPrefix(String uri) {
+        String prefix1 = added.getNsURIPrefix(uri);
+        if ( prefix1 != null )
+            return prefix1;
+        String prefix2 = other.getNsURIPrefix(uri);
+        if ( prefix2 == null )
+            return null;
+        if ( deleted.contains(prefix2) )
+            return null;
+        return prefix2;
+    }
+
     @Override
     protected void remove(String prefix) {
         deleted.add(prefix);
@@ -91,4 +110,37 @@ public class BufferingPrefixMapping extends PrefixMappingImpl implements Bufferi
     public boolean hasNoMappings() {
         return numPrefixes() == 0 ;
     }
+
+    @Override
+    protected void clear() {
+        apply((prefix, iri) -> remove(prefix));
+    }
+
+    @Override
+    protected boolean isEmpty() {
+        return size() == 0;
+    }
+
+    @Override
+    protected int size() {
+        return numPrefixes();
+    }
+
+    @Override
+    protected Map<String, String> asMap() {
+        return asMapCopy();
+    }
+
+    @Override
+    protected Map<String, String> asMapCopy() {
+        Map<String, String> map = other.getNsPrefixMap();
+        deleted.forEach(prefix->map.remove(prefix));
+        map.putAll(added.getNsPrefixMap());
+        return map;
+    }
+
+    @Override
+    protected void apply(BiConsumer<String, String> action) {
+        asMap().forEach(action);
+    }
 }
diff --git a/jena-arq/src/test/java/org/apache/jena/system/buffering/TestBufferingPrefixMapping.java b/jena-arq/src/test/java/org/apache/jena/system/buffering/TestBufferingPrefixMapping.java
index 072e8a0511..fc8af2b2a9 100644
--- a/jena-arq/src/test/java/org/apache/jena/system/buffering/TestBufferingPrefixMapping.java
+++ b/jena-arq/src/test/java/org/apache/jena/system/buffering/TestBufferingPrefixMapping.java
@@ -31,9 +31,13 @@ import org.junit.runners.MethodSorters;
 @FixMethodOrder(MethodSorters.NAME_ASCENDING)
 public class TestBufferingPrefixMapping {
 
+    protected PrefixMapping create(PrefixMapping base) {
+        return new BufferingPrefixMapping(base);
+    }
+
     @Test public void buffering_prefix_01_basic() {
         PrefixMapping base = new PrefixMappingImpl();
-        PrefixMapping pmap = new BufferingPrefixMapping(base);
+        PrefixMapping pmap = create(base);
         assertEquals(0, pmap.numPrefixes());
         assertTrue(pmap.hasNoMappings());
     }
@@ -42,7 +46,7 @@ public class TestBufferingPrefixMapping {
         // Base has a prefix.
         PrefixMapping base = new PrefixMappingImpl();
         base.setNsPrefix("x", "http://example/");
-        PrefixMapping pmap = new BufferingPrefixMapping(base);
+        PrefixMapping pmap = create(base);
         assertEquals(1, pmap.numPrefixes());
         assertFalse(pmap.hasNoMappings());
     }
@@ -50,7 +54,7 @@ public class TestBufferingPrefixMapping {
     @Test public void buffering_prefix_03_add() {
         PrefixMapping base = new PrefixMappingImpl();
 
-        PrefixMapping pmap = new BufferingPrefixMapping(base);
+        PrefixMapping pmap = create(base);
         pmap.setNsPrefix("x", "http://example/");
         assertFalse(pmap.hasNoMappings());
         assertEquals(1, pmap.numPrefixes());
@@ -62,7 +66,7 @@ public class TestBufferingPrefixMapping {
     @Test public void buffering_prefix_04_base_add() {
         PrefixMapping base = new PrefixMappingImpl();
         base.setNsPrefix("x1", "http://example/1#");
-        PrefixMapping pmap = new BufferingPrefixMapping(base);
+        PrefixMapping pmap = create(base);
         pmap.setNsPrefix("x2", "http://example/2#");
         assertEquals(2, pmap.numPrefixes());
         assertEquals(1, base.numPrefixes());
@@ -72,7 +76,7 @@ public class TestBufferingPrefixMapping {
     @Test public void buffering_prefix_05_add_remove() {
         PrefixMapping base = new PrefixMappingImpl();
         base.setNsPrefix("x", "http://example/");
-        PrefixMapping pmap = new BufferingPrefixMapping(base);
+        PrefixMapping pmap = create(base);
         pmap.removeNsPrefix("x");
 
         assertTrue(pmap.hasNoMappings());
@@ -85,12 +89,14 @@ public class TestBufferingPrefixMapping {
     @Test public void buffering_prefix_06_flush() {
         PrefixMapping base = new PrefixMappingImpl();
         base.setNsPrefix("x1", "http://example/1#");
-        BufferingPrefixMapping pmap = new BufferingPrefixMapping(base);
+        PrefixMapping pmap = create(base);
+        BufferingCtl ctl = (BufferingCtl)pmap;
         pmap.setNsPrefix("x2", "http://example/2#");
         assertEquals(2, pmap.numPrefixes());
         assertEquals(1, base.numPrefixes());
 
-        pmap.flush();
+        ctl.flush();
+
         assertEquals(2, base.numPrefixes());
         assertEquals("http://example/2#", base.getNsPrefixURI("x2"));
     }
diff --git a/jena-core/src/main/java/org/apache/jena/rdf/model/impl/Util.java b/jena-core/src/main/java/org/apache/jena/rdf/model/impl/Util.java
index f039c622e5..6d5d6a0216 100644
--- a/jena-core/src/main/java/org/apache/jena/rdf/model/impl/Util.java
+++ b/jena-core/src/main/java/org/apache/jena/rdf/model/impl/Util.java
@@ -39,11 +39,11 @@ public class Util extends Object {
 
     /**
      * Given an absolute URI, determine the split point between the namespace
-     * part and the localname part. See {@link SplitIRI#splitNamespaceXML} for details.
+     * part and the localname part. See {@link SplitIRI#splitXML} for details.
      */
     public static int splitNamespaceXML(String uri) {
         // Legacy. Call the moved code.
-        return SplitIRI.splitNamespaceXML(uri);
+        return SplitIRI.splitXML(uri);
     }
 
     public static String substituteStandardEntities( String s )
diff --git a/jena-core/src/main/java/org/apache/jena/util/SplitIRI.java b/jena-core/src/main/java/org/apache/jena/util/SplitIRI.java
index 77cf2725cb..fcc3e7d709 100644
--- a/jena-core/src/main/java/org/apache/jena/util/SplitIRI.java
+++ b/jena-core/src/main/java/org/apache/jena/util/SplitIRI.java
@@ -305,12 +305,16 @@ public class SplitIRI
         }
 
      */
-    /** Split point, according to XML qname rules.
+    /**
+     * Split point, according to XML qname rules.
      * This is the longest NCName at the end of the uri.
+     * Return a split at the end of the string if there is no match
+     * (e.g. the URI string ends in '/' or '#').
      */
     public static int splitXML(String string) { return splitNamespaceXML(string); }
 
-    /** Namespace, according to XML qname rules.
+    /**
+     * Namespace, according to XML qname rules.
      * Use with {@link #localnameXML}.
      */
     public static String namespaceXML(String string) {
@@ -409,7 +413,7 @@ public class SplitIRI
      * @return the index of the first character of the localname
      * @see SplitIRI
      */
-    public static int splitNamespaceXML(String uri) {
+    private static int splitNamespaceXML(String uri) {
 
         // XML Namespaces 1.0:
         // A qname name is NCName ':' NCName