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

feature(api-v2): Generate client test data for ontologies, resources, search, and lists #1549

Merged
merged 15 commits into from Dec 12, 2019

Conversation

benjamingeer
Copy link

@benjamingeer benjamingeer commented Dec 6, 2019

This PR incorporates more API v2 routes into the client code generation framework so they can return client test data.

  • ontologies route
  • resources route
  • search route
  • lists route

Resolves #1548.

@benjamingeer benjamingeer self-assigned this Dec 6, 2019
@benjamingeer benjamingeer added API/V2 clientapi frontend: Salsah, DSP-APP, BEOL, MLS, etc. labels Dec 6, 2019
@benjamingeer benjamingeer added this to the 2019-12 milestone Dec 6, 2019
@benjamingeer benjamingeer changed the title feature(api-v2): Generate clientapi test data for resources, ontologies, and lists feature(api-v2): Generate clientapi test data for ontologies, resources, search, and lists Dec 9, 2019
@benjamingeer benjamingeer changed the title feature(api-v2): Generate clientapi test data for ontologies, resources, search, and lists feature(api-v2): Generate client test data for ontologies, resources, search, and lists Dec 9, 2019
@tobiasschweizer
Copy link
Contributor

In v2 test data for resources, I cannot find JSON data for creation and metadata update ops.

@tobiasschweizer
Copy link
Contributor

@tobiasschweizer
Copy link
Contributor

I think the the values themselves do not have to be tested thoroughly because this is already done in https://github.com/dasch-swiss/knora-api-js-lib/blob/wip/create-resource/src/api/v2/values/values-endpoint.spec.ts with test data generated by Knora.

@benjamingeer
Copy link
Author

In v2 test data for resources, I cannot find JSON data for creation and metadata update ops.

I just included the stuff you asked for 😜:

#1546 (comment)

@benjamingeer
Copy link
Author

Will add test data for the update operations, too.

@tobiasschweizer
Copy link
Contributor

Will add test data for the update operations, too.

Thanks!

@tobiasschweizer
Copy link
Contributor

In the meantime I am changing from static to generated v2 test data files. Works already for the ontologies! :-)

My goal is to get rid of static test data at all.

@tobiasschweizer
Copy link
Contributor

there is a failing test in ExceptionHandlerR2RSpec: https://github.com/dasch-swiss/knora-api/pull/1549/checks?check_run_id=341570711#step:6:1310

override val description: String = "An endpoint for working with Knora lists."
override val functions: Seq[ClientFunction] = Seq.empty

def knoraApiPath: Route = getList ~ getNode
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment that this is actually the route definition?

private val ALL_LANGUAGES = "allLanguages"
private val LAST_MODIFICATION_DATE = "lastModificationDate"

def knoraApiPath: Route = {
def knoraApiPath: Route = dereferenceOntologyIri ~ getOntologyMetadata ~ updateOntologyMetadata ~ getOntologyMetadataForProjects ~
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment that this is actually the route definition?

@@ -127,138 +147,61 @@ class ResourcesRouteV2(routeData: KnoraRouteData) extends KnoraRoute(routeData)
}
}

def knoraApiPath: Route = createResource ~ updateResourceMetadata ~ getResourcesInProject ~
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment that this is actually the route definition?

private val LIMIT_TO_PROJECT = "limitToProject"
private val LIMIT_TO_RESOURCE_CLASS = "limitToResourceClass"
private val OFFSET = "offset"
private val LIMIT_TO_STANDOFF_CLASS = "limitToStandoffClass"

def knoraApiPath: Route = fullTextSearchCount ~ fullTextSearch ~ gravsearchCountGet ~ gravsearchCountPost ~
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment that this is actually the route definition?

@benjamingeer
Copy link
Author

there is a failing test in ExceptionHandlerR2RSpec

That's not a failing test, the test is supposed to throw an exception. The GitHub CI build is actually failing here, but I don't know why:

https://github.com/dasch-swiss/knora-api/pull/1549/checks?check_run_id=341570711#step:6:2012

@benjamingeer
Copy link
Author

Could you add a comment that this is actually the route definition?

I've added it to all routes.

I added more test requests/responses for the resources route. Do you want update requests for the ontologies route too?

@tobiasschweizer
Copy link
Contributor

I've added it to all routes.

Great, thanks.

I added more test requests/responses for the resources route. Do you want update requests for the ontologies route too?

Yes, we will need that too.

Could you also create JSON test data for a resource preview (response to a creation request)?

@tobiasschweizer
Copy link
Contributor

The preview JSON data has a higher priority to finish dasch-swiss/dsp-js-lib#115, so if you need more time for the ontologies and want to do this in a subsequent PR, that's also fine.

@benjamingeer
Copy link
Author

