improve model validation

- add isset to check "real" attribute and not descriptor
- verify only id is set in shortform
- verify either resource or resourceId value is set, not both.

blueprint improve-validation
related-bug: #1242830

Change-Id: Ie9e3f26c5d30cd36e6013a1f0b77c8fe466cb3f7

13 files changed, 135 insertions, 52 deletions

diff --git a/pycadf/attachment.py b/pycadf/attachment.py
index bfd9b95..0191471 100644
--- a/pycadf/attachment.py
+++ b/pycadf/attachment.py
@@ -57,7 +57,7 @@ class Attachment(cadftype.CADFAbstractType):
     def is_valid(self):
         # Existence test, All attributes must exist for valid Attachment type
         return (
-            hasattr(self, ATTACHMENT_KEYNAME_TYPEURI) and
-            hasattr(self, ATTACHMENT_KEYNAME_NAME) and
-            hasattr(self, ATTACHMENT_KEYNAME_CONTENT)
+            self._isset(ATTACHMENT_KEYNAME_TYPEURI) and
+            self._isset(ATTACHMENT_KEYNAME_NAME) and
+            self._isset(ATTACHMENT_KEYNAME_CONTENT)
         )
diff --git a/pycadf/cadftype.py b/pycadf/cadftype.py
index 604b178..89dffd5 100644
--- a/pycadf/cadftype.py
+++ b/pycadf/cadftype.py
@@ -85,6 +85,15 @@ class CADFAbstractType(object):
         """Return dict representation of Event."""
         return jsonutils.to_primitive(self, convert_instances=True)
 
+    def _isset(self, attr):
+        """Check to see if attribute is defined."""
+        try:
+            if isinstance(getattr(self, attr), ValidatorDescriptor):
+                return False
+            return True
+        except AttributeError:
+            return False
+
     # TODO(mrutkows): Eventually, we want to use the OrderedDict (introduced
     # in Python 2.7) type for all CADF classes to store attributes in a
     # canonical form.  Currently, OpenStack/Jenkins requires 2.6 compatibility
diff --git a/pycadf/credential.py b/pycadf/credential.py
index f58f6b6..55b85df 100644
--- a/pycadf/credential.py
+++ b/pycadf/credential.py
@@ -46,4 +46,4 @@ class Credential(cadftype.CADFAbstractType):
     # TODO(mrutkows): validate this cadf:Credential type against schema
     def is_valid(self):
         # TODO(mrutkows): validate specific attribute type/format
-        return hasattr(self, CRED_KEYNAME_TOKEN)
+        return self._isset(CRED_KEYNAME_TOKEN)
diff --git a/pycadf/endpoint.py b/pycadf/endpoint.py
index 046d7cd..2528767 100644
--- a/pycadf/endpoint.py
+++ b/pycadf/endpoint.py
@@ -49,4 +49,4 @@ class Endpoint(cadftype.CADFAbstractType):
 
     # TODO(mrutkows): validate this cadf:ENDPOINT type against schema
     def is_valid(self):
-        return hasattr(self, ENDPOINT_KEYNAME_URL)
+        return self._isset(ENDPOINT_KEYNAME_URL)
diff --git a/pycadf/event.py b/pycadf/event.py
index e9a91e4..434bf3f 100644
--- a/pycadf/event.py
+++ b/pycadf/event.py
@@ -46,6 +46,7 @@ EVENT_KEYNAME_MEASUREMENTS = "measurements"
 EVENT_KEYNAME_TAGS = "tags"
 EVENT_KEYNAME_ATTACHMENTS = "attachments"
 EVENT_KEYNAME_OBSERVER = "observer"
+EVENT_KEYNAME_OBSERVERID = "observerId"
 EVENT_KEYNAME_REPORTERCHAIN = "reporterchain"
 
 EVENT_KEYNAMES = [EVENT_KEYNAME_TYPEURI,
@@ -64,6 +65,7 @@ EVENT_KEYNAMES = [EVENT_KEYNAME_TYPEURI,
                   EVENT_KEYNAME_TAGS,
                   EVENT_KEYNAME_ATTACHMENTS,
                   EVENT_KEYNAME_OBSERVER,
+                  EVENT_KEYNAME_OBSERVERID,
                   EVENT_KEYNAME_REPORTERCHAIN]
 
 
@@ -100,12 +102,14 @@ class Event(cadftype.CADFAbstractType):
     observer = cadftype.ValidatorDescriptor(
         EVENT_KEYNAME_OBSERVER,
         (lambda x: isinstance(x, resource.Resource) and x.is_valid()))
+    observerId = cadftype.ValidatorDescriptor(
+        EVENT_KEYNAME_OBSERVERID, lambda x: identifier.is_valid(x))
 
     def __init__(self, eventType=cadftype.EVENTTYPE_ACTIVITY,
                  id=None, eventTime=None,
                  action=cadftaxonomy.UNKNOWN, outcome=cadftaxonomy.UNKNOWN,
                  initiator=None, initiatorId=None, target=None, targetId=None,
-                 severity=None, reason=None, observer=None):
+                 severity=None, reason=None, observer=None, observerId=None):
 
         # Establish typeURI for the CADF Event data type
         # TODO(mrutkows): support extended typeURIs for Event subtypes
@@ -127,13 +131,16 @@ class Event(cadftype.CADFAbstractType):
         # Event.outcome (Mandatory)
         setattr(self, EVENT_KEYNAME_OUTCOME, outcome)
 
-        # Event.observer (Mandatory)
-        setattr(self, EVENT_KEYNAME_OBSERVER, observer)
+        # Event.observer (Mandatory if no observerId)
+        if observer is not None:
+            setattr(self, EVENT_KEYNAME_OBSERVER, observer)
+        # Event.observerId (Dependent)
+        if observerId is not None:
+            setattr(self, EVENT_KEYNAME_OBSERVERID, observerId)
 
         # Event.initiator (Mandatory if no initiatorId)
         if initiator is not None:
             setattr(self, EVENT_KEYNAME_INITIATOR, initiator)
-
         # Event.initiatorId (Dependent)
         if initiatorId is not None:
             setattr(self, EVENT_KEYNAME_INITIATORID, initiatorId)
@@ -141,7 +148,6 @@ class Event(cadftype.CADFAbstractType):
         # Event.target (Mandatory if no targetId)
         if target is not None:
             setattr(self, EVENT_KEYNAME_TARGET, target)
-
         # Event.targetId (Dependent)
         if targetId is not None:
             setattr(self, EVENT_KEYNAME_TARGETID, targetId)
@@ -222,15 +228,17 @@ class Event(cadftype.CADFAbstractType):
         # TODO(mrutkows): Eventually, make sure all attributes are
         # from either the CADF spec. (or profiles thereof)
         # TODO(mrutkows): validate all child attributes that are CADF types
-        # TODO(mrutkows): Cannot have both an initiator and initiatorId
-        # TODO(mrutkows): Cannot have both an target and targetId
         return (
-            hasattr(self, EVENT_KEYNAME_TYPEURI) and
-            hasattr(self, EVENT_KEYNAME_EVENTTYPE) and
-            hasattr(self, EVENT_KEYNAME_ID) and
-            hasattr(self, EVENT_KEYNAME_EVENTTIME) and
-            hasattr(self, EVENT_KEYNAME_ACTION) and
-            hasattr(self, EVENT_KEYNAME_OUTCOME) and
-            hasattr(self, EVENT_KEYNAME_INITIATOR) and
-            hasattr(self, EVENT_KEYNAME_TARGET)
+            self._isset(EVENT_KEYNAME_TYPEURI) and
+            self._isset(EVENT_KEYNAME_EVENTTYPE) and
+            self._isset(EVENT_KEYNAME_ID) and
+            self._isset(EVENT_KEYNAME_EVENTTIME) and
+            self._isset(EVENT_KEYNAME_ACTION) and
+            self._isset(EVENT_KEYNAME_OUTCOME) and
+            (self._isset(EVENT_KEYNAME_INITIATOR) ^
+             self._isset(EVENT_KEYNAME_INITIATORID)) and
+            (self._isset(EVENT_KEYNAME_TARGET) ^
+             self._isset(EVENT_KEYNAME_TARGETID)) and
+            (self._isset(EVENT_KEYNAME_OBSERVER) ^
+             self._isset(EVENT_KEYNAME_OBSERVERID))
         )
diff --git a/pycadf/geolocation.py b/pycadf/geolocation.py
index e2ffd82..e41487b 100644
--- a/pycadf/geolocation.py
+++ b/pycadf/geolocation.py
@@ -118,8 +118,4 @@ class Geolocation(cadftype.CADFAbstractType):
 
     # self validate cadf:Geolocation type
     def is_valid(self):
-        # TODO(mrutkows): validate specific attribute type/format
-        for attr in GEO_KEYNAMES:
-            if not hasattr(self, attr):
-                return False
         return True
diff --git a/pycadf/host.py b/pycadf/host.py
index 565f00e..d606a0d 100644
--- a/pycadf/host.py
+++ b/pycadf/host.py
@@ -59,7 +59,4 @@ class Host(cadftype.CADFAbstractType):
 
     # TODO(mrutkows): validate this cadf:Host type against schema
     def is_valid(self):
-        return (hasattr(self, HOST_KEYNAME_ID) or
-                hasattr(self, HOST_KEYNAME_ADDR) or
-                hasattr(self, HOST_KEYNAME_AGENT) or
-                hasattr(self, HOST_KEYNAME_PLATFORM))
+        return True
diff --git a/pycadf/measurement.py b/pycadf/measurement.py
index 58bd03b..3cef90d 100644
--- a/pycadf/measurement.py
+++ b/pycadf/measurement.py
@@ -64,4 +64,6 @@ class Measurement(cadftype.CADFAbstractType):
 
     # self validate this cadf:Measurement type against schema
     def is_valid(self):
-        return hasattr(self, MEASUREMENT_KEYNAME_RESULT)
+        return (self._isset(MEASUREMENT_KEYNAME_RESULT) and
+                (self._isset(MEASUREMENT_KEYNAME_METRIC) ^
+                 self._isset(MEASUREMENT_KEYNAME_METRICID)))
diff --git a/pycadf/metric.py b/pycadf/metric.py
index 190c490..becce39 100644
--- a/pycadf/metric.py
+++ b/pycadf/metric.py
@@ -68,6 +68,6 @@ class Metric(cadftype.CADFAbstractType):
     def is_valid(self):
         # Existence test, id, and unit attributes must both exist
         return (
-            hasattr(self, METRIC_KEYNAME_METRICID) and
-            hasattr(self, METRIC_KEYNAME_UNIT)
+            self._isset(METRIC_KEYNAME_METRICID) and
+            self._isset(METRIC_KEYNAME_UNIT)
         )
diff --git a/pycadf/reason.py b/pycadf/reason.py
index cf41f8c..85d4ed6 100644
--- a/pycadf/reason.py
+++ b/pycadf/reason.py
@@ -68,7 +68,7 @@ class Reason(cadftype.CADFAbstractType):
     # TODO(mrutkows): validate this cadf:Reason type against schema
     def is_valid(self):
         # MUST have at least one valid pairing of reason+code or policy+id
