Y client: Adjust URI scheme to move client-specific options to fragment

For example option `compress` controls kind of compression that _client_
performs when saving data to server. Similarly cache-size, logfile and
read-only adjust on-client behaviour, not server.

From nexedi/neoppod!18 (comment 124725) :

    In general the most correct thing to do is:

    - use host part for where to connect (host:port, list of host ports, UNIX socket, etc)
    - use path part to identify a database or other on-server resource
    - use query part for parameters that are passed to remote server (e.g. `storage` in case of ZEO)
    - use fragment part for local parameters that are not passed to remote server (e.g. local `logfile`)
    - use credentials part for things required to authenticate/encrypt.

    To normalize an URL wcfs client would drop credentials and fragment, but keep host, path and query.

    Fragments are documented not to be sent to remote side and to be evaluated by local side only.

-> Move options that control client behaviour to fragment.

neo/client/zodburi.py

@@ -17,7 +17,7 @@
 
 URI format:
 
-    neo(s)://[credentials@]master1,master2,...,masterN/name?options
+    neo(s)://[credentials@]master1,master2,...,masterN/name?server_options#client_options
 """
 
 import ZODB.config
@@ -30,6 +30,9 @@ from urlparse import urlsplit, parse_qsl
 # _credopts defines which options correspond to credentials
 _credopts = {'ca', 'cert', 'key'}
 
+# _clientopts defines which options control client behaviour, not the storage itself
+_clientopts = {'compress', 'read-only', 'logfile', 'cache-size'}
+
 # neo_zconf_options returns set of zconfig options supported by NEO storage
 def neo_zconf_options():
     neo_schema = """<schema>
@@ -46,11 +49,15 @@ def neo_zconf_options():
     assert 'name' in options
     options.remove('master_nodes') # comes in netloc
     options.remove('name')         # comes in path
-    for opt in _credopts:
+    for opt in _credopts.union(_clientopts):
         assert opt in options, opt
 
     return options
 
+# _srvopts defines which options control server behaviour
+_srvopts = neo_zconf_options() .difference(_credopts) .difference(_clientopts)
+
+
 # canonical_opt_name returns "oPtion_nAme" as "option-name"
 def canonical_opt_name(name):
     return name.lower().replace('_', '-')
@@ -61,8 +68,6 @@ def _resolve_uri(uri):
 
     if scheme not in ("neo", "neos"):
         raise ValueError("invalid uri: %s : expected neo:// or neos:// scheme" % uri)
-    if frag != "":
-        raise ValueError("invalid uri: %s : non-empty fragment" % uri)
 
     # name is given as path
     if path.startswith("/"):
@@ -92,21 +97,33 @@ def _resolve_uri(uri):
                 raise ValueError("invalid uri: %s : unexpected credential %s" % (uri, k))
             neokw[k] = v
 
-    # get options from query: only those that are defined by NEO schema go to
-    # storage - rest are returned as database options
+    # get server options from query
     dbkw = {}
-    neo_options = neo_zconf_options()
     for k, v in OrderedDict(parse_qsl(query)).items():
         if k in _credopts:
             raise ValueError("invalid uri: %s : option %s must be in credentials" % (uri, k))
-        elif k in neo_options:
+        if k in _clientopts:
+            raise ValueError("invalid uri: %s : option %s must be in fragment"    % (uri, k))
+        elif k in _srvopts:
+            neokw[k] = v
+        else:
+            raise ValueError("invalid uri: %s : invalid option %s" % (uri, k))
+
+    # get client option from fragment: only those that are defined by NEO
+    # schema go to storage - rest are returned as database options
+    for k, v in OrderedDict(parse_qsl(frag)).items():
+        if k in _credopts:
+            raise ValueError("invalid uri: %s : fragment option %s must be in credentials" % (uri, k))
+        if k in _srvopts:
+            raise ValueError("invalid uri: %s : fragment option %s must be in query"       % (uri, k))
+        elif k in _clientopts:
             neokw[k] = v
         else:
             # it might be option for storage, but not in canonical form e.g.
             # read_only -> read-only  (zodburi world settled on using "_" and
             # ZConfig world on "-" as separators)
             k2 = canonical_opt_name(k)
-            if k2 in neo_options:
+            if k2 in _clientopts:
                 neokw[k2] = v
 
             # else keep this kv as db option

neo/tests/client/testZODBURI.py

@@ -34,7 +34,7 @@ testv = [
 """,
      {}),
 
-    ("neo://master1,master2:port2/db3?read_only=true",
+    ("neo://master1,master2:port2/db3#read_only=true",
      """\
   master_nodes\tmaster1 master2:port2
   name\tdb3
@@ -42,8 +42,8 @@ testv = [
 """,
      {}),
 
-    ("neos://ca=qqq;cert=rrr;key=sss@[2001:67c:1254:2a::1]:1234,master2:port2/db4?read_only=false"
-     "&compress=true&logfile=xxx&alpha=111&dynamic_master_list=zzz"
+    ("neos://ca=qqq;cert=rrr;key=sss@[2001:67c:1254:2a::1]:1234,master2:port2/db4?dynamic_master_list=zzz"
+     "#read_only=false&compress=true&logfile=xxx&alpha=111"
      "&beta=222",
      """\
   master_nodes\t[2001:67c:1254:2a::1]:1234 master2:port2
@@ -51,10 +51,10 @@ testv = [
   ca\tqqq
   cert\trrr
   key\tsss
+  dynamic_master_list\tzzz
   read-only\tfalse
   compress\ttrue
   logfile\txxx
-  dynamic_master_list\tzzz
 """,
      {"alpha": "111", "beta": "222"}),
 ]
@@ -64,9 +64,8 @@ testv = [
 class ZODBURITests(unittest.TestCase):
 
     def test_zodburi(self):
-        # invalid schema / fragment
+        # invalid schema
         self.assertRaises(ValueError, _resolve_uri, "http://master/db")
-        self.assertRaises(ValueError, _resolve_uri, "neo://master/db#frag")
 
         # master/db not fully specified
         self.assertRaises(ValueError, _resolve_uri, "neo://master")
@@ -75,8 +74,12 @@ class ZODBURITests(unittest.TestCase):
         self.assertRaises(ValueError, _resolve_uri, "neo://master/db?master_nodes=a,b,c")
         self.assertRaises(ValueError, _resolve_uri, "neo://master/db?name=zzz")
 
-        # option that corresponds to credential provided in query
+        # option that corresponds to credential provided in query or fragment
         self.assertRaises(ValueError, _resolve_uri, "neos://master/db?ca=123")
+        self.assertRaises(ValueError, _resolve_uri, "neos://master/db#ca=123")
+
+        # option that corresponds to client provided in query
+        self.assertRaises(ValueError, _resolve_uri, "neos://master/db?compress=1")
 
         # credentials with neo:// instead of neos://
         self.assertRaises(ValueError, _resolve_uri, "neo://key:zzz@master/db")