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

Switch from assembly to shade plugin to correct Jena re-packaging #189

Merged
merged 2 commits into from
Oct 13, 2022

Conversation

whikloj
Copy link
Contributor

@whikloj whikloj commented Aug 16, 2022

JIRA Ticket: https://fedora-repository.atlassian.net/browse/FCREPO-3836

What does this Pull Request do?

Uses the maven shade plugin to propertly re-package Jena according to https://jena.apache.org/documentation/notes/jena-repack.html and generate the runnable jar

This PR replaces #188

How should this be tested?

This should have no impact or usage except to fix the issue described below.

Previously if you had a Fedora 3 object with a RELS-EXT that contained an element with a xml:lang tag

<rdf:RDF xmlns:METS="http://www.loc.gov/METS/" xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
  <rdf:Description rdf:about="info:fedora/example:1">
    <cc:useGuidelines xmlns:cc="http://web.resource.org/cc/" xml:lang="en">Some string</cc:useGuidelines>
  </rdf:Description>
</rdf:RDF>

it would get serialized with both the language tag and datatypeUri.
i.e.

<info:fedora/example:1> <http://some.tag/theTag> "Some string"@en^^<http://www.w3.org/1999/02/22-rdf-syntax-ns#langString> .

Now it will only be the language tag.

<info:fedora/example:1> <http://some.tag/theTag> "Some string"@en .

Additional Notes:

Any additional information that you think would be helpful when reviewing this PR.

Example:

  • Does this change require documentation to be updated?
  • Does this change add any new dependencies?
  • Does this change require any other modifications to be made to the repository (ie. Regeneration activity, etc.)?
  • Could this change impact execution of existing code?

Interested parties

Tag (@ mention) interested parties or, if unsure, @fcrepo/committers

@Surfrdan
Copy link
Contributor

I'll try and take a look at this ASAP. I think our Systems users are currently utilising the previous migration dataset so I'll need to interrupt their service to replace it with a new copy.

Copy link
Contributor

@mikejritter mikejritter left a comment

Choose a reason for hiding this comment

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

@whikloj Looks good, I was able to run a migration with the xml:lang tag and see that it was serialized correctly. All the classes are in the Jena services file as well so we can see it's being shaded.

I just had a minor question about the fedora3.xml resource which shouldn't affect the PR.

pom.xml Outdated
</manifestEntries>
</transformer>
<transformer implementation="org.apache.maven.plugins.shade.resource.IncludeResourceTransformer">
<resource>conf/fedora3.xml</resource>
Copy link
Contributor

Choose a reason for hiding this comment

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

Building from main I don't see this in the artifact, is this something we had missed from before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is a good question, I was looking at the assembly plugin zip.xml file which appears to be including the contents of the conf directory in the packaged result. But if this file is not necessary I can remove that.

@dbernstein dbernstein merged commit b851ad0 into fcrepo-exts:main Oct 13, 2022
whikloj added a commit that referenced this pull request Feb 16, 2023
* added test for DISABLED digest type

* Adding Integration Test and a new exceptin for disabled digests

* checkstyle fixes

* changing index to match inline-it et al

* seperating test index in case of clash

* fixing issues with the disabled integration test

* removing superflous log message as we throw an exception now

* git wasn't adding an empty dir. used .gitkeep and now the test should parse tge datastream dir

* .gitkeep broke the indexer. copying a datastream from another test

* reverting to logging rather than throwing exception

* improved DISABLED fixity Integration Test to use fcrepo-storage-ocfl session object

* Switch from assembly to shade plugin to correct Jena re-packaging (#189)

Resolves:  https://fedora-repository.atlassian.net/browse/FCREPO-3836

* Alter test assertion to allow for ocfl 1.0 and 1.1 (#192)

Resolves: https://fedora-repository.atlassian.net/browse/FCREPO-3863

* Bump woodstox-core from 6.2.3 to 6.4.0 (#191)

Bumps [woodstox-core](https://github.com/FasterXML/woodstox) from 6.2.3 to 6.4.0.
- [Release notes](https://github.com/FasterXML/woodstox/releases)
- [Commits](FasterXML/woodstox@woodstox-core-6.2.3...woodstox-core-6.4.0)

---
updated-dependencies:
- dependency-name: com.fasterxml.woodstox:woodstox-core
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Increment version to 6.3.1-SNAPSHOT

* fixing issues with the disabled integration test

* removing superflous log message as we throw an exception now

* git wasn't adding an empty dir. used .gitkeep and now the test should parse tge datastream dir

* .gitkeep broke the indexer. copying a datastream from another test

* reverting to logging rather than throwing exception

* improved DISABLED fixity Integration Test to use fcrepo-storage-ocfl session object

* fixed test to look for a DISABLED digest

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Dan Field <[email protected]>
Co-authored-by: Dan Field <[email protected]>
Co-authored-by: Jared Whiklo <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Daniel Bernstein <[email protected]>
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.

4 participants