-        return ((hasattr(self, REASON_KEYNAME_REASONTYPE) and
-                 hasattr(self, REASON_KEYNAME_REASONCODE)) or
-                (hasattr(self, REASON_KEYNAME_POLICYTYPE) and
-                 hasattr(self, REASON_KEYNAME_POLICYID)))
+        return ((self._isset(REASON_KEYNAME_REASONTYPE) and
+                 self._isset(REASON_KEYNAME_REASONCODE)) or
+                (self._isset(REASON_KEYNAME_POLICYTYPE) and
+                 self._isset(REASON_KEYNAME_POLICYID)))
diff --git a/pycadf/reporterstep.py b/pycadf/reporterstep.py
index 94c7d75..6cc03d6 100644
--- a/pycadf/reporterstep.py
+++ b/pycadf/reporterstep.py
@@ -68,7 +68,7 @@ class Reporterstep(cadftype.CADFAbstractType):
     # self validate this cadf:Reporterstep type against schema
     def is_valid(self):
         return (
-            hasattr(self, REPORTERSTEP_KEYNAME_ROLE) and
-            (hasattr(self, REPORTERSTEP_KEYNAME_REPORTER) or
-             hasattr(self, REPORTERSTEP_KEYNAME_REPORTERID))
+            self._isset(REPORTERSTEP_KEYNAME_ROLE) and
+            (self._isset(REPORTERSTEP_KEYNAME_REPORTER) ^
+             self._isset(REPORTERSTEP_KEYNAME_REPORTERID))
         )
diff --git a/pycadf/resource.py b/pycadf/resource.py
index fd26d04..6398c1a 100644
--- a/pycadf/resource.py
+++ b/pycadf/resource.py
@@ -153,8 +153,9 @@ class Resource(cadftype.CADFAbstractType):
 
     # self validate this cadf:Resource type against schema
     def is_valid(self):
-        return (hasattr(self, RESOURCE_KEYNAME_ID) and
-                (getattr(self, RESOURCE_KEYNAME_ID) == "target" or
-                 getattr(self, RESOURCE_KEYNAME_ID) == "initiator" or
-                 hasattr(self, RESOURCE_KEYNAME_TYPEURI)))
+        return (self._isset(RESOURCE_KEYNAME_ID) and
+                (self._isset(RESOURCE_KEYNAME_TYPEURI) or
+                 ((getattr(self, RESOURCE_KEYNAME_ID) == "target" or
+                   getattr(self, RESOURCE_KEYNAME_ID) == "initiator") and
+                  len(vars(self).keys()) == 1)))
         # TODO(mrutkows): validate the Resource's attribute types
diff --git a/pycadf/tests/test_cadf_spec.py b/pycadf/tests/test_cadf_spec.py
index 23bee22..0635cdd 100644
--- a/pycadf/tests/test_cadf_spec.py
+++ b/pycadf/tests/test_cadf_spec.py
@@ -38,6 +38,7 @@ class TestCADFSpec(testtools.TestCase):
         endp = endpoint.Endpoint(url='http://192.168.0.1',
                                  name='endpoint name',
                                  port='8080')
+        self.assertEqual(endp.is_valid(), True)
         dict_endp = endp.as_dict()
         for key in endpoint.ENDPOINT_KEYNAMES:
             self.assertIn(key, dict_endp)
@@ -47,6 +48,7 @@ class TestCADFSpec(testtools.TestCase):
                       address='192.168.0.1',
                       agent='client',
                       platform='AIX')
+        self.assertEqual(h.is_valid(), True)
         dict_host = h.as_dict()
         for key in host.HOST_KEYNAMES:
             self.assertIn(key, dict_host)
@@ -54,6 +56,7 @@ class TestCADFSpec(testtools.TestCase):
     def test_credential(self):
         cred = credential.Credential(type='auth token',
                                      token=identifier.generate_uuid())
+        self.assertEqual(cred.is_valid(), True)
         dict_cred = cred.as_dict()
         for key in credential.CRED_KEYNAMES:
             self.assertIn(key, dict_cred)
@@ -67,6 +70,7 @@ class TestCADFSpec(testtools.TestCase):
                                       city='toronto',
                                       state='ontario',
                                       regionICANN='ca')
+        self.assertEqual(geo.is_valid(), True)
 
         dict_geo = geo.as_dict()
         for key in geolocation.GEO_KEYNAMES:
@@ -76,6 +80,7 @@ class TestCADFSpec(testtools.TestCase):
         metric_val = metric.Metric(metricId=identifier.generate_uuid(),
                                    unit='b',
                                    name='bytes')
+        self.assertEqual(metric_val.is_valid(), True)
 
         dict_metric_val = metric_val.as_dict()
         for key in metric.METRIC_KEYNAMES:
@@ -87,16 +92,30 @@ class TestCADFSpec(testtools.TestCase):
             metric=metric.Metric(),
             metricId=identifier.generate_uuid(),
             calculatedBy=resource.Resource(typeURI='storage'))
+        self.assertEqual(measure_val.is_valid(), False)
 
         dict_measure_val = measure_val.as_dict()
         for key in measurement.MEASUREMENT_KEYNAMES:
             self.assertIn(key, dict_measure_val)
 
+        measure_val = measurement.Measurement(
+            result='100',
+            metric=metric.Metric(),
+            calculatedBy=resource.Resource(typeURI='storage'))
+        self.assertEqual(measure_val.is_valid(), True)
+
+        measure_val = measurement.Measurement(
+            result='100',
+            metricId=identifier.generate_uuid(),
+            calculatedBy=resource.Resource(typeURI='storage'))
+        self.assertEqual(measure_val.is_valid(), True)
+
     def test_reason(self):
         reason_val = reason.Reason(reasonType='HTTP',
                                    reasonCode='200',
                                    policyType='poltype',
                                    policyId=identifier.generate_uuid())
+        self.assertEqual(reason_val.is_valid(), True)
 
         dict_reason_val = reason_val.as_dict()
         for key in reason.REASON_KEYNAMES:
@@ -108,15 +127,29 @@ class TestCADFSpec(testtools.TestCase):
             reporter=resource.Resource(typeURI='storage'),
             reporterId=identifier.generate_uuid(),
             reporterTime=timestamp.get_utc_now())
+        self.assertEqual(step.is_valid(), False)
 
         dict_step = step.as_dict()
         for key in reporterstep.REPORTERSTEP_KEYNAMES:
             self.assertIn(key, dict_step)
 
+        step = reporterstep.Reporterstep(
+            role='modifier',
+            reporter=resource.Resource(typeURI='storage'),
+            reporterTime=timestamp.get_utc_now())
+        self.assertEqual(step.is_valid(), True)
+
+        step = reporterstep.Reporterstep(
+            role='modifier',
+            reporterId=identifier.generate_uuid(),
+            reporterTime=timestamp.get_utc_now())
+        self.assertEqual(step.is_valid(), True)
+
     def test_attachment(self):
         attach = attachment.Attachment(typeURI='attachURI',
                                        content='content',
                                        name='attachment_name')
+        self.assertEqual(attach.is_valid(), True)
 
         dict_attach = attach.as_dict()
         for key in attachment.ATTACHMENT_KEYNAMES:
@@ -137,10 +170,21 @@ class TestCADFSpec(testtools.TestCase):
                                                  content='content',
                                                  name='attachment_name'))
         res.add_address(endpoint.Endpoint(url='http://192.168.0.1'))
+
+        self.assertEqual(res.is_valid(), True)
         dict_res = res.as_dict()
         for key in resource.RESOURCE_KEYNAMES:
             self.assertIn(key, dict_res)
 
+    def test_resource_shortform(self):
+        res = resource.Resource(id='target')
+        self.assertEqual(res.is_valid(), True)
+
+        res.add_attachment(attachment.Attachment(typeURI='attachURI',
+                                                 content='content',
+                                                 name='attachment_name'))
+        self.assertEqual(res.is_valid(), False)
+
     def test_event(self):
         ev = event.Event(eventType='activity',
                          id=identifier.generate_uuid(),
@@ -151,11 +195,14 @@ class TestCADFSpec(testtools.TestCase):
                          target=resource.Resource(typeURI='storage'),
                          targetId=identifier.generate_uuid(),
                          observer=resource.Resource(id='target'),
+                         observerId=identifier.generate_uuid(),
                          outcome='success',
                          reason=reason.Reason(reasonType='HTTP',
                                               reasonCode='200'),
                          severity='high')
-        ev.add_measurement(measurement.Measurement(result='100'))
+        ev.add_measurement(
+            measurement.Measurement(result='100',
+                                    metricId=identifier.generate_uuid())),
         ev.add_tag(tag.generate_name_value_tag('name', 'val'))
         ev.add_attachment(attachment.Attachment(typeURI='attachURI',
                                                 content='content',
@@ -166,50 +213,73 @@ class TestCADFSpec(testtools.TestCase):
             reporter=resource.Resource(typeURI='service/security')))
         ev.add_reporterstep(reporterstep.Reporterstep(
             reporterId=identifier.generate_uuid()))
+        self.assertEqual(ev.is_valid(), False)
 
         dict_ev = ev.as_dict()
         for key in event.EVENT_KEYNAMES:
             self.assertIn(key, dict_ev)
 
-    def test_event_unique(self):
         ev = event.Event(eventType='activity',
+                         id=identifier.generate_uuid(),
+                         eventTime=timestamp.get_utc_now(),
                          initiator=resource.Resource(typeURI='storage'),
-                         initiatorId=identifier.generate_uuid(),
                          action='read',
                          target=resource.Resource(typeURI='storage'),
+                         observer=resource.Resource(id='target'),
+                         outcome='success')
+        self.assertEqual(ev.is_valid(), True)
+
+        ev = event.Event(eventType='activity',
+                         id=identifier.generate_uuid(),
+                         eventTime=timestamp.get_utc_now(),
+                         initiatorId=identifier.generate_uuid(),
+                         action='read',
                          targetId=identifier.generate_uuid(),
+                         observerId=identifier.generate_uuid(),
+                         outcome='success')
+        self.assertEqual(ev.is_valid(), True)
+
+        ev = event.Event(eventType='activity',
+                         id=identifier.generate_uuid(),
+                         eventTime=timestamp.get_utc_now(),
+                         initiator=resource.Resource(typeURI='storage'),
+                         action='read',
+                         targetId=identifier.generate_uuid(),
+                         observer=resource.Resource(id='target'),
+                         outcome='success')
+        self.assertEqual(ev.is_valid(), True)
+
+    def test_event_unique(self):
+        ev = event.Event(eventType='activity',
+                         initiator=resource.Resource(typeURI='storage'),
+                         action='read',
+                         target=resource.Resource(typeURI='storage'),
                          observer=resource.Resource(id='target'),
                          outcome='success')
         time.sleep(1)
         ev2 = event.Event(eventType='activity',
                           initiator=resource.Resource(typeURI='storage'),
-                          initiatorId=identifier.generate_uuid(),
                           action='read',
                           target=resource.Resource(typeURI='storage'),
-                          targetId=identifier.generate_uuid(),
                           observer=resource.Resource(id='target'),
                           outcome='success')
         self.assertNotEqual(ev.id, ev2.id)
         self.assertNotEqual(ev.eventTime, ev2.eventTime)
 
-    def test_event_resource_shortform(self):
+    def test_event_resource_shortform_not_self(self):
         self.assertRaises(ValueError,
                           lambda: event.Event(
                               eventType='activity',
                               initiator=resource.Resource(typeURI='storage'),
-                              initiatorId=identifier.generate_uuid(),
                               action='read',
                               target=resource.Resource(id='target'),
-                              targetId=identifier.generate_uuid(),
                               observer=resource.Resource(id='target'),
                               outcome='success'))
         self.assertRaises(ValueError,
                           lambda: event.Event(
                               eventType='activity',
                               initiator=resource.Resource(id='initiator'),
-                              initiatorId=identifier.generate_uuid(),
                               action='read',
                               target=resource.Resource(typeURI='storage'),
-                              targetId=identifier.generate_uuid(),
                               observer=resource.Resource(id='target'),
                               outcome='success'))