diff --git a/graph/graph-adapter-input-rest-spring-mvc/src/main/kotlin/org/orkg/graph/adapter/input/rest/BulkStatementController.kt b/graph/graph-adapter-input-rest-spring-mvc/src/main/kotlin/org/orkg/graph/adapter/input/rest/BulkStatementController.kt index 72a37a50e..bba8f08bb 100644 --- a/graph/graph-adapter-input-rest-spring-mvc/src/main/kotlin/org/orkg/graph/adapter/input/rest/BulkStatementController.kt +++ b/graph/graph-adapter-input-rest-spring-mvc/src/main/kotlin/org/orkg/graph/adapter/input/rest/BulkStatementController.kt @@ -4,6 +4,7 @@ import com.fasterxml.jackson.annotation.JsonProperty import org.orkg.common.MediaTypeCapabilities import org.orkg.common.ThingId import org.orkg.common.annotations.RequireLogin +import org.orkg.common.contributorId import org.orkg.contenttypes.domain.pmap import org.orkg.graph.adapter.input.rest.mapping.StatementRepresentationAdapter import org.orkg.graph.domain.StatementId @@ -15,6 +16,7 @@ import org.springframework.data.domain.Pageable import org.springframework.http.MediaType import org.springframework.http.ResponseEntity import org.springframework.http.ResponseEntity.noContent +import org.springframework.security.core.Authentication import org.springframework.web.bind.annotation.DeleteMapping import org.springframework.web.bind.annotation.GetMapping import org.springframework.web.bind.annotation.PutMapping @@ -72,12 +74,14 @@ class BulkStatementController( fun edit( @RequestParam("ids") statementsIds: List, @RequestBody(required = true) statementEditRequest: BulkStatementEditRequest, + currentUser: Authentication?, capabilities: MediaTypeCapabilities ): Iterable = statementsIds.map { statementService.update( UpdateStatementUseCase.UpdateCommand( statementId = it, + contributorId = currentUser.contributorId(), subjectId = statementEditRequest.subjectId, predicateId = statementEditRequest.predicateId, objectId = statementEditRequest.objectId, diff --git a/graph/graph-adapter-input-rest-spring-mvc/src/main/kotlin/org/orkg/graph/adapter/input/rest/StatementController.kt b/graph/graph-adapter-input-rest-spring-mvc/src/main/kotlin/org/orkg/graph/adapter/input/rest/StatementController.kt index 7dc9a5f34..7fdddc1ce 100644 --- a/graph/graph-adapter-input-rest-spring-mvc/src/main/kotlin/org/orkg/graph/adapter/input/rest/StatementController.kt +++ b/graph/graph-adapter-input-rest-spring-mvc/src/main/kotlin/org/orkg/graph/adapter/input/rest/StatementController.kt @@ -112,11 +112,13 @@ class StatementController( @PathVariable id: StatementId, @RequestBody request: UpdateStatementRequest, uriComponentsBuilder: UriComponentsBuilder, + currentUser: Authentication?, capabilities: MediaTypeCapabilities ): ResponseEntity { statementService.update( UpdateStatementUseCase.UpdateCommand( statementId = id, + contributorId = currentUser.contributorId(), subjectId = request.subjectId, predicateId = request.predicateId, objectId = request.objectId diff --git a/graph/graph-core-services/src/main/kotlin/org/orkg/graph/domain/Extensions.kt b/graph/graph-core-services/src/main/kotlin/org/orkg/graph/domain/Extensions.kt index fab330c51..90381e0c7 100644 --- a/graph/graph-core-services/src/main/kotlin/org/orkg/graph/domain/Extensions.kt +++ b/graph/graph-core-services/src/main/kotlin/org/orkg/graph/domain/Extensions.kt @@ -27,20 +27,27 @@ fun Resource.apply(command: UpdateResourceUseCase.UpdateCommand): Resource = ) fun UpdateStatementUseCase.UpdateCommand.hasNoContents(): Boolean = - subjectId == null && predicateId == null && objectId == null + subjectId == null && predicateId == null && objectId == null && modifiable == null fun GeneralStatement.apply( command: UpdateStatementUseCase.UpdateCommand, thingRepository: ThingRepository, - predicateRepository: PredicateRepository + predicateRepository: PredicateRepository, + subjectValidator: (Thing) -> Unit = {}, + predicateValidator: (Predicate) -> Unit = {}, + objectValidator: (Thing) -> Unit = {}, ): GeneralStatement = copy( subject = command.subjectId?.takeIf { id -> id != subject.id } ?.let { id -> thingRepository.findByThingId(id).orElseThrow { StatementSubjectNotFound(id) } } + ?.also(subjectValidator) ?: subject, predicate = command.predicateId?.takeIf { id -> id != predicate.id } ?.let { id -> predicateRepository.findById(id).orElseThrow { StatementPredicateNotFound(id) } } + ?.also(predicateValidator) ?: predicate, `object` = command.objectId?.takeIf { id -> id != `object`.id } ?.let { id -> thingRepository.findByThingId(id).orElseThrow { StatementObjectNotFound(id) } } - ?: `object` + ?.also(objectValidator) + ?: `object`, + modifiable = command.modifiable ?: modifiable, ) diff --git a/graph/graph-core-services/src/main/kotlin/org/orkg/graph/domain/StatementService.kt b/graph/graph-core-services/src/main/kotlin/org/orkg/graph/domain/StatementService.kt index ad855a3d1..3afc9230d 100644 --- a/graph/graph-core-services/src/main/kotlin/org/orkg/graph/domain/StatementService.kt +++ b/graph/graph-core-services/src/main/kotlin/org/orkg/graph/domain/StatementService.kt @@ -131,48 +131,31 @@ class StatementService( } override fun update(command: UpdateStatementUseCase.UpdateCommand) { - val found = statementRepository.findByStatementId(command.statementId) + if (command.hasNoContents()) return + val statement = statementRepository.findByStatementId(command.statementId) .orElseThrow { StatementNotFound(command.statementId.value) } - if (!found.modifiable) { - throw StatementNotModifiable(found.id) + if (!statement.modifiable) { + throw StatementNotModifiable(statement.id) } - if (found.predicate.id == Predicates.hasListElement && found.subject is Resource && Classes.list in (found.subject as Resource).classes) { + if (statement.predicate.id == Predicates.hasListElement && statement.subject is Resource && Classes.list in (statement.subject as Resource).classes) { throw InvalidStatement.isListElementStatement() } - - var statement = found - - if (command.subjectId != null && command.subjectId != found.subject.id) { - val foundSubject = thingRepository.findByThingId(command.subjectId!!) - .orElseThrow { StatementSubjectNotFound(command.subjectId!!) } - - if (foundSubject is Resource) { - if (Classes.rosettaStoneStatement in foundSubject.classes) { + val updated = statement.apply(command, thingRepository, predicateService, { subject -> + if (subject is Resource) { + if (Classes.rosettaStoneStatement in subject.classes) { throw InvalidStatement.includesRosettaStoneStatementResource() - } else if ((found.predicate.id == Predicates.hasListElement || command.predicateId == Predicates.hasListElement) && Classes.list in foundSubject.classes) { + } else if ((statement.predicate.id == Predicates.hasListElement || command.predicateId == Predicates.hasListElement) && Classes.list in subject.classes) { throw InvalidStatement.isListElementStatement() } } - statement = statement.copy(subject = foundSubject) - } - if (command.predicateId != null && command.predicateId != found.predicate.id) { - val foundPredicate = predicateService.findById(command.predicateId!!) - .orElseThrow { StatementPredicateNotFound(command.predicateId!!) } - statement = statement.copy(predicate = foundPredicate) - } - if (command.objectId != null && command.objectId != found.`object`.id) { - val foundObject = thingRepository.findByThingId(command.objectId!!) - .orElseThrow { StatementObjectNotFound(command.objectId!!) } - statement = statement.copy(`object` = foundObject) - } - - if (statement != found) { + }) + if (updated != statement) { statementRepository.deleteByStatementId(command.statementId) - // Restore literal that may have automatically been deleted by statement deletion - if (found.`object` is Literal && statement.`object` == found.`object`) { - literalRepository.save(found.`object` as Literal) + // restore literal that may have automatically been deleted by statement deletion + if (statement.`object` is Literal && updated.`object` == statement.`object`) { + literalRepository.save(statement.`object` as Literal) } - statementRepository.save(statement) + statementRepository.save(updated) } } diff --git a/graph/graph-core-services/src/test/kotlin/org/orkg/graph/domain/StatementServiceUnitTest.kt b/graph/graph-core-services/src/test/kotlin/org/orkg/graph/domain/StatementServiceUnitTest.kt index 44d562e00..fc6cab105 100644 --- a/graph/graph-core-services/src/test/kotlin/org/orkg/graph/domain/StatementServiceUnitTest.kt +++ b/graph/graph-core-services/src/test/kotlin/org/orkg/graph/domain/StatementServiceUnitTest.kt @@ -30,6 +30,7 @@ import org.orkg.graph.testing.fixtures.createLiteral import org.orkg.graph.testing.fixtures.createPredicate import org.orkg.graph.testing.fixtures.createResource import org.orkg.graph.testing.fixtures.createStatement +import org.orkg.testing.MockUserId import org.orkg.testing.pageOf import org.springframework.data.domain.PageImpl import org.springframework.data.domain.Sort @@ -406,6 +407,7 @@ internal class StatementServiceUnitTest : MockkDescribeSpec({ } describe("updating a statement") { + val contributorId = ContributorId(MockUserId.USER) context("with a non-literal object") { it("successfully updates the statement") { val id = StatementId("S1") @@ -415,6 +417,7 @@ internal class StatementServiceUnitTest : MockkDescribeSpec({ val newObject = createResource(ThingId("newObject")) val command = UpdateStatementUseCase.UpdateCommand( statementId = id, + contributorId = contributorId, subjectId = newSubject.id, predicateId = newPredicate.id, objectId = newObject.id @@ -457,6 +460,7 @@ internal class StatementServiceUnitTest : MockkDescribeSpec({ val newPredicate = createPredicate(ThingId("newPredicate")) val command = UpdateStatementUseCase.UpdateCommand( statementId = id, + contributorId = contributorId, subjectId = newSubject.id, predicateId = newPredicate.id, ) @@ -496,6 +500,7 @@ internal class StatementServiceUnitTest : MockkDescribeSpec({ val statement = createStatement() val command = UpdateStatementUseCase.UpdateCommand( statementId = id, + contributorId = contributorId, subjectId = statement.subject.id, predicateId = statement.predicate.id, objectId = statement.`object`.id @@ -513,7 +518,9 @@ internal class StatementServiceUnitTest : MockkDescribeSpec({ it("throws an error") { val id = StatementId("S1") val command = UpdateStatementUseCase.UpdateCommand( - statementId = id + statementId = id, + contributorId = contributorId, + subjectId = ThingId("someChange") ) every { statementRepository.findByStatementId(id) } returns Optional.empty() @@ -529,7 +536,7 @@ internal class StatementServiceUnitTest : MockkDescribeSpec({ context("that is non-modifiable") { val id = StatementId("S1") val fakeStatement = createStatement(id, modifiable = false) - val command = UpdateStatementUseCase.UpdateCommand(id) + val command = UpdateStatementUseCase.UpdateCommand(id, contributorId, subjectId = ThingId("someChange")) it("throws an exception") { every { statementRepository.findByStatementId(id) } returns Optional.of(fakeStatement) @@ -546,7 +553,9 @@ internal class StatementServiceUnitTest : MockkDescribeSpec({ val listId = ThingId("L1") val id = StatementId("S1") val command = UpdateStatementUseCase.UpdateCommand( - statementId = id + statementId = id, + contributorId = contributorId, + subjectId = ThingId("someChange") ) val existingStatement = GeneralStatement( id = id, @@ -577,6 +586,7 @@ internal class StatementServiceUnitTest : MockkDescribeSpec({ val statement = createStatement() val command = UpdateStatementUseCase.UpdateCommand( statementId = id, + contributorId = contributorId, subjectId = ThingId("missing") ) every { statementRepository.findByStatementId(id) } returns Optional.of(statement) @@ -598,6 +608,7 @@ internal class StatementServiceUnitTest : MockkDescribeSpec({ val id = StatementId("S1") val command = UpdateStatementUseCase.UpdateCommand( statementId = id, + contributorId = contributorId, subjectId = listId, predicateId = Predicates.hasListElement ) @@ -645,6 +656,7 @@ internal class StatementServiceUnitTest : MockkDescribeSpec({ service.update( UpdateStatementUseCase.UpdateCommand( statementId = id, + contributorId = contributorId, subjectId = subjectId, predicateId = Predicates.description, objectId = ThingId("R1") @@ -665,6 +677,7 @@ internal class StatementServiceUnitTest : MockkDescribeSpec({ val statement = createStatement() val command = UpdateStatementUseCase.UpdateCommand( statementId = id, + contributorId = contributorId, predicateId = ThingId("missing") ) every { statementRepository.findByStatementId(id) } returns Optional.of(statement) @@ -686,6 +699,7 @@ internal class StatementServiceUnitTest : MockkDescribeSpec({ val statement = createStatement() val command = UpdateStatementUseCase.UpdateCommand( statementId = id, + contributorId = contributorId, objectId = ThingId("missing") ) every { statementRepository.findByStatementId(id) } returns Optional.of(statement) diff --git a/graph/graph-core-services/src/test/kotlin/org/orkg/graph/domain/UnsafeStatementServiceUnitTest.kt b/graph/graph-core-services/src/test/kotlin/org/orkg/graph/domain/UnsafeStatementServiceUnitTest.kt index bbc446b6e..d4a6d0a1b 100644 --- a/graph/graph-core-services/src/test/kotlin/org/orkg/graph/domain/UnsafeStatementServiceUnitTest.kt +++ b/graph/graph-core-services/src/test/kotlin/org/orkg/graph/domain/UnsafeStatementServiceUnitTest.kt @@ -10,6 +10,7 @@ import io.mockk.verify import java.time.OffsetDateTime import java.util.* import org.junit.jupiter.api.Test +import org.orkg.common.ContributorId import org.orkg.common.ThingId import org.orkg.common.testing.fixtures.MockkBaseTest import org.orkg.common.testing.fixtures.fixedClock @@ -24,6 +25,7 @@ import org.orkg.graph.testing.fixtures.createLiteral import org.orkg.graph.testing.fixtures.createPredicate import org.orkg.graph.testing.fixtures.createResource import org.orkg.graph.testing.fixtures.createStatement +import org.orkg.testing.MockUserId internal class UnsafeStatementServiceUnitTest : MockkBaseTest { @@ -144,7 +146,8 @@ internal class UnsafeStatementServiceUnitTest : MockkBaseTest { val command = updateStatementCommand().copy( subjectId = ThingId("R321"), predicateId = ThingId("P321"), - objectId = ThingId("L321") + objectId = ThingId("L321"), + modifiable = false ) val statement = createStatement(command.statementId) val subject = createResource(command.subjectId!!) @@ -173,7 +176,7 @@ internal class UnsafeStatementServiceUnitTest : MockkBaseTest { it.`object` shouldBe `object` it.createdBy shouldBe statement.createdBy it.createdAt shouldBe statement.createdAt - it.modifiable shouldBe statement.modifiable + it.modifiable shouldBe command.modifiable }) } } @@ -219,7 +222,8 @@ internal class UnsafeStatementServiceUnitTest : MockkBaseTest { @Test fun `Given a statement update command, when updating no properties, it does nothing`() { val id = StatementId("S123") - val command = UpdateStatementUseCase.UpdateCommand(id) + val contributorId = ContributorId(MockUserId.USER) + val command = UpdateStatementUseCase.UpdateCommand(id, contributorId) service.update(command) } @@ -227,11 +231,14 @@ internal class UnsafeStatementServiceUnitTest : MockkBaseTest { @Test fun `Given a statement update command, when inputs are identical to existing statement, it does nothing`() { val statement = createStatement() + val contributorId = ContributorId(MockUserId.USER) val command = UpdateStatementUseCase.UpdateCommand( statementId = statement.id, + contributorId = contributorId, subjectId = statement.subject.id, predicateId = statement.predicate.id, objectId = statement.`object`.id, + modifiable = statement.modifiable, ) every { statementRepository.findByStatementId(command.statementId) } returns Optional.of(statement) diff --git a/graph/graph-ports-input/src/main/kotlin/org/orkg/graph/input/statement-commands.kt b/graph/graph-ports-input/src/main/kotlin/org/orkg/graph/input/statement-commands.kt index 6205ed84f..9ba868122 100644 --- a/graph/graph-ports-input/src/main/kotlin/org/orkg/graph/input/statement-commands.kt +++ b/graph/graph-ports-input/src/main/kotlin/org/orkg/graph/input/statement-commands.kt @@ -20,12 +20,13 @@ interface CreateStatementUseCase { interface UpdateStatementUseCase { fun update(command: UpdateCommand) - // TODO: Add contributorId and statementId data class UpdateCommand( val statementId: StatementId, + val contributorId: ContributorId, val subjectId: ThingId? = null, val predicateId: ThingId? = null, val objectId: ThingId? = null, + val modifiable: Boolean? = null ) } diff --git a/graph/graph-ports-input/src/testFixtures/kotlin/org/orkg/graph/input/testing/fixtures/StatementFactoryFunctions.kt b/graph/graph-ports-input/src/testFixtures/kotlin/org/orkg/graph/input/testing/fixtures/StatementFactoryFunctions.kt index 1df868f37..662682028 100644 --- a/graph/graph-ports-input/src/testFixtures/kotlin/org/orkg/graph/input/testing/fixtures/StatementFactoryFunctions.kt +++ b/graph/graph-ports-input/src/testFixtures/kotlin/org/orkg/graph/input/testing/fixtures/StatementFactoryFunctions.kt @@ -19,7 +19,9 @@ fun createStatementCommand(): CreateStatementUseCase.CreateCommand = fun updateStatementCommand(): UpdateStatementUseCase.UpdateCommand = UpdateStatementUseCase.UpdateCommand( statementId = StatementId("S123"), + contributorId = ContributorId("cfaaec46-d5e9-49a8-8c39-4aa9651928fe"), subjectId = ThingId("R123"), predicateId = ThingId("P123"), objectId = ThingId("L123"), + modifiable = true ) diff --git a/rest-api-server/build.gradle.kts b/rest-api-server/build.gradle.kts index 19957af94..9b4a1e371 100644 --- a/rest-api-server/build.gradle.kts +++ b/rest-api-server/build.gradle.kts @@ -241,7 +241,6 @@ dependencies { "integrationTestImplementation"("org.junit.jupiter:junit-jupiter-params") "integrationTestImplementation"("org.springframework.boot:spring-boot-test") "integrationTestImplementation"("org.springframework.restdocs:spring-restdocs-core") - "integrationTestImplementation"("org.springframework.security:spring-security-test") "integrationTestImplementation"("org.springframework:spring-beans") "integrationTestImplementation"("org.springframework:spring-test") "integrationTestImplementation"("org.springframework:spring-tx") diff --git a/rest-api-server/src/integrationTest/kotlin/org/orkg/graph/adapter/input/rest/StatementControllerIntegrationTest.kt b/rest-api-server/src/integrationTest/kotlin/org/orkg/graph/adapter/input/rest/StatementControllerIntegrationTest.kt index e31506308..12ff9a5c8 100644 --- a/rest-api-server/src/integrationTest/kotlin/org/orkg/graph/adapter/input/rest/StatementControllerIntegrationTest.kt +++ b/rest-api-server/src/integrationTest/kotlin/org/orkg/graph/adapter/input/rest/StatementControllerIntegrationTest.kt @@ -4,10 +4,10 @@ import org.assertj.core.api.Assertions.assertThat import org.hamcrest.Matchers.hasSize import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test -import org.orkg.createStatement import org.orkg.createLiteral import org.orkg.createPredicate import org.orkg.createResource +import org.orkg.createStatement import org.orkg.graph.adapter.input.rest.PredicateControllerIntegrationTest.RestDoc.predicateResponseFields import org.orkg.graph.adapter.input.rest.testing.fixtures.resourceResponseFields import org.orkg.graph.input.LiteralUseCases @@ -23,7 +23,6 @@ import org.springframework.restdocs.payload.PayloadDocumentation.fieldWithPath import org.springframework.restdocs.payload.PayloadDocumentation.responseFields import org.springframework.restdocs.request.RequestDocumentation.parameterWithName import org.springframework.restdocs.request.RequestDocumentation.pathParameters -import org.springframework.security.test.context.support.WithMockUser import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status import org.springframework.transaction.annotation.Transactional @@ -138,7 +137,7 @@ internal class StatementControllerIntegrationTest : MockMvcBaseTest("statements" } @Test - @WithMockUser + @TestWithMockUser fun editResourceStatement() { val s = resourceService.createResource(label = "ORKG") val p = predicateService.createPredicate(label = "created by") @@ -162,7 +161,7 @@ internal class StatementControllerIntegrationTest : MockMvcBaseTest("statements" } @Test - @WithMockUser + @TestWithMockUser fun editLiteralStatement() { val s = resourceService.createResource(label = "ORKG") val p = predicateService.createPredicate(label = "based in")