Merge "Make `is_valid` more flexible with uuid validation"

2 files changed, 83 insertions, 8 deletions

diff --git a/pycadf/identifier.py b/pycadf/identifier.py
index 0b30d93..69619eb 100644
--- a/pycadf/identifier.py
+++ b/pycadf/identifier.py
@@ -12,6 +12,7 @@
 # License for the specific language governing permissions and limitations under
 # the License.
 import hashlib
+import re
 import uuid
 import warnings
 
@@ -33,6 +34,8 @@ if CONF.audit.namespace:
     md5_hash = hashlib.md5(CONF.audit.namespace.encode('utf-8'))
     AUDIT_NS = uuid.UUID(md5_hash.hexdigest())
 
+VALID_EXCEPTIONS = ['default', 'initiator', 'observer', 'target']
+
 
 def generate_uuid():
     """Generate a CADF identifier."""
@@ -48,15 +51,31 @@ def norm_ns(str_id):
     return prefix + str_id
 
 
+def _check_valid_uuid(value):
+    """Checks a value for one or multiple valid uuids joined together."""
+
+    if not value:
+        raise ValueError
+
+    value = re.sub('[{}-]|urn:uuid:', '', value)
+    for val in [value[i:i + 32] for i in range(0, len(value), 32)]:
+        uuid.UUID(val)
+
+
 def is_valid(value):
-    """Validation to ensure Identifier is correct."""
-    if value in ['target', 'initiator', 'observer']:
+    """Validation to ensure Identifier is correct.
+
+    If the Identifier value is a string type but not a valid UUID string,
+    warn against interoperability issues and return True. This relaxes
+    the requirement of having strict UUID checking.
+    """
+    if value in VALID_EXCEPTIONS:
         return True
     try:
-        uuid.UUID(value)
+        _check_valid_uuid(value)
     except (ValueError, TypeError):
         if not isinstance(value, six.string_types) or not value:
             return False
-        warnings.warn('Invalid uuid. To ensure interoperability, identifiers '
-                      'should be a valid uuid.')
+        warnings.warn(('Invalid uuid: %s. To ensure interoperability, '
+                      'identifiers should be a valid uuid.' % (value)))
     return True
diff --git a/pycadf/tests/test_cadf_spec.py b/pycadf/tests/test_cadf_spec.py
index 4a49625..416638f 100644
--- a/pycadf/tests/test_cadf_spec.py
+++ b/pycadf/tests/test_cadf_spec.py
@@ -38,16 +38,72 @@ from pycadf import timestamp
 class TestCADFSpec(base.TestCase):
 
     @mock.patch('pycadf.identifier.warnings.warn')
-    def test_identifier(self, warning_mock):
-        # empty string
-        self.assertFalse(identifier.is_valid(''))
+    def test_identifier_generated_uuid(self, warning_mock):
         # generated uuid
         self.assertTrue(identifier.is_valid(identifier.generate_uuid()))
         self.assertFalse(warning_mock.called)
+
+    @mock.patch('pycadf.identifier.warnings.warn')
+    def test_identifier_empty_string_is_invalid(self, warning_mock):
+        # empty string
+        self.assertFalse(identifier.is_valid(''))
+        self.assertFalse(warning_mock.called)
+
+    @mock.patch('pycadf.identifier.warnings.warn')
+    def test_identifier_any_string_is_invalid(self, warning_mock):
         # any string
         self.assertTrue(identifier.is_valid('blah'))
         self.assertTrue(warning_mock.called)
 
+    @mock.patch('pycadf.identifier.warnings.warn')
+    def test_identifier_joined_uuids_are_valid(self, warning_mock):
+        # multiple uuids joined together
+        long_128_uuids = [
+            ('3adce28e67e44544a5a9d5f1ab54f578a86d310aac3a465e9d'
+             'd2693a78b45c0e42dce28e67e44544a5a9d5f1ab54f578a86d'
+             '310aac3a465e9dd2693a78b45c0e'),
+            ('{3adce28e67e44544a5a9d5f1ab54f578a86d310aac3a465e9d'
+             'd2693a78b45c0e42dce28e67e44544a5a9d5f1ab54f578a86d'
+             '310aac3a465e9dd2693a78b45c0e}'),
+            ('{12345678-1234-5678-1234-567812345678'
+             '12345678-1234-5678-1234-567812345678'
+             '12345678-1234-5678-1234-567812345678'
+             '12345678-1234-5678-1234-567812345678}'),
+            ('urn:uuid:3adce28e67e44544a5a9d5f1ab54f578a86d310aac3a465e9d'
+             'd2693a78b45c0e42dce28e67e44544a5a9d5f1ab54f578a86d'
+             '310aac3a465e9dd2693a78b45c0e')]
+
+        for value in long_128_uuids:
+            self.assertTrue(identifier.is_valid(value))
+            self.assertFalse(warning_mock.called)
+
+    @mock.patch('pycadf.identifier.warnings.warn')
+    def test_identifier_long_nonjoined_uuid_is_invalid(self, warning_mock):
+        # long uuid not of size % 32
+        char_42_id = '3adce28e67e44544a5a9d5f1ab54f578a86d310aac'
+        self.assertTrue(identifier.is_valid(char_42_id))
+        self.assertTrue(warning_mock.called)
+
+    @mock.patch('pycadf.identifier.warnings.warn')
+    def test_identifier_specific_exceptions_are_valid(self, warning_mock):
+        # uuid exceptions
+        for value in identifier.VALID_EXCEPTIONS:
+            self.assertTrue(identifier.is_valid(value))
+            self.assertFalse(warning_mock.called)
+
+    @mock.patch('pycadf.identifier.warnings.warn')
+    def test_identifier_valid_id_extra_chars_is_valid(self, warning_mock):
+        # valid uuid with additional characters according to:
+        # https://docs.python.org/2/library/uuid.html
+        valid_ids = [
+            '{1234567890abcdef1234567890abcdef}',
+            '{12345678-1234-5678-1234-567812345678}',
+            'urn:uuid:12345678-1234-5678-1234-567812345678']
+
+        for value in valid_ids:
+            self.assertTrue(identifier.is_valid(value))
+            self.assertFalse(warning_mock.called)
+
     def test_endpoint(self):
         endp = endpoint.Endpoint(url='http://192.168.0.1',
                                  name='endpoint name',