Ticket 50004 - lib389 - improve X-ORIGIN schema parsing

Bug Description: Schema parsing assumed X-ORIGIN was always in this
                 format "X-ORIGIN '", but it can also be in other
                 formats like: "X-ORIGIN (".  So when it did not contain
                 the original format we got list index errors.

Fix Description: Loosen the format to " X-ORIGIN " which all the formats.

                 Also:
                     improve from UI schema error messages
                     updated create_test for python3

https://pagure.io/389-ds-base/issue/50004

Reviewed by: firstyear & spichugi(Thanks!!)

dirsrvtests/create_test.py

@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 #
 # --- BEGIN COPYRIGHT BLOCK ---
 # Copyright (C) 2016 Red Hat, Inc.
@@ -122,10 +122,11 @@ def check_id_uniqueness(id_value):
 
     for root, dirs, files in os.walk(tests_dir):
         for name in files:
-            with open(os.path.join(root, name), "r") as file:
-                for line in file:
-                    if re.search(str(id_value), line):
-                        return False
+            if name.endswith('.py'):
+                with open(os.path.join(root, name), "r") as cifile:
+                    for line in cifile:
+                        if re.search(str(id_value), line):
+                            return False
 
     return True
 

src/cockpit/389-console/src/schema.js

@@ -127,7 +127,7 @@ function load_schema_objects_to_select(object, select_id) {
         }));
     });
   }).fail(function(select_data) {
-      console.log("failed: " + select_data.message);
+      console.log("Get schema failed: " + select_data.message);
       check_inst_alive(1);
   });
 }
@@ -219,7 +219,7 @@ function get_and_set_schema_tables() {
       }]
     });
   }).fail(function(oc_data) {
-      console.log("failed: " + oc_data.message);
+      console.log("Get objectclasses failed: " + oc_data.message);
       check_inst_alive(1);
   });
 
@@ -342,12 +342,12 @@ function get_and_set_schema_tables() {
         }]
       });
     }).fail(function(at_data) {
-        console.log("failed: " + at_data.message);
+        console.log("Get attributes failed: " + at_data.message);
         check_inst_alive(1);
     });
 
   }).fail(function(syntax_data) {
-      console.log("failed: " + syntax_data.message);
+      console.log("Get syntaxes failed: " + syntax_data.message);
       check_inst_alive(1);
   });
 
@@ -379,7 +379,7 @@ function get_and_set_schema_tables() {
     console.log("Finished loading schema.");
 
   }).fail(function(mr_data) {
-      console.log("failed: " + mr_data.cmd);
+      console.log("Get matching rules failed: " + mr_data.cmd);
       check_inst_alive(1);
   });
 }
@@ -498,7 +498,7 @@ $(document).ready( function() {
         }).
         fail(function(oc_data) {
           popup_err("err", oc_data.message);
-          console.log("failed: " + oc_data.message);
+          console.log("Search objectclasses failed: " + oc_data.message);
           check_inst_alive(1);
         });
         // Replace the option in 'Edit objectClass' window
@@ -693,7 +693,7 @@ $(document).ready( function() {
         }).
         fail(function(at_data) {
           popup_err("err", at_data.message);
-          console.log("failed: " + at_data.message);
+          console.log("Query attributes failed: " + at_data.message);
           check_inst_alive(1);
         });
         if (!edit) {

src/lib389/lib389/schema.py

@@ -13,6 +13,7 @@
 import glob
 import ldap
 import ldif
+import re
 from itertools import count
 from json import dumps as dump_json
 from operator import itemgetter
@@ -41,7 +42,7 @@ ATTR_SYNTAXES = {"1.3.6.1.4.1.1466.115.121.1.5": "Binary",
                  "1.3.6.1.4.1.1466.115.121.1.7": "Boolean",
                  "1.3.6.1.4.1.1466.115.121.1.11": "Country String",
                  "1.3.6.1.4.1.1466.115.121.1.12": "DN",
-                 "1.3.6.1.4.1.1466.115.121.1.14":  "Delivery Method",
+                 "1.3.6.1.4.1.1466.115.121.1.14": "Delivery Method",
                  "1.3.6.1.4.1.1466.115.121.1.15": "Directory String",
                  "1.3.6.1.4.1.1466.115.121.1.21": "Enhanced Guide",
                  "1.3.6.1.4.1.1466.115.121.1.22": "Facsimile",
@@ -64,6 +65,9 @@ ATTR_SYNTAXES = {"1.3.6.1.4.1.1466.115.121.1.5": "Binary",
                  "1.3.6.1.4.1.1466.115.121.1.52": "Telex Number"}
 
 
+X_ORIGIN_REGEX = r'\'(.*?)\''
+
+
 class Schema(DSLdapObject):
     """An object that represents the schema entry
 
@@ -113,35 +117,46 @@ class Schema(DSLdapObject):
         return result
 
     def _get_schema_objects(self, object_model, json=False):
+        """Get all the schema objects for a specific model: Attribute, Objectclass,
+        or Matchingreule.
+        """
         attr_name = self._get_attr_name_by_model(object_model)
-
         results = self.get_attr_vals_utf8(attr_name)
 
         if json:
-            object_insts = [vars(object_model(obj_i)) for obj_i in results]
-
-            for obj_i in object_insts:
-                # Add normalized name for sorting. Some matching rules don't have a name
+            object_insts = []
+            for obj in results:
+                obj_i = vars(object_model(obj))
                 if len(obj_i["names"]) == 1:
-                    obj_i['name'] = obj_i['names'][0]
+                    obj_i['name'] = obj_i['names'][0].lower()
                     obj_i['aliases'] = ""
                 elif len(obj_i["names"]) > 1:
-                    obj_i['name'] = obj_i['names'][0]
+                    obj_i['name'] = obj_i['names'][0].lower()
                     obj_i['aliases'] = obj_i['names'][1:]
                 else:
                     obj_i['name'] = ""
+
                 # Temporary workaround for X-ORIGIN in ObjectClass objects.
                 # It should be removed after https://github.com/python-ldap/python-ldap/pull/247 is merged
-                if "x_origin" not in obj_i and object_model != MatchingRule:
-                    for object_str in results:
-                        if obj_i['names'] == vars(object_model(object_str))['names'] and "X-ORIGIN" in object_str:
-                            obj_i['x_origin'] = object_str.split("X-ORIGIN '")[1].split("'")[0]
-            object_insts = sorted(object_insts, key=itemgetter('name'))
-            result = {'type': 'list', 'items': object_insts}
+                if " X-ORIGIN " in obj and obj_i['names'] == vars(object_model(obj))['names']:
+                    remainder = obj.split(" X-ORIGIN ")[1]
+                    if remainder[:1] == "(":
+                        # Have multiple values
+                        end = remainder.find(')')
+                        vals = remainder[1:end]
+                        vals = re.findall(X_ORIGIN_REGEX, vals)
+                        # For now use the first value, but this should be a set (another bug in python-ldap)
+                        obj_i['x_origin'] = vals[0]
+                    else:
+                        # Single X-ORIGIN value
+                        obj_i['x_origin'] = obj.split(" X-ORIGIN ")[1].split("'")[1]
+                object_insts.append(obj_i)
 
-            return result
+            object_insts = sorted(object_insts, key=itemgetter('name'))
+            return {'type': 'list', 'items': object_insts}
         else:
-            return [object_model(obj_i) for obj_i in results]
+            object_insts = [object_model(obj_i) for obj_i in results]
+            return sorted(object_insts, key=lambda x: x.names, reverse=False)
 
     def _get_schema_object(self, name, object_model, json=False):
         objects = self._get_schema_objects(object_model, json=json)

src/lib389/lib389/tests/cli/conf_schema_test.py

@@ -0,0 +1 @@
+# -*- coding: utf-8 -*-

src/lib389/lib389/tests/schema_test.py

@@ -1,72 +1,121 @@
 # --- BEGIN COPYRIGHT BLOCK ---
-# Copyright (C) 2015 Red Hat, Inc.
+# Copyright (C) 2018 Red Hat, Inc.
 # All rights reserved.
 #
 # License: GPL (version 3 or any later version).
 # See LICENSE for details.
 # --- END COPYRIGHT BLOCK ---
 #
-import pytest
-
-from lib389 import DirSrv
-from lib389._constants import SER_HOST, SER_PORT, SER_SERVERID_PROP, LOCALHOST
-from lib389.schema import Schema
-
-INSTANCE_PORT = 54321
-INSTANCE_SERVERID = 'standalone'
-
-
-class TopologyInstance(object):
-    def __init__(self, instance):
-        instance.open()
-        self.instance = instance
 
 
[email protected](scope="module")
-def topology(request):
-    instance = DirSrv(verbose=False)
-    instance.log.debug("Instance allocated")
-    args = {SER_HOST: LOCALHOST,
-            SER_PORT: INSTANCE_PORT,
-            SER_SERVERID_PROP: INSTANCE_SERVERID}
-    instance.allocate(args)
-    if instance.exists():
-        instance.delete()
-    instance.create()
-    instance.open()
+import logging
+import pytest
+import os
+from lib389._constants import *
+from lib389.schema import Schema
+from lib389.topologies import topology_st as topo
 
-    def fin():
-        instance.delete()
-    request.addfinalizer(fin)
+DEBUGGING = os.getenv("DEBUGGING", default=False)
+if DEBUGGING:
+    logging.getLogger(__name__).setLevel(logging.DEBUG)
+else:
+    logging.getLogger(__name__).setLevel(logging.INFO)
+log = logging.getLogger(__name__)
 
-    return TopologyInstance(instance)
 
+def test_schema(topo):
+    """Basic schema querying test
 
-def test_schema(topology):
-    must_expect = ['uidObject', 'account', 'posixAccount', 'shadowAccount']
+    :id: 995acc60-243b-45b0-9c1c-12bbe6a2882f
+    :setup: Standalone Instance
+    :steps:
+        1. Get attribute info for 'uid'
+        2. Check the attribute is found in the expected objectclasses (must, may)
+        3. Check that the 'account' objectclass is found
+    :expectedresults:
+        1. Success
+        2. Success
+        3. Success
+    """
+    must_expect = ['uidObject', 'account', 'posixAccount', 'shadowAccount', 'ipaUser',
+                   'sambaSamAccount']
     may_expect = ['cosDefinition', 'inetOrgPerson', 'inetUser',
-                  'mailRecipient']
-    attrtype, must, may = topology.instance.schema.query_attributetype('uid')
+                  'mailRecipient', 'nsOrgPerson', 'ipaUserOverride']
+    attrtype, must, may = topo.standalone.schema.query_attributetype('uid')
     assert attrtype.names == ('uid', 'userid')
     for oc in must:
         assert oc.names[0] in must_expect
     for oc in may:
         assert oc.names[0] in may_expect
-    assert topology.instance.schema.query_objectclass('account').names == \
+    assert topo.standalone.schema.query_objectclass('account').names == \
         ('account', )
 
-def test_schema_reload(topology):
-    """Test that the appropriate task entry is created when reloading schema."""
-    schema = Schema(topology.instance)
+
+def test_schema_reload(topo):
+    """Run a schema reload task
+
+    :id: 995acc60-243b-45b0-9c1c-12bbe6a2882e
+    :setup: Standalone Instance
+    :steps:
+        1. Add schema reload task
+        2. Schema reload task succeeds
+    :expectedresults:
+        1. Success
+        2. Success
+    """
+
+    schema = Schema(topo.standalone)
     task = schema.reload()
     assert task.exists()
     task.wait()
     assert task.get_exit_code() == 0
 
 
+def test_x_origin(topo):
+    """ Test that the various forms of X-ORIGIN are handled correctly
+
+    :id: 995acc60-243b-45b0-9c1c-12bbe6a2882d
+    :setup: Standalone Instance
+    :steps:
+        1. Create schema file with specific/unique formats for X-ORIGIN
+        2. Reload the schema file (schema reload task)
+        3. List all attributes without error
+        4. Confirm the expected results
+    :expectedresults:
+        1. Success
+        2. Success
+        3. Success
+        4. Success
+    """
+
+    # Create a custom schema file so we can tests specific formats
+    schema_file_name = topo.standalone.schemadir + '/98test.ldif'
+    schema_file = open(schema_file_name, "w")
+    testschema = ("dn: cn=schema\n" +
+                  "attributetypes: ( 8.9.10.11.12.13.16 NAME 'testattr' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'USER_DEFINED' )\n" +
+                  "attributetypes: ( 8.9.10.11.12.13.17 NAME 'testattrtwo' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN ( 'USER_DEFINED' 'should be not ignored!!' ) )\n")
+    schema_file.write(testschema)
+    schema_file.close()
+
+    # Reload the schema
+    myschema = Schema(topo.standalone)
+    task = myschema.reload()
+    assert task.exists()
+    task.wait()
+    assert task.get_exit_code() == 0
+
+    # Now search attrs and make sure there are no errors
+    myschema.get_attributetypes()
+    myschema.get_objectclasses()
+
+    # Check we have X-ORIGIN as expected
+    assert " 'USER_DEFINED' " in str(myschema.query_attributetype("testattr"))
+    assert " 'USER_DEFINED' " in str(myschema.query_attributetype("testattrtwo"))
+
+
 if __name__ == '__main__':
     # Run isolated
     # -s for DEBUG mode
-    import os
     CURRENT_FILE = os.path.realpath(__file__)
-    pytest.main("-s %s" % CURRENT_FILE)
+    pytest.main(["-s", CURRENT_FILE])
+