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

feat(api-v2): Add metadata routes (DSP-662) #1734

Merged
merged 74 commits into from Oct 26, 2020
Merged

Conversation

@benjamingeer
Copy link
Collaborator

@benjamingeer benjamingeer commented Oct 15, 2020

Resolves DSP-662.

  • Refactor the plumbing in HttpTriplestoreConnector and TriplestoreMessages.scala
  • Refactor RouteUtilV2 and KnoraResponseV2 to handle this new kind of response message containing a Turtle string
  • Implement two-way conversion between JSON-LD and Turtle
  • Implement the metadata routes and messages
  • Implement metadataResponderV2
  • Add documentation

SHACL validation will be added in a later pull request.

The processing of Turtle requests and responses is a first draft, and will be refactored later for DSP-902.

SepidehAlassi and others added 7 commits Oct 14, 2020
- Move RDF formatting from RouteUtilV2 into KnoraResponseV2.
- Generate Turtle and XML from an RDF4J Model instead of parsing JSON-LD.
- Have JsonLDDocument convert itself to an RDF4J Model when needed.
- Refactor KnoraResponseV2 to have subclasses KnoraJsonLDResponseV2 and KnoraTurtleResponseV2.
benjamingeer and others added 19 commits Oct 16, 2020
…ataGetRequestV2, etc.
- Refactor JsonLDUtil for readability.
SepidehAlassi and others added 20 commits Oct 21, 2020
- Move RDF parsing/formatting utils to one place, RdfFormatUtil
- Use InternalSchema for Turtle responses
- Throw AssertionException if saved metadata is incorrect
- Include project shortcode in metadata named graph IRI
- Fix some error messages
@benjamingeer benjamingeer requested a review from subotic Oct 22, 2020
/**
* Constructs the default [[ResourcesResponderV2]].
*/
protected final def makeDefaultResourcesResponderV2: ResourcesResponderV2 = new ResourcesResponderV2(responderData)

/**
* Constructs the default [[ValuesResponderV2]].
* Subclasses of the can override this member to substitute a with a custom implementation instead of the default resources responder.

This comment has been minimized.

@subotic

subotic Oct 26, 2020
Collaborator

What does that mean?

This comment has been minimized.

@benjamingeer

benjamingeer Oct 26, 2020
Author Collaborator

The same typo was in all the existing comments in ResponderManager, we just copied and pasted it. :) Fixed now.

@@ -311,11 +322,10 @@ class ResponderManager(appActor: ActorRef) extends Actor with ActorLogging {
protected final def makeDefaultSipiResponderADM: SipiResponderADM = new SipiResponderADM(responderData)

/**
* Subclasses of the can override this member to substitute a with a custom implementation instead of the default resources responder.
* Subclasses can override this member to substitute it with a custom implementation instead of the default sipi responder.

This comment has been minimized.

@subotic

subotic Oct 26, 2020
Collaborator

I guess it should mean this?

Copy link
Collaborator

@subotic subotic left a comment

LGTM, Thanks!

The only thing that is missing is the route for getting all metadata of all projects. This can be done in another PR. Please open a YouTrack issue. Thanks!

"//webapi:test_library",
"@maven//:org_apache_jena_apache_jena_libs",
] + BASE_TEST_DEPENDENCIES_WITH_JSON,
)

This comment has been minimized.

@subotic

subotic Oct 26, 2020
Collaborator

Missing newline

"//webapi:test_library",
"@maven//:org_apache_jena_apache_jena_libs",
] + BASE_TEST_DEPENDENCIES,
)

This comment has been minimized.

@subotic

subotic Oct 26, 2020
Collaborator

Missing newline

"//webapi:test_library",
"@maven//:org_apache_jena_apache_jena_libs",
] + BASE_TEST_DEPENDENCIES,
)

This comment has been minimized.

@subotic

subotic Oct 26, 2020
Collaborator

Missing newline

prefix rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#>
prefix jedi: <http://jedi.org/#>
<http://luke> jedi:tries "force for the first time" ;

This comment has been minimized.

@subotic

subotic Oct 26, 2020
Collaborator

:-)

@benjamingeer
Copy link
Collaborator Author

@benjamingeer benjamingeer commented Oct 26, 2020

@subotic Thanks for the review!

The only thing that is missing is the route for getting all metadata of all projects.

I'm concerned that when we have hundreds of projects, returning all metadata for all projects in one API response could mean returning a huge API response that could put too much load on the triplestore. Let's talk about it first.

@benjamingeer benjamingeer merged commit bf48968 into develop Oct 26, 2020
8 checks passed
8 checks passed
Build Everything
Details
API Unit Tests
Details
API E2E Tests
Details
API Integration Tests
Details
Upgrade Integration Tests
Details
Docs Build Test
Details
Update next release draft
Details
Publish (on release only)
Details
@benjamingeer benjamingeer deleted the wip/DSP-662-MetadataRoutes branch Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants