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.

Sign up for GitHub

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

Jump to bottom

feat: Add check for can a cardinality be set for specific class and property #2382

Merged
merged 22 commits into from Jan 16, 2023
Merged

feat: Add check for can a cardinality be set for specific class and property #2382

merged 22 commits into from Jan 16, 2023

Conversation

seakayone
Copy link
Collaborator

@seakayone seakayone commented Jan 13, 2023 •
edited

Pull Request Checklist

Task Description/Number

Issue Number: DEV-1563

Extend existing route
(1)GET v2/ontologies/canreplacecardinalities/{classIri}
with
(2)GET v2/ontologies/canreplacecardinalities/{classIri}?propertyIri={iriEncode}&newCardinality=[0-1|1|1-n|0-n]

In the (1) case the check is done as previously, i.e. checks if the classIri is in use.
In the (2) case the api now checks whether for the the classIri/propertyIri combination it is save to set the newCardinality. It is safe only if no super class exists with a stricter cardinality on this property.

Basic Requirements

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
    • Currently only the check is implemented. I plan to document the feature once the api allows for setting the widened cardinalities in a followup PR.

PR Type

What kind of change does this PR introduce?

  • Bugfix: represents bug fixes
  • Refactor: represents production code refactoring
  • Feature: represents a new feature
  • Documentation: documentation changes (no production code change)
  • Chore: maintenance tasks (no production code change)
  • Style: styles updates (no production code change)
  • Test: all about tests: adding, refactoring tests (no production code change)
  • Other... Please describe:

Does this PR introduce a breaking change?

  • Yes
  • No

