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

DRYD-1399: RestrictedMedia #428

Open
wants to merge 16 commits into
base: v8.1-branch
Choose a base branch
from

Conversation

mikejritter
Copy link
Contributor

What does this do?

  • Add a RestrictedMedia client and service
  • Add validator handler for RestrictedMedia
  • Add document model handler for RestrictedMedia
  • Add restricted media id generator
  • Add restrictedmedia xsd

Why are we doing this? (with JIRA link)
Jira: https://collectionspace.atlassian.net/browse/DRYD-1399

This adds a new type of Media procedure, RestrictedMedia, which is intended for use in workflows where there may be sensitive media which should not be immediately accessible. Currently it functions the same as the Media service, with the same logic for the Resource, Client, Proxy, and DocumentHandler classes.

How should this be tested? Do these changes have associated tests?

  • Re-build and deploy collectionspace
  • Start collectionspace
  • Test that the restrictedmedia service is accessible, e.g.
curl -u '[email protected]:Administrator' http://localhost:8180/cspace-services/restrictedmedia/
  • Run the integration tests

Dependencies for merging? Releasing to production?
RestrictedMediaResource#createBlob is deprecated and likely doesn't make sense to carry over to a new service. It seems like the original deprecation (in MediaResource) was just in reference to using PUT vs POST.

I had been considering doing all the Blob handling for RestrictedMedia in the new Resource as well, but I opted to keep this simple for now as getting things setup to access the persistence layer was not as straightforward as I was expecting.

Has the application documentation been updated for these changes?
No

Did someone actually run this code to verify it works?
@mikejritter tested with the old PR, not new

@mikejritter mikejritter changed the base branch from master to v8.1-branch October 29, 2024 18:35
@axb21
Copy link

axb21 commented Nov 19, 2024

I deployed CollectionSpace locally with this PR as well as collectionspace/application#301. I verified both curl -u '[email protected]:PASSWORD' http://localhost:8180/cspace-services/restrictedmedia/ and curl -u '[email protected]:PASSWORD' http://localhost:8180/cspace-services/restrictedmedia/ return the expected results.

mvn test -fae returns one test failure as far as I can see:

[INFO] services.person.client ............................. FAILURE

That test failed before I installed the PR so I don't think the PR is responsible.

I have not looked into the merge conflicts that GitHub is reporting. @mikejritter will you do that?

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.

2 participants