Refactor statement update (!1208)

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<StatementId>,
         @RequestBody(required = true) statementEditRequest: BulkStatementEditRequest,
+        currentUser: Authentication?,
         capabilities: MediaTypeCapabilities
     ): Iterable<BulkPutStatementResponse> =
         statementsIds.map {
             statementService.update(
                 UpdateStatementUseCase.UpdateCommand(
                     statementId = it,
+                    contributorId = currentUser.contributorId(),
                     subjectId = statementEditRequest.subjectId,
                     predicateId = statementEditRequest.predicateId,
                     objectId = statementEditRequest.objectId,

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<StatementRepresentation> {
         statementService.update(
             UpdateStatementUseCase.UpdateCommand(
                 statementId = id,
+                contributorId = currentUser.contributorId(),
                 subjectId = request.subjectId,
                 predicateId = request.predicateId,
                 objectId = request.objectId

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,
 )

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)
         }
     }
 

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)

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,19 +222,23 @@ 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)
     }
 
     @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)

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
     )
 }
 

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
     )

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")

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")