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

Create image file values in API v2 #1011

Merged
merged 44 commits into from
Nov 28, 2018
Merged

Create image file values in API v2 #1011

merged 44 commits into from
Nov 28, 2018

Conversation

benjamingeer
Copy link

@benjamingeer benjamingeer commented Oct 17, 2018

As per #998 (comment):

  • Add Sipi routes to process image file uploads.
    • upload.lua: called by client, for uploading, image conversion, and storage in Sipi's tmp dir
      • should automatically delete old files in tmp (recursively in subdirectories)
    • store.lua: called by Knora, for moving a file from tmp to permanent storage dir
    • delete_temp_file.lua: called by Knora, for deleting a file from tmp
    • these routes must:
      • check JWT tokens:
        • Is the token valid?
        • Is it signed by Knora?
        • Is it issued by Knora for an audience that includes Sipi?
        • Does it give Sipi permission to perform the requested operation (filename + permission)?
      • support using subdirectories in the temp dir
      • support using subdirectories in the permanent storage dir
  • Refactor ReadValueV2 so it has two subtypes, one for links and one for non-links (simplifies some code).
  • Implement file value creation in API v2.
    • When processing a JSON-LD request representing a file value to be created/updated, ask Sipi for the file's metadata (using Sipi's built-in knora.json route).
  • Have Knora send appropriate JWT tokens to Sipi.
  • If Knora rejects a request, it should have Sipi delete the temporary file.
  • Remove deprecated properties isPreview and qualityLevel from API v2.
  • Make originalMimeType and originalFilename required in knora-base:FileValue (seems to be a mistake that they weren't, see Question regarding the handling of optional values in SPARQL templates #342).
  • Add responder tests, with mock Sipi that pretends to move files to permanent storage.
    • values responder
    • resources responder
    • Test that Knora returns a Sipi error if Sipi fails to move the file to permanent storage after Knora has already updated the triplestore.
    • Test that Knora does not return a Sipi error if Knora rejects a request and Sipi fails to delete the temporary file (Knora returns its own error saying why it rejected the request, and the Sipi error is simply logged).
  • Add ValuesV2R2RSpec to check JSON-LD processing, getting image metadata from mock Sipi
  • Add integration tests with Sipi.
    • Test JWT validation.
    • Test creating a file value.
    • Test deleting temporary file if file value creation fails.
    • Test changing a file value.
    • Test creating a resource with file values.
  • Correct strings in responses that should be xsd:anyURI (need to be fixed in Core module: update textValueHasMapping knora-ui#87):
    • knora-api:fileValueAsUrl
    • knora-api:stillImageFileValueHasIIIFBaseUrl
  • Have API v1 ignore preview image file values in the triplestore and generate IIIF URLs instead.
  • Add docs.
    • API docs
    • design docs
  • Update release notes.

Fixes #342.
Resolves #1044.

Requires Sipi with tagged commit v1.4.1-SNAPSHOT or later.

For a subsequent PR:

This was referenced Oct 17, 2018
- Remove deprecated properties isPreview and qualityLevel from API v2.
@subotic
Copy link
Collaborator

subotic commented Oct 23, 2018

@benjamingeer since you're working on the Sipi Lua scripts, could you maybe implement #874?

@benjamingeer
Copy link
Author

@subotic Yes, I'll try to do that, too.

@subotic
Copy link
Collaborator

subotic commented Oct 23, 2018

thanks :-)

@benjamingeer benjamingeer changed the title Create file values in API v2 Create image file values in API v2 Oct 31, 2018
@dasch-swiss dasch-swiss deleted a comment Nov 2, 2018
@dasch-swiss dasch-swiss deleted a comment Nov 2, 2018
@dasch-swiss dasch-swiss deleted a comment Nov 2, 2018
@dasch-swiss dasch-swiss deleted a comment Nov 5, 2018
@dasch-swiss dasch-swiss deleted a comment Nov 5, 2018
@benjamingeer
Copy link
Author

@tobiasschweizer @subotic I changed API v1 to generate IIIF preview URLs, ignoring the preview image file values in the triplestore. I updated the test data so the tests pass. I browsed around in the SALSAH 1 GUI and it looks like it works (IIIF preview images are requested and displayed), but I haven't been able to get the SALSAH 1 GUI tests to work, because of timeouts and other errors that I don't understand.

@tobiasschweizer
Copy link
Contributor

@loicjaouen had the same problem (
#1018 (comment)).

My guess is that there were ontology changes that break the tests. In the tests, everything works with positions (n-th property).

If you are here next week, we could have a look at it.

@benjamingeer
Copy link
Author

If you are here next week, we could have a look at it.

Yes, thanks, I'll be there.

@tobiasschweizer
Copy link
Contributor

Ok, great. Mostly it is really trivial. You just have to adapt the positions. But it is very cumbersome since the GUI was written in a testing-hostile manner ;-)

@subotic
Copy link
Collaborator

subotic commented Nov 26, 2018

The BEOL Import Test run without problems. 2000 letters with images where imported.

Benjamin Geer added 2 commits November 26, 2018 15:37
# Conflicts:
#	docs/src/paradox/00-release-notes/v3.x.x.md
#	webapi/src/main/scala/org/knora/webapi/responders/v2/ValuesResponderV2.scala
@benjamingeer benjamingeer requested review from subotic and removed request for subotic November 27, 2018 11:24
sipi/scripts/jwt.lua Outdated Show resolved Hide resolved
sipi/scripts/upload.lua Show resolved Hide resolved
sipi/scripts/jwt.lua Show resolved Hide resolved
sipi/scripts/upload.lua Outdated Show resolved Hide resolved
@benjamingeer
Copy link
Author

@subotic I think this is OK to merge now.

@benjamingeer benjamingeer merged commit 14e17df into develop Nov 28, 2018
@benjamingeer benjamingeer deleted the wip/v2-file-values branch November 28, 2018 12:27
@benjamingeer benjamingeer mentioned this pull request Feb 26, 2019
9 tasks
@subotic
Copy link
Collaborator

subotic commented May 23, 2019

This pull request has been mentioned on Discuss DaSCH. There might be relevant details there:

https://discuss.dasch.swiss/t/support-non-image-files-in-knora-api-v2/33/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace deprecated JWT library Question regarding the handling of optional values in SPARQL templates
3 participants