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.

Sign up for GitHub

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

Jump to bottom

Fix #105: "multiple conformsTo values" #111

Merged
merged 24 commits into from
Jun 23, 2023
Merged

Fix #105: "multiple conformsTo values" #111

merged 24 commits into from
Jun 23, 2023

Conversation

Pfeil
Copy link
Member

@Pfeil Pfeil commented May 5, 2023 •
edited
Loading

  • refactor the function which caused the issue
  • Support reading 1.2-DRAFT crates
    • Create a test which reads a 1.2-DRAFT Metadata file
    • implementation
    • Create a test which writes the crate after reading, showing that nothing changed
    • Test: Compare loaded crate from original and from copy
  • Support on crate creation to choose the specification version (1.2-DRAFT will after stabilization be the same as 1.2, we do not plan to support the creation of draft versions where stable/newer versions exist)
    • test
    • implementation
      • Note: ideally, something like "appendProfile(...)" would only be available to descriptors containing the 1.2(-DRAFT) profile.
  • Support modification of 1.2-DRAFT crates profiles (conformsTo array)
    • write test for modification
    • test: ensure we do modify it, not overwrite the descriptor
    • implementation
      • methods exist in builder
      • can put crate into v1.2 builder
  • Support a mechanism to upgrade from 1.1 to 1.2-DRAFT (as far as possible, here at least regarding the amount of conformsTo values, other changes may be supported in later PRs)
    • implementation is implied by modification
  • Update Readme about supported versions and examples

Further resources:

@Pfeil
refactor(reader): reduce complexity
ffaacf4 
- increase readability to make planned changes easier
- using streams and optional to avoid missed cases
- using node.path(...) to avoid too many json validation checks
@Pfeil Pfeil linked an issue May 5, 2023 that may be closed by this pull request
Multiple conformsTo values #105
Closed
@coveralls
Copy link

coveralls commented May 5, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build #149

  • 165 of 179 (92.18%) changed or added relevant lines in 7 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.09%) to 89.757%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/main/java/edu/kit/datamanager/ro_crate/special/JsonUtilFunctions.java 4 5 80.0%
src/main/java/edu/kit/datamanager/ro_crate/entities/contextual/JsonDescriptor.java 37 39 94.87%
src/main/java/edu/kit/datamanager/ro_crate/externalproviders/organizationprovider/RorProvider.java 8 10 80.0%
src/main/java/edu/kit/datamanager/ro_crate/RoCrate.java 32 35 91.43%
src/main/java/edu/kit/datamanager/ro_crate/externalproviders/personprovider/OrcidProvider.java 10 13 76.92%
src/main/java/edu/kit/datamanager/ro_crate/special/CrateVersion.java 21 24 87.5%
Files with Coverage Reduction New Missed Lines %
src/main/java/edu/kit/datamanager/ro_crate/externalproviders/organizationprovider/RorProvider.java 1 86.96%
Totals Coverage Status
Change from base Build #132: 0.09%
Covered Lines: 1402
Relevant Lines: 1562
💛 - Coveralls

@Pfeil Pfeil self-assigned this May 5, 2023
@Pfeil Pfeil added the enhancement New feature or request label May 5, 2023
@Pfeil
Copy link
Member Author

Pfeil commented May 9, 2023

Current state: I am not sure how to bring together the "algorithm" of v1.1 and v1.2 to find the root data entity. I already have a rough plan, but I am highly interested in an answer to my questions at ResearchObject/ro-crate#253.

@Pfeil Pfeil added this to the 1.1.0 milestone Jun 7, 2023
@sonatype-lift

This comment was marked as outdated.

Sign in to view
Pfeil and others added 2 commits June 20, 2023 15:21
@Pfeil @sonatype-lift
sonarlint: compare strings using equals
caee947 
Co-authored-by: sonatype-lift[bot] <37194012+sonatype-lift[bot]@users.noreply.github.com>
@Pfeil
feat: enable reading of crates with multiple conformsTo values
82322f4
@Pfeil Pfeil force-pushed the 105-multiple-conformsto-values branch 4 times, most recently from 18a3bd4 to 5ce54df Compare June 20, 2023 17:36
@Pfeil
Copy link
Member Author

Pfeil commented Jun 20, 2023 •
edited
Loading

Reading and writing crates works now. Modification of "conformsTo" values might result in crashes yet (not tested). I'll consider reducing the to-dos to a minimum and consider the removed ones to be an extra issue or so, so we get faster results for testing. But I think from here we should progress faster.

@Pfeil Pfeil force-pushed the 105-multiple-conformsto-values branch from 5ce54df to e81174b Compare June 21, 2023 16:21
@Pfeil Pfeil force-pushed the 105-multiple-conformsto-values branch from 4314e6d to 295a46a Compare June 23, 2023 13:59
@Pfeil
refactor: simplify crate builders
0d4fd82 
On purpose, we do not add a setVersion method, so it is easier to
maintain a lower version, if no new features are being used.
@Pfeil
fix(docs): Add missing summary
123f789
@Pfeil
docs: state compatibility in README.md
f6c7602
@Pfeil
feat: convenience methods for comparing versions
ad1e914
@Pfeil Pfeil force-pushed the 105-multiple-conformsto-values branch from 55e94e8 to ad1e914 Compare June 23, 2023 15:28
@Pfeil Pfeil marked this pull request as ready for review June 23, 2023 15:59
@Pfeil Pfeil merged commit 53aebf7 into main Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sonatype-lift sonatype-lift sonatype-lift left review comments

Assignees

@Pfeil Pfeil

Labels
enhancement New feature or request
Projects
None yet
Milestone
v1.1.0
Development

Successfully merging this pull request may close these issues.

Multiple conformsTo values
2 participants
@Pfeil @coveralls