namespace.py serializers/rdfxml.py stric qnames

The rdfxml serializer relied on namespace.py compute_qname to enforce
qname correctness. Chagnging split characteristics to support n3
prefixes broke that assumption. This commit is a first pass at fixing
the behvior. It may be worth renaming functions to make it clear that
the current 'compute_qname' function is no longer actually computing
a qname but a n3 prefix (or something like that).

7 files changed, 109 insertions, 23 deletions

diff --git a/rdflib/namespace.py b/rdflib/namespace.py
index 3bd80054..bea99905 100644
--- a/rdflib/namespace.py
+++ b/rdflib/namespace.py
@@ -353,6 +353,7 @@ class NamespaceManager(object):
     def __init__(self, graph):
         self.graph = graph
         self.__cache = {}
+        self.__cache_strict = {}
         self.__log = None
         self.__strie = {}
         self.__trie = {}
@@ -381,6 +382,13 @@ class NamespaceManager(object):
         else:
             return ":".join((prefix, name))
 
+    def qname_strict(self, uri):
+        prefix, namespace, name = self.compute_qname_strict(uri)
+        if prefix == '':
+            return name
+        else:
+            return ':'.join((prefix, name))
+
     def normalizeUri(self, rdfTerm):
         """
         Takes an RDF Term and 'normalizes' it into a QName (using the
@@ -448,6 +456,56 @@ class NamespaceManager(object):
             self.__cache[uri] = (prefix, namespace, name)
         return self.__cache[uri]
 
+    def compute_qname_strict(self, uri, generate=True):
+        # code repeated to avoid branching on strict every time
+        # if output needs to be strict (e.g. for xml) then
+        # only the strict output should bear the overhead
+        prefix, namespace, name = self.compute_qname(uri)
+        if is_ncname(text_type(name)):
+            return prefix, namespace, name
+        else:
+            if uri not in self.__cache_strict:
+                try:
+                    namespace, name = split_uri(uri, NAME_START_CATEGORIES)
+                except ValueError as e:
+                    message = ('This graph cannot be serialized to a strict format '
+                               'because there is no valid way to shorten {}'.format(uri))
+                    raise ValueError(message)
+                    # omitted for strict since NCNames cannot be empty
+                    #namespace = URIRef(uri)
+                    #prefix = self.store.prefix(namespace)
+                    #if not prefix:
+                        #raise e
+
+                if namespace not in self.__strie:
+                    insert_strie(self.__strie, self.__trie, namespace)
+
+                # omitted for strict
+                #if self.__strie[namespace]:
+                    #pl_namespace = get_longest_namespace(self.__strie[namespace], uri)
+                    #if pl_namespace is not None:
+                        #namespace = pl_namespace
+                        #name = uri[len(namespace):]
+
+                namespace = URIRef(namespace)
+                prefix = self.store.prefix(namespace)  # warning multiple prefixes problem
+
+                if prefix is None:
+                    if not generate:
+                        raise KeyError(
+                            "No known prefix for {} and generate=False".format(namespace)
+                        )
+                    num = 1
+                    while 1:
+                        prefix = "ns%s" % num
+                        if not self.store.namespace(prefix):
+                            break
+                        num += 1
+                    self.bind(prefix, namespace)
+                self.__cache_strict[uri] = (prefix, namespace, name)
+
+            return self.__cache_strict[uri]
+
     def bind(self, prefix, namespace, override=True, replace=False):
         """bind a given namespace to the prefix
 
@@ -568,27 +626,28 @@ ALLOWED_NAME_CHARS = [u"\u00B7", u"\u0387", u"-", u".", u"_", u":"]
 
 
 def is_ncname(name):
-    first = name[0]
-    if first == "_" or category(first) in NAME_START_CATEGORIES:
-        for i in range(1, len(name)):
-            c = name[i]
-            if not category(c) in NAME_CATEGORIES:
-                if c != ':' and c in ALLOWED_NAME_CHARS:
-                    continue
-                return 0
-            # if in compatibility area
-            # if decomposition(c)!='':
-            #    return 0
-
-        return 1
-    else:
-        return 0
+    if name:
+        first = name[0]
+        if first == "_" or category(first) in NAME_START_CATEGORIES:
+            for i in range(1, len(name)):
+                c = name[i]
+                if not category(c) in NAME_CATEGORIES:
+                    if c != ':' and c in ALLOWED_NAME_CHARS:
+                        continue
+                    return 0
+                # if in compatibility area
+                # if decomposition(c)!='':
+                #    return 0
+
+            return 1
+
+    return 0
 
 
 XMLNS = "http://www.w3.org/XML/1998/namespace"
 
 
-def split_uri(uri):
+def split_uri(uri, split_start=SPLIT_START_CATEGORIES):
     if uri.startswith(XMLNS):
         return (XMLNS, uri.split(XMLNS)[1])
     length = len(uri)
@@ -598,7 +657,7 @@ def split_uri(uri):
             if c in ALLOWED_NAME_CHARS:
                 continue
             for j in range(-1 - i, length):
-                if category(uri[j]) in SPLIT_START_CATEGORIES or uri[j] == "_":
+                if category(uri[j]) in split_start or uri[j] == "_":
                     # _ prevents early split, roundtrip not generate
                     ns = uri[:j]
                     if not ns:
@@ -641,4 +700,3 @@ def get_longest_namespace(trie, value):
             else:
                 return out
     return None
-
diff --git a/rdflib/plugins/serializers/rdfxml.py b/rdflib/plugins/serializers/rdfxml.py
index d5ca78b9..631c8fe0 100644
--- a/rdflib/plugins/serializers/rdfxml.py
+++ b/rdflib/plugins/serializers/rdfxml.py
@@ -32,7 +32,7 @@ class XMLSerializer(Serializer):
         bindings = {}
 
         for predicate in set(store.predicates()):
-            prefix, namespace, name = nm.compute_qname(predicate)
+            prefix, namespace, name = nm.compute_qname_strict(predicate)
             bindings[prefix] = URIRef(namespace)
 
         RDFNS = URIRef("http://www.w3.org/1999/02/22-rdf-syntax-ns#")
@@ -116,7 +116,7 @@ class XMLSerializer(Serializer):
     def predicate(self, predicate, object, depth=1):
         write = self.write
         indent = "  " * depth
-        qname = self.store.namespace_manager.qname(predicate)
+        qname = self.store.namespace_manager.qname_strict(predicate)
 
         if isinstance(object, Literal):
             attributes = ""
@@ -175,7 +175,7 @@ class PrettyXMLSerializer(Serializer):
             store.objects(None, RDF.type))
 
         for predicate in possible:
-            prefix, namespace, local = nm.compute_qname(predicate)
+            prefix, namespace, local = nm.compute_qname_strict(predicate)
             namespaces[prefix] = namespace
 
         namespaces["rdf"] = "http://www.w3.org/1999/02/22-rdf-syntax-ns#"
diff --git a/rdflib/plugins/serializers/xmlwriter.py b/rdflib/plugins/serializers/xmlwriter.py
index 1b67b8a7..de720e8c 100644
--- a/rdflib/plugins/serializers/xmlwriter.py
+++ b/rdflib/plugins/serializers/xmlwriter.py
@@ -107,4 +107,4 @@ class XMLWriter(object):
                 else:
                     return uri[len(ns):]
 
-        return self.nm.qname(uri)
+        return self.nm.qname_strict(uri)
diff --git a/test/n3/n3-writer-test-30.n3 b/test/n3/n3-writer-test-30.n3
index e8fd655c..88cf2210 100644
--- a/test/n3/n3-writer-test-30.n3
+++ b/test/n3/n3-writer-test-30.n3
@@ -2,9 +2,21 @@
 
 @prefix :       <http://example.org/here#> .
 @prefix full:   <http://example.org/full> .
+@prefix pref:     <http://example.org/prefix/> .
+@prefix more:   <http://example.org/prefix/more> .
+
+# Test namespace generation
 
 full: :x :y .
 :x full: :y .
 :x :y full: .
 
 full: full: full: . 
+
+# Test existing namespace
+
+more: :x :y .
+:x more: :y .
+:x :y more: .
+
+more: more: more: .
diff --git a/test/n3/n3-writer-test-31.n3 b/test/n3/n3-writer-test-31.n3
new file mode 100644
index 00000000..1c4494f1
--- /dev/null
+++ b/test/n3/n3-writer-test-31.n3
@@ -0,0 +1,15 @@
+# Test unshortenable strict qnames no predicates for xml sanity check
+
+@prefix :       <http://example.org/here#> .
+@prefix evil1:   <http://example.org/1> .
+@prefix evil2:   <http://example.org/prefix/1#> .
+
+# Test namespace generation
+
+evil1: :x :y .
+:x :y evil1: .
+
+# Test existing namespace
+
+evil2:1 :x :y .
+:x :y evil2:1 .
diff --git a/test/test_n3_suite.py b/test/test_n3_suite.py
index e28fab00..cebe086f 100644
--- a/test/test_n3_suite.py
+++ b/test/test_n3_suite.py
@@ -24,7 +24,6 @@ def _get_test_files_formats():
 def all_n3_files():
     skiptests = [
         'test/n3/example-lots_of_graphs.n3',  # the issue here seems to be with QuotedGraph, should this work?
-        # 'test/n3/n3-writer-test-29.n3', # this reveals the broken xml qname handling
         # 'test/n3/n3-writer-test-30.n3', # this reveals the broken xml qname handling
     ]
     for fpath, fmt in _get_test_files_formats():
diff --git a/test/test_roundtrip.py b/test/test_roundtrip.py
index 3d6ad604..fd58ea6a 100644
--- a/test/test_roundtrip.py
+++ b/test/test_roundtrip.py
@@ -28,6 +28,8 @@ tests roundtripping through rdf/xml with only the literals-02 file
 
 
 SKIP = [
+    ('xml', 'test/n3/n3-writer-test-29.n3'),  # has predicates that cannot be shortened to strict qnames
+    ('application/rdf+xml', 'test/n3/n3-writer-test-29.n3'),  # has predicates that cannot be shortened to strict qnames
     ('xml', 'test/nt/qname-02.nt'),  # uses a property that cannot be qname'd
     # uses a property that cannot be qname'd
     ('application/rdf+xml', 'test/nt/qname-02.nt'),