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

Check that all internal references are valid references to other prod… #347

Merged
merged 13 commits into from
May 28, 2021

Conversation

qchaupds
Copy link
Contributor

@qchaupds qchaupds commented May 18, 2021

…ucts within the current bundle

Should check that all internal references are valid references to other products within the current bundle.
If the logical_identifier of theproduct containing the said reference does NOT belong to the bundle, the WARNING about
non-existent identifier is not raised but other WARNINGs may be raised, perhaps not being a member of any collection.

Closes #308

Tested in DEV for #308

There should be no WARNING regarding a reference pointing to a logical identifier.
% validate -R pds4.bundle -r report_github308_bundle_valid.json -s json -t src/test/resources/github308/valid/bundle_kaguya_derived.xml

There should be 2 warnings for referencing a logical identifier for a product not found in this bundle for label bundle_kaguya_derived.xml
There should be 1 warning for referencing a logical identifier for a product not found in this bundle for label kgrs_calibrated_spectra_per1.xml

% validate -R pds4.bundle -r report_github308_bundle_invalid.json -s json -t src/test/resources/github308/invalid/bundle_kaguya_derived.xml

Qui T Chau and others added 7 commits May 18, 2021 14:35
…ucts within the current bundle

1. Add test resources for github308 to src/test/resources
2. Add functions to support parsing for lid_reference, lidvid_reference and logical_identifier tags and move some constants from function to private class variables for readability in LabelUtil.java
3. Add new check if a reference is pointing to logical_identifier not in the current bundle in BundleReferentialIntegrityRule.java
4. Add debugs to CollectionReferentialIntegrityRule.java
5. Add new tests and update github51 message count in validate.feature

Refs:

#308 As a user, I want to check that all Internal References are valid references to other PDS4 products within the current validating bundle
Copy link
Member

@jordanpadams jordanpadams left a comment

Choose a reason for hiding this comment

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

@qchaupds looks good. a couple changes requested.

additionally, for future issues, we need to think about our test cases a bit. i appreciate the diligence in providing a valid/invalid test case for each bug fix, but our test data is getting huge (~62M). we can start with whatever data we need to perform our testing, but if the test data already exists, or another test already accomplishes one of our test cases, we should just call that out in the PR. if it isn't obvious that another test case meets our needs, we can totally just add more data. not a huge deal. just something to keep in mind.

for instance, we already have this kaguya data elsewhere in the test suite, and we definitely already have a valid bundle run somewhere in here. we can just re-use that test data, or even those test cases, in the pull request.

jordanpadams and others added 3 commits May 19, 2021 08:57
…eference or lidvid_reference to map to a logical_identifier

1. Add getIdentifiersCommon() function to refactoring in LabelUtil.java
2. Use lid_reference or lidvid_reference to map to a logical_identifier instead of using a filename in BundleReferentialIntegrityRule.java
3. Remove slash when checking for combination of two string to avoid confusion in BundleReferentialIntegrityRule.java

Refs:

#308 As a user, I want to check that all Internal References are valid references to other PDS4 products within the current validating bundle
Copy link
Member

@jordanpadams jordanpadams 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. thanks @qchaupds !

@jordanpadams jordanpadams merged commit 18e5c56 into master May 28, 2021
@jordanpadams jordanpadams deleted the #308 branch May 28, 2021 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants