refactor(api-v2): Use SparqlExtendedConstructResponse in ConstructResponseUtilV2 #1367

benjamingeer · 2019-07-02T11:43:35Z

This PR improves type safety in ConstructResponseUtilV2 by using SparqlExtendedConstructResponse, which provides the types returned by the triplestore.

Change SparqlExtendedConstructResponse to use SmartIri for predicates.
Refactor ConstructResponseUtilV2 and related classes.
Fix datatype typo in v2/generateInsertStatementsForValueContent.scala.txt (was xsd:valueHasDecimal, should be xsd:decimal).
Add upgrade script in case anyone was affected by that typo.
Add test.

Resolves #763.

…dedConstructResponse.

benjamingeer · 2019-07-03T08:36:20Z

@tobiasschweizer Issue #763 says to:

Take the language tag into account. But we don't use language tags in data in the triplestore, do we?
Not return a list node's label for a list value. This was done in Handle List Nodes in Response #1321, right?

tobiasschweizer · 2019-07-03T08:58:12Z

Take the language tag into account. But we don't use language tags in data in the triplestore, do we?

No, I don't think so. We do have a language property for text values (ISO language code) but it's not the same that is used for labels etc. in the ontology.

Not return a list node's label for a list value. This was done in #1321, right?

This was already done, yes.

tobiasschweizer · 2019-07-03T09:16:43Z

@benjamingeer I think I can review this PR next week. Would that be ok?

benjamingeer · 2019-07-03T09:17:53Z

I think I can review this PR next week. Would that be ok?

Sure, no problem. Would you have time to look at #1259 with @andreas-aeschlimann this week?

tobiasschweizer · 2019-07-03T09:19:32Z

Would you have time to look at #1259 with @andreas-aeschlimann this week?

He said he would look at it. I will remind him :-)

benjamingeer · 2019-07-03T09:20:02Z

I will remind him :-)

Thanks!

tobiasschweizer · 2019-07-08T08:16:05Z

The update script is needed in case someone created decimal values using v2, correct?

benjamingeer · 2019-07-08T08:29:27Z

The update script is needed in case someone created decimal values using v2, correct?

Yes.

webapi/src/main/scala/org/knora/webapi/responders/v2/SearchResponderV2.scala

tobiasschweizer · 2019-07-08T08:32:45Z

webapi/src/main/scala/org/knora/webapi/responders/v2/SearchResponderV2.scala