Does this PR change client-test-data?

  • Yes (don't forget to update the JS-LIB team about the change)
  • No

@seakayone seakayone force-pushed the wip/widen-cardinality-rebased branch from f231f43 to 12e6c64 Compare January 13, 2023 10:21
@codecov
Copy link

codecov bot commented Jan 13, 2023 •
edited

Codecov Report

Base: 86.48% // Head: 8.49% // Decreases project coverage by -77.98% ⚠️

Coverage data is based on head (e34d19c) compared to base (73a18ff).
Patch coverage: 48.19% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #2382       +/-   ##
==========================================
- Coverage   86.48%   8.49%   -77.99%     
==========================================
  Files         271     278        +7     
  Lines       28336   28395       +59     
==========================================
- Hits        24506    2412    -22094     
- Misses       3830   25983    +22153
Impacted Files Coverage Δ
...rc/main/scala/dsp/schema/domain/SchemaDomain.scala 100.00% <ø> (ø)
dsp-shared/src/main/scala/dsp/errors/Errors.scala 5.00% <0.00%> (-45.00%) ⬇️
...la/org/knora/webapi/core/actors/RoutingActor.scala 0.00% <0.00%> (-96.30%) ⬇️
...tore/triplestoremessages/TriplestoreMessages.scala 6.03% <0.00%> (-68.75%) ⬇️
...ages/util/search/gravsearch/GravsearchParser.scala 0.00% <ø> (-68.14%) ⬇️
...webapi/messages/v2/responder/KnoraResponseV2.scala 0.00% <0.00%> (-85.19%) ⬇️
...esponder/ontologymessages/OntologyMessagesV2.scala 2.32% <ø> (-84.55%) ⬇️
...org/knora/webapi/responders/ActorToZioBridge.scala 0.00% <ø> (ø)
...a/webapi/responders/EntityAndClassIriService.scala 0.00% <ø> (-92.00%) ⬇️
.../scala/org/knora/webapi/responders/IriLocker.scala 0.00% <ø> (-88.89%) ⬇️
... and 233 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@seakayone seakayone force-pushed the wip/widen-cardinality-rebased branch from 12e6c64 to 3d662a0 Compare January 13, 2023 10:35
@seakayone
Move knora-ontologies to webapi/src/main/resources and keep symbolic …
06068eb 
…link to folder in project root

Remove unused parameter `appConfig` from org.knora.webapi.util.ActorUtil#zio2Message

cleanup code: remove unused code, simplify

Revert "Move knora-ontologies to webapi/src/main/resources and keep symbolic link to folder in project root"

This reverts commit 48f5e93b7e443a6867baa257db2a076e1df202b6.

fmt

Fix warnings in CardinalityHandler

Overload case class constructor and simplify SparqlAskRequest construction

Use SmartIri type in query

fix scaladoc

use InternalIri as parameter

use InternalIri in CardinalitiesSpec

wip // add isPropertyUsedInResources to CardinalityService

wip // add TriplestoreServiceFake

wip // add CardinalityService

wip // add CardinalityService

wip // add CardinalityService

wip // add canWidenCardinality method to CardinalityService

wip // add canWidenCardinality method to CardinalityService

wip // add canWidenCardinality method to CardinalityService

fmt

reorganize TestDatasetBuilder

add headers

cleanup

cleanup

cleanup

wip // Cardinality model

fmt

reorganize packages

fix compile error and turn Cardinality into sealed trait

wip add test

wip add failing tests

wip add failing tests

add OntologyCache and fake implementation

add methods to IriConverter

add methods to IriConverter

add check and tests

add check and tests

Add headers

Update webapi/src/it/scala/org/knora/webapi/slice/resourceinfo/api/IriConverterLiveSpec.scala

remove file

simplify

remove unused code

fix test labels

fmt

fix compile warnings and add type annotation to public field

Replace if else with "pattern matching"

Replace if else with "pattern matching"

Rename isStricterThan method

cleanup, remove unused code

Remove Option.get by more elaborate pattern matching

extract explanation method

prevent knora-admin and knora-base ontologies to be change

Introduce CanSetCardinalityCheckResult in order to distinguish why setting is not possible

Introduce CanSetCardinalityCheckResult in order to distinguish why setting is not possible

Introduce CanSetCardinalityCheckResult in order to distinguish why setting is not possible

Introduce CanSetCardinalityCheckResult in order to distinguish why setting is not possible

fix type in trait

Add RestCardinalityService

Use RestCardinalityService in OntologiesRouteV2 and assemble layers

fmt

add headers

extend response with optional reason of failure

fix test setup

fmt

Introduce common ReadOnlyRepository and CrudRe
traits

header && fmt

Re-add old behaviour

fmt

fixup R2R spec

Introduce canUpdateCardinality

Introduce requestcontext completion with run unsafe zio

fmt

fix error message

make private

move getStringQueryParam to RouteUtilV2

move request params checking into RestCardinalityService

fmt

finetuning

fmt
@seakayone seakayone force-pushed the wip/widen-cardinality-rebased branch from 3d662a0 to 06068eb Compare January 13, 2023 11:50
@seakayone seakayone self-assigned this Jan 13, 2023
@seakayone seakayone requested review from irinaschubert, BalduinLandolt and mpro7 and removed request for irinaschubert January 13, 2023 11:51
@seakayone seakayone marked this pull request as ready for review January 13, 2023 12:36
irinaschubert
irinaschubert approved these changes Jan 13, 2023
View reviewed changes
Copy link

@irinaschubert irinaschubert left a comment

Choose a reason for hiding this comment

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

LGTM

webapi/src/it/scala/org/knora/webapi/responders/v2/ResourcesResponderV2Spec.scala Show resolved Hide resolved
webapi/src/main/scala/org/knora/webapi/messages/StringFormatter.scala Outdated Show resolved Hide resolved
webapi/src/main/scala/org/knora/webapi/messages/StringFormatter.scala Outdated Show resolved Hide resolved
webapi/src/main/scala/org/knora/webapi/routing/Authenticator.scala Outdated Show resolved Hide resolved
webapi/src/main/scala/org/knora/webapi/routing/RouteUtilV2.scala Show resolved Hide resolved
webapi/src/main/scala/org/knora/webapi/slice/ontology/domain/model/Cardinality.scala Show resolved Hide resolved
webapi/src/main/scala/org/knora/webapi/slice/ontology/domain/service/CardinalityService.scala Outdated Show resolved Hide resolved
webapi/src/main/scala/org/knora/webapi/slice/ontology/domain/service/CardinalityService.scala Show resolved Hide resolved
webapi/src/main/twirl/org/knora/webapi/queries/sparql/v2/isPropertyUsed.scala.txt Outdated Show resolved Hide resolved
webapi/src/test/scala/org/knora/webapi/store/triplestore/api/TriplestoreServiceFake.scala Show resolved Hide resolved
seakayone and others added 10 commits January 13, 2023 16:35
@seakayone
Update webapi/src/main/scala/org/knora/webapi/messages/StringFormatte…
e0c2238 
…r.scala

Co-authored-by: irinaschubert <irina.schubert@dasch.swiss>
@seakayone
Update webapi/src/main/scala/org/knora/webapi/slice/ontology/domain/s…
95d5774 
…ervice/CardinalityService.scala

Co-authored-by: irinaschubert <irina.schubert@dasch.swiss>
@seakayone
Rename getUserADMZ
c041ac4
@seakayone
Remove StringFormatter.toInternalSmartIri
ef9523e
@seakayone
Update webapi/src/main/scala/org/knora/webapi/routing/UnsafeZioRun.scala
f3784dd 
Co-authored-by: irinaschubert <irina.schubert@dasch.swiss>
@seakayone
Rename RestCardinalityService.canChangeCardinality
4f3024c
@seakayone
Move IriConverterLiveSpec to test source set and simplify implementation
d0c0043
@seakayone
Fix test
5c7dc3a
@seakayone
Remove unused CanChangeCardinalitiesRequestV2
93e7f2c
@seakayone
Update documentation of the isPropertyUsed query
53cb70d
seakayone and others added 3 commits January 16, 2023 10:31
@seakayone
fmt
8ac006d
@seakayone @mpro7
Update webapi/src/main/scala/org/knora/webapi/routing/Authenticator.s…
787b27e 
…cala

Co-authored-by: Marcin Procyk <marcin.procyk@dasch.swiss>
@seakayone @mpro7
Update webapi/src/main/scala/org/knora/webapi/slice/ontology/domain/s…
541ab66 
…ervice/CardinalityService.scala

Co-authored-by: Marcin Procyk <marcin.procyk@dasch.swiss>
seakayone and others added 2 commits January 16, 2023 10:32
@seakayone @mpro7
Update webapi/src/main/scala/org/knora/webapi/slice/ontology/domain/s…
d98a72a 
…ervice/CardinalityService.scala

Co-authored-by: Marcin Procyk <marcin.procyk@dasch.swiss>
@seakayone @mpro7
Update webapi/src/main/scala/org/knora/webapi/slice/ontology/domain/s…
f0b1494 
…ervice/CardinalityService.scala

Co-authored-by: Marcin Procyk <marcin.procyk@dasch.swiss>
BalduinLandolt
BalduinLandolt approved these changes Jan 16, 2023
View reviewed changes
Copy link
Collaborator

@BalduinLandolt BalduinLandolt left a comment

Choose a reason for hiding this comment

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

I don't think I grasped everything (big PR and I went through it too quickly), but looks good

webapi/src/it/scala/org/knora/webapi/e2e/v2/OntologyV2R2RSpec.scala Show resolved Hide resolved
webapi/src/main/scala/org/knora/webapi/messages/OntologyConstants.scala Outdated Show resolved Hide resolved
webapi/src/main/scala/org/knora/webapi/messages/v2/responder/KnoraResponseV2.scala Show resolved Hide resolved
webapi/src/main/scala/org/knora/webapi/responders/v2/ontology/CardinalityHandler.scala Show resolved Hide resolved
webapi/src/main/scala/org/knora/webapi/routing/v2/OntologiesRouteV2.scala Show resolved Hide resolved
webapi/src/test/scala/org/knora/webapi/slice/ontology/domain/CardinalityServiceLiveSpec.scala Show resolved Hide resolved
@seakayone seakayone enabled auto-merge (squash) January 16, 2023 12:04
@seakayone seakayone merged commit 17e7064 into main Jan 16, 2023
@seakayone seakayone deleted the wip/widen-cardinality-rebased branch January 16, 2023 12:30
@daschbot daschbot mentioned this pull request Jan 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@irinaschubert irinaschubert irinaschubert approved these changes

@mpro7 mpro7 mpro7 approved these changes

@BalduinLandolt BalduinLandolt BalduinLandolt approved these changes

Assignees

@seakayone seakayone

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@seakayone @irinaschubert @mpro7 @BalduinLandolt