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 support for fixing problems #4

Merged
merged 2 commits into from Jun 2, 2022
Merged

Conversation

pwinckles
Copy link
Contributor

What does this Pull Request do?

Adds support for fixing problems. Currently, this just means fixing binary descriptions with invalid subjects.

How should this be tested?

  1. Run a version of Fedora prior to this commit
  2. Create binary descriptions with subjects like <info:fedora/resource/fcr:metadata>
  3. Run fcrepo-doctor as described in the README
  4. Confirm that the fixed RDF is correct and that the resources are usable in Fedora

Interested parties

@fcrepo/committers

@@ -231,15 +287,15 @@ private OcflObjectSessionFactory createObjectSessionFactory(final MutableOcflRep
objectMapper,
new CaffeineCache<>(headersCache),
new CaffeineCache<>(rootIdCache),
// TODO does this need to be an option?
CommitType.NEW_VERSION,
disableAutoVersioning ? CommitType.UNVERSIONED : CommitType.NEW_VERSION,
Copy link

Choose a reason for hiding this comment

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

I haven't tried this yet, but if you have an auto-versioned repo and your disable autoversioning does that wreak havoc, same with vice-versa?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be fine. If you have auto-versioning disabled and you enable it here, then it just means that it will commit the mutable head and create a new version when it fixes a problem. Conversely, if your repo has it enabled, and you disable it here then it will apply the fix to a new mutable head and not create a version. Next time you update the resource through Fedora it would then create a new version as normal.

But, yes, generally speaking, you probably want to use the same settings as you're using in Fedora, and, even if you have auto-versioning disabled in Fedora, I personally would probably auto-version here.

Copy link

Choose a reason for hiding this comment

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

Ok I might still give it a try to see the outcome. I agree that in general running fcrepo-doctor with auto-versioning makes the most sense. It is nice to see the change in the filesystem neatly packaged.

> rocfl show info:fedora/binary1 6 
Version 6
Author:  fedoraAdmin <info:fedora/fedoraAdmin>
Date:    Sat, 21 May 2022 12:20:13 -0500
Message: Authored by Fedora 6

Operation Logical Path
Modified  .fcrepo/fcr-root.json
Modified  .fcrepo/fcr-root~fcr-desc.json
Modified  binary1~fcr-desc.nt

> rocfl show info:fedora/binary1 7
Version 7
Author:  fedoraAdmin <info:fedora/fedoraAdmin>
Date:    Sat, 21 May 2022 12:24:50 -0500
Message: Generated by fcrepo-doctor

Operation Logical Path
Modified  .fcrepo/fcr-root~fcr-desc.json
Modified  binary1~fcr-desc.nt

Copy link

@whikloj whikloj left a comment

Choose a reason for hiding this comment

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

This looks good and I think I understood all of it 🤯
Also works as expected.

Copy link

@whikloj whikloj left a comment

Choose a reason for hiding this comment

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

👍

@whikloj whikloj merged commit 7946ebc into fcrepo-exts:main Jun 2, 2022
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.

None yet

2 participants