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
Conversation
- 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.
…-swiss/knora-api into wip/DSP-662-MetadataRoutes
…ataGetRequestV2, etc.
…-swiss/knora-api into wip/DSP-662-MetadataRoutes
- Refactor JsonLDUtil for readability.
…-swiss/knora-api into wip/DSP-662-MetadataRoutes
- 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
…-swiss/knora-api into wip/DSP-662-MetadataRoutes
…-swiss/knora-api into wip/DSP-662-MetadataRoutes
/** | ||
* 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does that mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it should mean this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing newline
"//webapi:test_library", | ||
"@maven//:org_apache_jena_apache_jena_libs", | ||
] + BASE_TEST_DEPENDENCIES, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing newline
"//webapi:test_library", | ||
"@maven//:org_apache_jena_apache_jena_libs", | ||
] + BASE_TEST_DEPENDENCIES, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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" ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:-)
@subotic Thanks for the review!
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. |
Resolves DSP-662.
metadataResponderV2
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.