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

Conversation

benjamingeer
Copy link

@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 October 14, 2020 15:34
- 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.
@SepidehAlassi SepidehAlassi added API/V2 enhancement improve existing code or new feature labels Oct 15, 2020
SepidehAlassi and others added 21 commits October 21, 2020 08:28
- 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
/**
* 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.
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does that mean?

Copy link
Author

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.
Copy link
Collaborator

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?

Copy link
Collaborator

@subotic subotic left a 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,
)
Copy link
Collaborator

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,
)
Copy link
Collaborator

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,
)
Copy link
Collaborator

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" ;
Copy link
Collaborator

Choose a reason for hiding this comment

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

:-)

@benjamingeer
Copy link
Author

@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
@benjamingeer benjamingeer deleted the wip/DSP-662-MetadataRoutes branch October 26, 2020 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API/V2 enhancement improve existing code or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants