cti-python-stix2 icon indicating copy to clipboard operation
cti-python-stix2 copied to clipboard

@Custom decorators should warn if defining a common property it already does for you

Open clenk opened this issue 3 years ago • 4 comments

For example, this should raise a warning:

@CustomObject('x-animal', [
    ('created', TimestampProperty(required=True, precision='millisecond')),
    ('species', properties.StringProperty(required=True)),
    ('animal_class', properties.StringProperty()),
])
class Animal(object):

because created is a common property that the decorator already defines automatically.

clenk avatar May 27 '21 14:05 clenk

When you say raise a warning, do you mean something like a UserWarning or an actual exception? I'm guessing the former, but just interested. What is the current expected behaviour when a user does something like this? Is the property provided by the user respected or is it overwritten by the library?

maybe-sybr avatar Jul 08 '21 01:07 maybe-sybr

Clenk isn't around for awhile to speak for himself, but I suspect he meant to just display a warning. I don't know if he intended a simple logging statement of some sort, or a warnings.warn() type of warning. Current behavior unfortunately isn't consistent: a clashing property could overwrite what the decorator defines, or it could be overwritten.

The implementation creates a property order, e.g. [some properties] + [your properties] + [more properties]. So if your property clashes with a property in the first group, yours will overwrite it. If it clashes with a property in the last group, it will be overwritten. So you could look at the implementation and infer what will happen, but property conflicts should be avoided. Depending on how you define the conflicting property, you could wind up with a noncompliant object.

chisholm avatar Jul 08 '21 20:07 chisholm

Makes sense, thanks for the clarification @chisholm

maybe-sybr avatar Jul 09 '21 01:07 maybe-sybr

Yes, @chisholm has the right of it. I was thinking of warnings.warn().

clenk avatar Sep 22 '21 15:09 clenk