Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add is_sdo() et al functions #483

Merged
merged 20 commits into from
Jan 29, 2021
Merged

add is_sdo() et al functions #483

merged 20 commits into from
Jan 29, 2021

Conversation

chisholm
Copy link
Contributor

Fixes #450

Other changes include:

  • Added many unit tests to test the new is_sdo() et al functions
  • stix2.version._detect_spec_version() is moved to stix2.utils.detect_spec_version(), to resolve a circular import problem. And it could be thought of as a general utility function anyway.
  • add stix2.registry.class_for_type(). This gives a nicer way to look up a stix2 library class from a STIX type string. This factored out some code from the versioning module, and is generally useful anywhere classes need to be looked up.
  • Change the internal class mapping data structure to be keyed at the top level by version strings in "X.Y" syntax, which is used by all public APIs, not the "vXY" syntax used for the python packages. This allows us to eliminate nearly all the ugly code which translated between the two syntaxes. All dependent code in this library is updated accordingly.
  • Update stix2.versioning.new_version() to enforce ID contributing property restrictions when versioning dicts (not just parsed stix2 objects).
  • Update stix2.versioning.new_version() to make use of the is_*() functions to simplify implementation and make it more readable
  • added unit tests for stix2.versioning.new_version() which exercise code which ensures callers can't change ID contributing properties of 2.1 SCOs when creating a new version.
  • Update stix2.parsing to make use of the new class_for_type() function, to simplify implementation and make it more readable
  • Remove some ugly python2 compatibility code from stix2.versioning

while I address circular import problems.
circular import refactoring.  I think I just forgot to include
this in the previous commit...
really just a simple accessor into the class maps table), and
change other code to use it, in places where it was simple and
made sense.
level with STIX versions in the same format as is used everywhere
else in the API: "X.Y", as opposed to the "vXY" format used by
the version-specific python packages.  This eliminates all of
the awkward conversion from public API format to "vXX" format.

Also a little bit of code rearranging in the registration module
to ensure that some STIX 2.1-specific checks are done whether
version 2.1 is given explicitly or is defaulted to.

In the same module I also added a missing import of
stix2.properties, since my IDE was claiming it could not find a
function from that module.
class map structure is keyed by normal "X.Y" style versions,
the convenience that function provided is no longer necessary.
So it no longer makes sense to have the function (at least,
not for that reason).  Change users of that function to use
the STIX2_OBJ_MAPS structure directly.
dict/mapping values: do a simple verification of the value's
STIX version, not just its type.  Added a lot more unit tests to
test behavior on dicts.  To make the implementation work, I had
to move the detect_spec_version() function out of the parsing
module and into utils.  So that required small changes at all
its previous call sites.
functions.  Changed some ">= 2.1" stix version semantics to be
"== 2.1", because we don't have any version >= 2.1, so they are
currently equivalent, and the is_*() functions don't support
STIX version ranges.  They only support exact versions.  We can
look at this again if a newer STIX version ever emerges.

Also added a class_for_type() function to the registry module,
which was useful for the versioning module changes described
above.  I thought that function would be helpful in the parsing
module, to simplify code there, so I changed that module a bit
to use it.
properties of a 2.1 SCO with UUIDv5 ID, when creating a new
version.
class in the registry when you have an instance of one of those
classes.  Because in that case, you can just get the class of the
instance and not deal with the registry at all.
module, since we no longer support python2.
@emmanvg emmanvg added this to the 3.0.0 milestone Jan 22, 2021
Copy link
Contributor

@clenk clenk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just a couple comments.

stix2/utils.py Show resolved Hide resolved
stix2/utils.py Outdated Show resolved Hide resolved
update to is_sro() as well, so it doesn't talk as if you can
register custom SROs.  That didn't actually make sense.
stix2/utils.py Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Jan 29, 2021

Codecov Report

Merging #483 (3878788) into master (03b3423) will increase coverage by 0.03%.
The diff coverage is 90.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #483      +/-   ##
==========================================
+ Coverage   89.27%   89.31%   +0.03%     
==========================================
  Files         146      147       +1     
  Lines       16032    16245     +213     
==========================================
+ Hits        14313    14509     +196     
- Misses       1719     1736      +17     
Impacted Files Coverage Δ
stix2/registry.py 22.22% <47.05%> (+22.22%) ⬆️
stix2/registration.py 81.17% <72.72%> (-0.85%) ⬇️
stix2/utils.py 78.74% <83.69%> (+5.41%) ⬆️
stix2/versioning.py 82.29% <90.47%> (+1.48%) ⬆️
stix2/parsing.py 80.00% <91.66%> (-4.32%) ⬇️
stix2/properties.py 74.33% <100.00%> (-0.07%) ⬇️
stix2/test/test_spec_version_detect.py 100.00% <100.00%> (ø)
stix2/test/test_utils_type_checks.py 100.00% <100.00%> (ø)
stix2/test/v20/test_custom.py 100.00% <100.00%> (ø)
stix2/test/v20/test_parsing.py 100.00% <100.00%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 03b3423...3878788. Read the comment docs.

Copy link
Contributor

@clenk clenk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @chisholm!

@clenk clenk merged commit 5e2d888 into oasis-open:master Jan 29, 2021
@chisholm chisholm deleted the is_functions branch January 29, 2021 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement is_sdo, is_sco utility functions
4 participants