-                        case (pred, obj) =>
-                            pred == OntologyConstants.KnoraBase.IsMainResource && obj.toBoolean
+                    val subjectIsMainResource: Boolean = assertions.get(OntologyConstants.KnoraBase.IsMainResource.toSmartIri) match {
+                        case Some(objs) => objs.contains(BooleanLiteralV2(true))


Couldn't you just check for the first element with headOption? There should be only one statement IsMainResource.

Yes, you‘re right.

Done in 3d26cf3.

tobiasschweizer

I think it would make sense to add a basic test case for ConstructResponseUtilV2. Do you think that is feasible?

webapi/src/main/scala/org/knora/webapi/util/ConstructResponseUtilV2.scala

benjamingeer · 2019-07-08T12:33:08Z

I think it would make sense to add a basic test case for ConstructResponseUtilV2. Do you think that is feasible?

I could refactor HttpTriplestoreConnector to make it possible to parse Turtle and construct a SparqlExtendedConstructResponse without actually connecting to the triplestore. Then I could add a test that took some hard-coded Turtle, parsed it, and ran it through ConstructResponseUtilV2. Would that be what you have in mind?

tobiasschweizer · 2019-07-08T12:35:41Z

Would that be what you have in mind?

Yes, no actual connection the triplestore is needed. Also we can get an idea how much time complex results need for processing (nested resources).

benjamingeer · 2019-07-08T14:18:23Z

Yes, no actual connection the triplestore is needed.

Done in 3d26cf3.

Also we can get an idea how much time complex results need for processing (nested resources).

We can't really see this now, because ConstructResponseUtilV2 does additional SPARQL queries, e.g. to get project info.

tobiasschweizer

That looks great, thanks!

Unfortunately, I am not able to test any of this locally (see #1370).

Benjamin Geer added 3 commits July 1, 2019 12:24

refactor(store): Use SmartIri in SparqlExtendedConstructResponse.

4ab420d

refactor(api-v2): Refactor ConstructResponseUtilV2 to use SparqlExten…

440a2d6

…dedConstructResponse.

Merge branch 'develop' into wip/763-construct

f444a70

benjamingeer mentioned this pull request Jul 2, 2019

Ben's PR history #571

Open

feat(upgrade): Add update script to fix incorrect datatype.

9bcd0ff

benjamingeer requested a review from tobiasschweizer July 2, 2019 14:01

benjamingeer self-assigned this Jul 2, 2019

benjamingeer added API/V2 breaking (existing data) labels Jul 2, 2019

Benjamin Geer added 5 commits July 3, 2019 11:00

style(api-v2): Add type aliases for clarity.

1f7c34a

style(api-v2): Reformat code.

a8e7eae

style(api-v2): Add functions to make empty maps.

33d925e

style(api-v2): Use type aliases in function definitions.

2f63a17

style(api-v2): Use type alias in PermissionUtilADM.

a034abe

Benjamin Geer added 3 commits July 3, 2019 11:26

style(api-v2): Use type aliases in MainQueryResultProcessor.

32a1609

style(api-v2): Make code more concise.

529d330

Merge branch 'develop' into wip/763-construct

0dea87e

tobiasschweizer reviewed Jul 8, 2019

View reviewed changes

webapi/src/main/scala/org/knora/webapi/util/ConstructResponseUtilV2.scala Show resolved Hide resolved

test(ConstructResponseUtilV2): Add test.

3d26cf3

style(api-v2): Use type aliases.

a51e0d4

tobiasschweizer approved these changes Jul 8, 2019

View reviewed changes

Merge branch 'develop' into wip/763-construct

0b01ba3

benjamingeer merged commit cf8f23d into develop Jul 9, 2019

benjamingeer deleted the wip/763-construct branch July 9, 2019 12:42

benjamingeer mentioned this pull request Jul 9, 2019

fix(upgrade): Update knora-admin comments #1371

Merged

tobiasschweizer mentioned this pull request Aug 26, 2019

Problem with interval value #1399

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(api-v2): Use SparqlExtendedConstructResponse in ConstructResponseUtilV2 #1367

refactor(api-v2): Use SparqlExtendedConstructResponse in ConstructResponseUtilV2 #1367

benjamingeer commented Jul 2, 2019 •

edited

Loading

benjamingeer commented Jul 3, 2019

tobiasschweizer commented Jul 3, 2019

tobiasschweizer commented Jul 3, 2019

benjamingeer commented Jul 3, 2019

tobiasschweizer commented Jul 3, 2019

benjamingeer commented Jul 3, 2019

tobiasschweizer commented Jul 8, 2019

benjamingeer commented Jul 8, 2019

tobiasschweizer Jul 8, 2019

benjamingeer Jul 8, 2019

tobiasschweizer Jul 8, 2019

benjamingeer Jul 8, 2019

tobiasschweizer left a comment

benjamingeer commented Jul 8, 2019

tobiasschweizer commented Jul 8, 2019

benjamingeer commented Jul 8, 2019

tobiasschweizer left a comment

refactor(api-v2): Use SparqlExtendedConstructResponse in ConstructResponseUtilV2 #1367

refactor(api-v2): Use SparqlExtendedConstructResponse in ConstructResponseUtilV2 #1367

Conversation

benjamingeer commented Jul 2, 2019 • edited Loading

benjamingeer commented Jul 3, 2019

tobiasschweizer commented Jul 3, 2019

tobiasschweizer commented Jul 3, 2019

benjamingeer commented Jul 3, 2019

tobiasschweizer commented Jul 3, 2019

benjamingeer commented Jul 3, 2019

tobiasschweizer commented Jul 8, 2019

benjamingeer commented Jul 8, 2019

tobiasschweizer Jul 8, 2019

Choose a reason for hiding this comment

benjamingeer Jul 8, 2019

Choose a reason for hiding this comment

tobiasschweizer Jul 8, 2019

Choose a reason for hiding this comment

benjamingeer Jul 8, 2019

Choose a reason for hiding this comment

tobiasschweizer left a comment

Choose a reason for hiding this comment

benjamingeer commented Jul 8, 2019

tobiasschweizer commented Jul 8, 2019

benjamingeer commented Jul 8, 2019

tobiasschweizer left a comment

Choose a reason for hiding this comment

benjamingeer commented Jul 2, 2019 •

edited

Loading