Fix versionability detection #401
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #401 +/- ##
==========================================
+ Coverage 98.43% 98.45% +0.01%
==========================================
Files 127 128 +1
Lines 14652 14777 +125
==========================================
+ Hits 14423 14548 +125
Misses 229 229
Continue to review full report at Codecov.
|
of objects. Move versioning-related stuff out of stix2.utils to its own module. Some misc versioning-related fixes.
was used but not imported there. Also import sorting from the pre-commit hook.
use allow_custom=False to force custom properties to be disallowed. This is extra insurance against a custom prop accidentally getting through.
three-value logic for allow_custom, it needed some much better documentation.
python2, Mapping.keys() returns a list instead of a set!
a list of tuples! (despite what it looks like if you glance at it quickly.)
properties from being changed, if a UUIDv5 is in use. Changing one of those properties would imply an ID change, which is not allowed across versions. Also: - add a trailing comma - change unchangable_properties to a set instead of a list, in case there are dupe props between STIX_UNMOD_PROPERTIES and sco_locked_props
- remove var 'properties_to_change' since it's unnecessary - delete most of remove_custom_stix() since it was unnecessary, greatly simplify it
chisholm
force-pushed
the
fix_versionable_detection
branch
from
fce4f15 to
c13cb18
Compare
clenk
approved these changes
Jun 10, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #394 .
Add versionability detection to versioning function (new_version()), and move versioning code out of stix2.utils and into its own module. It eases circular import issues which came up due new imports required for versionability detection, and also due to stix2.utils being imported so widely into other modules. The new module is named "versioning"... so now we have "version" and "versioning". Is that too confusing? There are also some versioning-related bugfixes.
Some details include:
Revoking an object unconditionally added the
revokedproperty, whether or not it was supported for the object type (due to hard-coded allow_custom=True). That seemed wrong and is now removed. If therevokedproperty is not supported, then revocation will fail because of that.Versioning didn't generically work for objects with custom properties. You have to have known it had custom properties in advance, and pass the allow_custom=True kwarg. Otherwise, creation of the new object ignored the allow_custom preference of the old one. This is fixed.
The function is designed to modify an existing object, so it made sense to have a three-value logic for allow_custom in this case: if True, the resulting object may have custom properties (regardless of whether the original did); if False, it may not; if None, it inherits the preference from the original object. The latter is probably most often what people would intuitively expect, so it is the default. (The
_allow_customattribute is checked, inspired by__deepcopy__.)If versioning a dict, there is no previous preference to propagate nor object to propagate it to, so allow_custom doesn't make sense and is ignored.
allow_custom is now a separate kwarg, rather than being lumped into the other **kwargs which confuses it with a normal object property. It also prevents that key from winding up in the result when you're versioning dicts (which the old implementation did).
The object marking unit tests were trying to change markings on a dict with no "type" property. That doesn't work because a markings change is a versioning change, and versionability checking on dicts requires looking up a registered class based on object type. So I added a type property.
Moved the TYPE_REGEX variables out of stix2.utils and into properties.py since that's the only place they're used.
When versioning a 2.1 SCO with a UUIDv5 ID, treat ID contributing properties as unchangeable. Changing them would either break the UUIDv5 correspondence, or require changing the ID (assuming the UUIDv5 was created in the spec-prescribed way), which is not allowed across versions.