if you need more time for the ontologies and want to do this in a subsequent PR

Yes, because I'd like to finish #1509 first.

@tobiasschweizer
Copy link
Contributor

When I access http://localhost:3333/clientapi/typescript I now get

{
"error": "akka.http.scaladsl.model.IllegalUriException: Illegal URI reference: Invalid input '/', expected userinfo-char, pct-encoded or '@' (line 1, column 36): http://0.0.0.0:3333resourcespreview/http%3A%2F%2Frdfh.ch%2F0001%2Fa-thing\n ^"
}

@tobiasschweizer
Copy link
Contributor

tobiasschweizer commented Dec 11, 2019

"anything:hasDecimal" : {
    "@type" : "knora-api:DecimalValue",
    "knora-api:decimalValueAsDecimal" : {
      "@type" : "x  sd:decimal",
      "@value" : "100000000000000.000000000000001"
    }
  }

I think this is too much precision, JS serializes this to "100000000000000".

@tobiasschweizer
Copy link
Contributor

Maybe I could look at https://github.com/royNiladri/js-big-decimal or some similar lib.

@tobiasschweizer
Copy link
Contributor

if possible, could you also generate data for

  • update a resource's metadata submitting the last modification date
  • response to metadata update
  • response to resource deletion

@benjamingeer
Copy link
Author

benjamingeer commented Dec 11, 2019

"error": "akka.http.scaladsl.model.IllegalUriException

Sorry, stupid mistake.

@tobiasschweizer
Copy link
Contributor

When I try to generate client api code I now get:

{"error":"org.knora.webapi.ClientApiGenerationException: Failed to get test data: The requested resource could not be found."}

However, I can get http://localhost:3333/v2/resourcespreview/http%3A%2F%2Frdfh.ch%2F0001%2Fa-thing

@@ -376,7 +376,7 @@ class ResourcesRouteV2(routeData: KnoraRouteData) extends KnoraRoute(routeData)

private def getResourcesPreviewTestResponse: Future[SourceCodeFileContent] = {
for {
responseStr <- doTestDataRequest(Get(s"${baseApiUrl}resourcespreview/${SharedTestDataADM.AThing.iriEncoded}"))
responseStr <- doTestDataRequest(Get(s"$baseApiUrl/resourcespreview/${SharedTestDataADM.AThing.iriEncoded}"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Shall I fix it?

Copy link
Author

Choose a reason for hiding this comment

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

I'm doing it now along with the other stuff you asked for.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, thank you.

@benjamingeer
Copy link
Author

org.knora.webapi.ClientApiGenerationException

Yet another stupid mistake. I think I need to hibernate for the winter.

Can I change the file extensions to .jsonld instead of .json, or would that be a pain?

@tobiasschweizer
Copy link
Contributor

Can I change the file extensions to .jsonld instead of .json, or would that be a pain?

I experienced some problems earlier with that extension in the past when loading the files in a JS env. But we can give it a try in a separate PR. I am supposed to publish a new version of knora-api-js-lib today for @lrosenth, so I just want to finish resource creation.

@benjamingeer
Copy link
Author

OK please try it now.

@tobiasschweizer
Copy link
Contributor

Ok, that's all I need to finish dasch-swiss/dsp-js-lib#115, thanks!

I spotted a potential issue for the automatic integration tests in Knora we want to set up (#1554): some of the data is dynamic (such as creation / modification dates) and the tests will fail because the assertions are hard-coded (but they won't fail because of an actual problem we would have to fix).

@benjamingeer
Copy link
Author

some of the data is dynamic (such as creation / modification dates)

I think I've fixed this now. Let me know if it's OK to merge.

@tobiasschweizer
Copy link
Contributor

I think I've fixed this now. Let me know if it's OK to merge.

Works :-)

@benjamingeer
Copy link
Author

Works :-)

Did you forget to press the "Approve" button, or is there something else I need to do?

Copy link
Contributor

@tobiasschweizer tobiasschweizer left a comment

Choose a reason for hiding this comment

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

Ok, I did a final check. Good to go!

@benjamingeer
Copy link
Author

Thanks for your patience!

@tobiasschweizer
Copy link
Contributor

Thanks for your patience!

No worries!

@tobiasschweizer
Copy link
Contributor

Once this is merged, could you update the branches for the modification date and the time value in Knora. I then have to regenerate the client data from these branches to integrate them in the PR is knora-api-js-lib.

@benjamingeer benjamingeer merged commit aa6b773 into develop Dec 12, 2019
@benjamingeer benjamingeer deleted the wip/1548-clientapi-testdata branch December 12, 2019 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API/V2 clientapi frontend: Salsah, DSP-APP, BEOL, MLS, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add v2 clientapi test data for resources, ontologies, and lists
2 participants