Repository: BlueBrain/nexus (scala)

Issue:
Improve how diff is performed in the resource update
**Motivation**
When performing an update and there is no detected diff, a new event is still created when a schema id is provided.
We want to change this depending on how the schema is resolved

**Acceptance criteria**
1. If the schema is resolved to the same schema with the same or newer revision , no update event is created
2. If the schema is resolved to another schema, a updated schema event is created
3. If a tag is provided, the tag is always created (it could be tied to a different event though)
4. There is no change of behaviour when a change is detected on the resource

Pull request:
Fixes #4970

Patch:
diff --git a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/Resources.scala b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/Resources.scala
index 6ffc19986a..18e938419b 100644
--- a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/Resources.scala
+++ b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/Resources.scala
@@ -323,8 +323,8 @@ object Resources {
       s.copy(rev = e.rev, tags = s.tags - e.tag, updatedAt = e.instant, updatedBy = e.subject)
     }
 
-    def resourceSchemaUpdated(e: ResourceSchemaUpdated): Option[ResourceState] = state.map {
-      _.copy(rev = e.rev, schema = e.schema, schemaProject = e.schemaProject, updatedAt = e.instant, updatedBy = e.subject)
+    def resourceSchemaUpdated(e: ResourceSchemaUpdated): Option[ResourceState] = state.map { s =>
+      s.copy(rev = e.rev, schema = e.schema, schemaProject = e.schemaProject, tags = s.tags ++ Tags(e.tag, e.rev), updatedAt = e.instant, updatedBy = e.subject)
     }
     // format: on
 
@@ -417,13 +417,29 @@ object Resources {
 
     def update(u: UpdateResource) = {
 
-      def onChange(state: ResourceState) = {
+      def validateWithSchema(state: ResourceState) = {
         val schemaClaim = SchemaClaim.onUpdate(u.project, u.schemaOpt, ResourceRef.Latest(state.schema.iri), u.caller)
+        validate(u.jsonld, schemaClaim, u.projectContext.enforceSchema)
+      }
+
+      def updateEvent(state: ResourceState) =
         for {
-          (schemaRev, schemaProject) <- validate(u.jsonld, schemaClaim, u.projectContext.enforceSchema)
+          (schemaRev, schemaProject) <- validateWithSchema(state)
           time                       <- clock.realTimeInstant
         } yield ResourceUpdated(u.project, schemaRev, schemaProject, u.jsonld, state.rev + 1, time, u.subject, u.tag)
-      }
+
+      def updateSchema(state: ResourceState) =
+        for {
+          (schemaRev, schemaProject) <- validateWithSchema(state)
+          time                       <- clock.realTimeInstant
+          sameSchema                  = schemaRev.original == state.schema.original && schemaProject == state.schemaProject
+          schemaUpdatedEvent          = {
+            val types   = u.jsonld.types
+            val nextRev = state.rev + 1
+            ResourceSchemaUpdated(u.id, u.project, schemaRev, schemaProject, types, nextRev, time, u.subject, u.tag)
+          }
+          event                      <- if (!sameSchema) IO.pure(schemaUpdatedEvent) else fallbackToTag(state)
+        } yield event
 
       // If there is no changes but a tag is provided, we still apply it
       def fallbackToTag(state: ResourceState) =
@@ -435,7 +451,15 @@ object Resources {
       for {
         state          <- stateWhereResourceIsEditable(u)
         changeDetected <- detectChange(u.jsonld, state)
-        event          <- if (u.schemaOpt.isDefined || changeDetected) onChange(state) else fallbackToTag(state)
+        event          <-
+          (changeDetected, u.schemaOpt.isDefined) match {
+            // Changes have been detected
+            case (true, _)      => updateEvent(state)
+            // No changes but the schema may have been updated
+            case (false, true)  => updateSchema(state)
+            // No changes and no schema has been provided
+            case (false, false) => fallbackToTag(state)
+          }
       } yield event
     }
 
@@ -446,8 +470,9 @@ object Resources {
         schemaClaim                 = SchemaClaim.onUpdate(u.project, u.schemaRef, state.schema, u.caller)
         (schemaRev, schemaProject) <- validate(stateJsonLd, schemaClaim, u.projectContext.enforceSchema)
         types                       = state.expanded.getTypes.getOrElse(Set.empty)
+        nextRev                     = state.rev + 1
         time                       <- clock.realTimeInstant
-      } yield ResourceSchemaUpdated(u.id, u.project, schemaRev, schemaProject, types, state.rev + 1, time, u.subject)
+      } yield ResourceSchemaUpdated(u.id, u.project, schemaRev, schemaProject, types, nextRev, time, u.subject, None)
     }
 
     def refresh(r: RefreshResource) = {
@@ -528,10 +553,11 @@ object Resources {
       ResourceState.serializer,
       Tagger[ResourceEvent](
         {
-          case r: ResourceCreated  => r.tag.map(t => t -> r.rev)
-          case r: ResourceUpdated  => r.tag.map(t => t -> r.rev)
-          case r: ResourceTagAdded => Some(r.tag -> r.targetRev)
-          case _                   => None
+          case r: ResourceCreated       => r.tag.map(t => t -> r.rev)
+          case r: ResourceUpdated       => r.tag.map(t => t -> r.rev)
+          case r: ResourceTagAdded      => Some(r.tag -> r.targetRev)
+          case r: ResourceSchemaUpdated => r.tag.map(t => t -> r.rev)
+          case _                        => None
         },
         {
           case r: ResourceTagDeleted => Some(r.tag)
diff --git a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/model/ResourceEvent.scala b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/model/ResourceEvent.scala
index 32f455ca33..2b59086963 100644
--- a/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/model/ResourceEvent.scala
+++ b/delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/model/ResourceEvent.scala
@@ -205,6 +205,26 @@ object ResourceEvent {
 
   }
 
+  /**
+    * Event representing a change of schema for a resource
+    *
+    * @param id
+    *   the resource identifier
+    * @param project
+    *   the project where the resource belongs
+    * @param schema
+    *   the schema used to constrain the resource
+    * @param schemaProject
+    *   the project where the schema belongs
+    * @param types
+    *   the collection of known resource types
+    * @param rev
+    *   the resource revision
+    * @param instant
+    *   the instant when this event was created
+    * @param subject
+    *   the subject which created this event
+    */
   final case class ResourceSchemaUpdated(
       id: Iri,
       project: ProjectRef,
@@ -213,7 +233,8 @@ object ResourceEvent {
       types: Set[Iri],
       rev: Int,
       instant: Instant,
-      subject: Subject
+      subject: Subject,
+      tag: Option[UserTag]
   ) extends ResourceEvent
 
   /**


Test patch:
diff --git a/delta/sdk/src/test/resources/resources/database/resource-schema-updated.json b/delta/sdk/src/test/resources/resources/database/resource-schema-updated.json
index ea711a3a2f..2c2353eaea 100644
--- a/delta/sdk/src/test/resources/resources/database/resource-schema-updated.json
+++ b/delta/sdk/src/test/resources/resources/database/resource-schema-updated.json
@@ -13,5 +13,6 @@
     "realm": "myrealm",
     "@type": "User"
   },
+  "tag": "mytag",
   "@type": "ResourceSchemaUpdated"
 }
diff --git a/delta/sdk/src/test/resources/resources/sse/resource-schema-updated.json b/delta/sdk/src/test/resources/resources/sse/resource-schema-updated.json
index a96974e6be..c2b07ec340 100644
--- a/delta/sdk/src/test/resources/resources/sse/resource-schema-updated.json
+++ b/delta/sdk/src/test/resources/resources/sse/resource-schema-updated.json
@@ -6,6 +6,7 @@
   "_project": "http://localhost/v1/projects/myorg/myproj",
   "_resourceId": "https://bluebrain.github.io/nexus/vocabulary/myId",
   "_rev": 6,
+  "tag": "mytag",
   "_schemaProject": "http://localhost/v1/projects/myorg/myproj",
   "_subject": "http://localhost/v1/realms/myrealm/users/username",
   "_types": [
diff --git a/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/ResourcesSpec.scala b/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/ResourcesSpec.scala
index def09a2cb3..3e9357f32e 100644
--- a/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/ResourcesSpec.scala
+++ b/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/ResourcesSpec.scala
@@ -16,7 +16,7 @@ import ch.epfl.bluebrain.nexus.delta.sdk.resources.model.ResourceEvent._
 import ch.epfl.bluebrain.nexus.delta.sdk.resources.model.ResourceRejection._
 import ch.epfl.bluebrain.nexus.delta.sdk.resources.model.{ResourceCommand, ResourceEvent, ResourceState}
 import ch.epfl.bluebrain.nexus.delta.sourcing.model.Identity.User
-import ch.epfl.bluebrain.nexus.delta.sourcing.model.Label
+import ch.epfl.bluebrain.nexus.delta.sourcing.model.{Label, ResourceRef}
 import ch.epfl.bluebrain.nexus.delta.sourcing.model.ResourceRef.{Latest, Revision}
 import ch.epfl.bluebrain.nexus.delta.sourcing.model.Tag.UserTag
 import ch.epfl.bluebrain.nexus.testkit.CirceLiteral
@@ -113,23 +113,36 @@ class ResourcesSpec extends CatsEffectSpec with CirceLiteral with ValidateResour
           )
       }
 
-      "create a tag event from a UpdateResource command when no changes are detected and a tag is provided" in {
-        val schema  = Latest(schemas.resources)
-        val current = ResourceGen.currentState(projectRef, jsonld, schema)
+      "create a schema updated event from a UpdateResource command when no changes are detected and a new schema is provided" in {
+        val schema        = Latest(schemas.resources)
+        val newSchema     = ResourceRef.Revision(nxv + "new-schema", 1)
+        val current       = ResourceGen.currentState(projectRef, jsonld, schema)
+        val expectedEvent =
+          ResourceSchemaUpdated(myId, projectRef, newSchema, projectRef, jsonld.types, 2, epoch, subject, Some(tag))
+        eval(
+          Some(current),
+          UpdateResource(projectRef, projectContext, Some(newSchema), jsonld, 1, caller, Some(tag))
+        ).accepted shouldEqual expectedEvent
+      }
+
+      "create a tag event from a UpdateResource command when no changes are detected and the same schema is provided" in {
+        val schema        = ResourceRef.Revision(nxv + "schema", 1)
+        val current       = ResourceGen.currentState(projectRef, jsonld, schema)
+        val expectedEvent = ResourceTagAdded(myId, projectRef, jsonld.types, 1, tag, 2, epoch, subject)
+        eval(
+          Some(current),
+          UpdateResource(projectRef, projectContext, Some(schema), jsonld, 1, caller, Some(tag))
+        ).accepted shouldEqual expectedEvent
+      }
+
+      "create a tag event from a UpdateResource command when no changes are detected, no schema is given, and a tag is provided" in {
+        val schema        = Latest(schemas.resources)
+        val current       = ResourceGen.currentState(projectRef, jsonld, schema)
+        val expectedEvent = ResourceTagAdded(myId, projectRef, jsonld.types, 1, tag, 2, epoch, subject)
         eval(
           Some(current),
           UpdateResource(projectRef, projectContext, None, jsonld, 1, caller, Some(tag))
-        ).accepted shouldEqual
-          ResourceTagAdded(
-            myId,
-            projectRef,
-            jsonld.types,
-            1,
-            tag,
-            2,
-            epoch,
-            subject
-          )
+        ).accepted shouldEqual expectedEvent
       }
 
       "create a new event from a RefreshResource command on a new remote context" in {
diff --git a/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/model/ResourceSerializationSuite.scala b/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/model/ResourceSerializationSuite.scala
index 7c288a1348..8581ccd9e7 100644
--- a/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/model/ResourceSerializationSuite.scala
+++ b/delta/sdk/src/test/scala/ch/epfl/bluebrain/nexus/delta/sdk/resources/model/ResourceSerializationSuite.scala
@@ -28,15 +28,16 @@ class ResourceSerializationSuite extends SerializationSuite with ResourceInstanc
   val jsonld           = JsonLdAssembly(myId, source, compacted, expanded, graph, remoteContexts)
 
   // format: off
-  private val created        = ResourceCreated(projectRef, Revision(schemas.resources, 1), projectRef, jsonld, instant, subject, None)
+  private val schemaRev = Revision(schemas.resources, 1)
+  private val created        = ResourceCreated(projectRef, schemaRev, projectRef, jsonld, instant, subject, None)
   private val createdWithTag = created.copy(tag = Some(tag))
-  private val updated        = ResourceUpdated(projectRef, Revision(schemas.resources, 1), projectRef, jsonld, 2, instant, subject, Some(tag))
-  private val refreshed      = ResourceRefreshed(projectRef, Revision(schemas.resources, 1), projectRef, jsonld, 2, instant, subject)
-  private val tagged         = ResourceTagAdded(myId, projectRef, types, 1, UserTag.unsafe("mytag"), 3, instant, subject)
+  private val updated        = ResourceUpdated(projectRef, schemaRev, projectRef, jsonld, 2, instant, subject, Some(tag))
+  private val refreshed      = ResourceRefreshed(projectRef, schemaRev, projectRef, jsonld, 2, instant, subject)
+  private val tagged         = ResourceTagAdded(myId, projectRef, types, 1, tag, 3, instant, subject)
   private val deprecated     = ResourceDeprecated(myId, projectRef, types, 4, instant, subject)
   private val undeprecated   = ResourceUndeprecated(myId, projectRef, types, 5, instant, subject)
   private val tagDeleted     = ResourceTagDeleted(myId, projectRef, types, tag, 5, instant, subject)
-  private val schemaUpdated  = ResourceSchemaUpdated(myId, projectRef, Revision(schemas.resources, 1), projectRef, types, 6, instant, subject)
+  private val schemaUpdated  = ResourceSchemaUpdated(myId, projectRef, schemaRev, projectRef, types, 6, instant, subject, Some(tag))
   // format: on
 
   private val resourcesMapping = List(
diff --git a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/ResourcesSpec.scala b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/ResourcesSpec.scala
index 8543012e09..aa32f48df0 100644
--- a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/ResourcesSpec.scala
+++ b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/ResourcesSpec.scala
@@ -8,8 +8,8 @@ import cats.effect.IO
 import cats.implicits._
 import ch.epfl.bluebrain.nexus.delta.kernel.utils.UrlUtils
 import ch.epfl.bluebrain.nexus.testkit.scalatest.ResourceMatchers.deprecated
-import ch.epfl.bluebrain.nexus.tests.Identity.{Anonymous, ServiceAccount}
 import ch.epfl.bluebrain.nexus.tests.Identity.resources.{Morty, Rick}
+import ch.epfl.bluebrain.nexus.tests.Identity.{Anonymous, ServiceAccount}
 import ch.epfl.bluebrain.nexus.tests.Optics.admin._constrainedBy
 import ch.epfl.bluebrain.nexus.tests.Optics.listing._total
 import ch.epfl.bluebrain.nexus.tests.Optics.{_rev, filterKey, filterMetadataKeys}
@@ -17,7 +17,6 @@ import ch.epfl.bluebrain.nexus.tests.admin.ProjectPayload
 import ch.epfl.bluebrain.nexus.tests.iam.types.Permission.Resources
 import ch.epfl.bluebrain.nexus.tests.resources.SimpleResource
 import ch.epfl.bluebrain.nexus.tests.{BaseIntegrationSpec, Optics, SchemaPayload}
-import io.circe.syntax.{EncoderOps, KeyOps}
 import io.circe.{Json, JsonObject}
 import org.scalatest.Assertion
 import org.scalatest.matchers.{HavePropertyMatchResult, HavePropertyMatcher}
@@ -373,19 +372,11 @@ class ResourcesSpec extends BaseIntegrationSpec {
     }
 
     "succeed" in {
-      for {
-        _ <-
-          deltaClient.put[Json](s"/resources/$project1/test-schema/test-resource:1?rev=1", payload, Rick) {
-            (json, response) =>
-              response.status shouldEqual StatusCodes.OK
-              _rev.getOption(json).value shouldEqual 2
-          }
-        // Sending the same update should not create a new revision
-        _ <- deltaClient.put[Json](s"/resources/$project1/_/test-resource:1?rev=2", payload, Rick) { (json, response) =>
-               response.status shouldEqual StatusCodes.OK
-               _rev.getOption(json).value shouldEqual 2
-             }
-      } yield succeed
+      deltaClient.put[Json](s"/resources/$project1/test-schema/test-resource:1?rev=1", payload, Rick) {
+        (json, response) =>
+          response.status shouldEqual StatusCodes.OK
+          _rev.getOption(json).value shouldEqual 2
+      }
     }
 
     "fetch the update" in {
@@ -860,34 +851,6 @@ class ResourcesSpec extends BaseIntegrationSpec {
 
   }
 
-  "Checking for update changes for a large resource" should {
-    "succeed" in {
-      val id                 = "large"
-      val tpe                = "Random"
-      val largeRandomPayload = {
-        val entry = Json.obj(
-          "array"  := (1 to 100).toList,
-          "string" := "some-value"
-        )
-        (1 to 500).foldLeft(JsonObject("@type" := tpe)) { case (acc, index) =>
-          acc.add(s"prop$index", entry)
-        }
-      }.asJson
-
-      for {
-        _ <- deltaClient.put[Json](s"/resources/$project2/_/test-resource:$id", largeRandomPayload, Rick) {
-               expectCreated
-             }
-        _ <- deltaClient
-               .put[Json](s"/resources/$project2/_/test-resource:$id?rev=1", largeRandomPayload, Rick) {
-                 (json, response) =>
-                   response.status shouldEqual StatusCodes.OK
-                   Optics._rev.getOption(json).value shouldEqual 1
-               }
-      } yield succeed
-    }
-  }
-
   "Passing an unknown suffix on the resource endpoint" should {
 
     val endpoint = s"/resources/$project2/_/test-resource:id/xxx"
diff --git a/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/resources/ResourceUpdatesSpec.scala b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/resources/ResourceUpdatesSpec.scala
new file mode 100644
index 0000000000..0dc74a2d45
--- /dev/null
+++ b/tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/kg/resources/ResourceUpdatesSpec.scala
@@ -0,0 +1,101 @@
+package ch.epfl.bluebrain.nexus.tests.kg.resources
+
+import akka.http.scaladsl.model.StatusCodes
+import ch.epfl.bluebrain.nexus.delta.kernel.utils.UrlUtils
+import ch.epfl.bluebrain.nexus.tests.Identity.listings.Bob
+import ch.epfl.bluebrain.nexus.tests.Optics._rev
+import ch.epfl.bluebrain.nexus.tests.resources.SimpleResource
+import ch.epfl.bluebrain.nexus.tests.{BaseIntegrationSpec, SchemaPayload}
+import io.circe.syntax.{EncoderOps, KeyOps}
+import io.circe.{Json, JsonObject}
+
+class ResourceUpdatesSpec extends BaseIntegrationSpec {
+
+  private val orgId   = genId()
+  private val projId  = genId()
+  private val project = s"$orgId/$projId"
+
+  private val schema        = "test-schema"
+  private val unconstrained = "https://bluebrain.github.io/nexus/schemas/unconstrained.json"
+
+  override def beforeAll(): Unit = {
+    super.beforeAll()
+    val setup = for {
+      _             <- createProjects(Bob, orgId, projId)
+      schemaPayload <- SchemaPayload.loadSimple()
+      _             <- deltaClient.put[Json](s"/schemas/$project/$schema", schemaPayload, Bob) { expectCreated }
+    } yield ()
+    setup.accepted
+  }
+
+  private def createResource(schema: String, payload: Json) = {
+    val resourceId = genString()
+    deltaClient
+      .put[Json](s"/resources/$project/${UrlUtils.encode(schema)}/$resourceId", payload, Bob) { expectCreated }
+      .as(resourceId)
+  }
+
+  private def updateResource(resourceId: String, rev: Int, schema: String, payload: Json) =
+    deltaClient.put[Json](s"/resources/$project/${UrlUtils.encode(schema)}/$resourceId?rev=$rev", payload, Bob) {
+      expectOk
+    }
+
+  private def assertRev(resourceId: String, expectedRev: Int) =
+    deltaClient.get[Json](s"/resources/$project/_/$resourceId", Bob) { (json, response) =>
+      response.status shouldEqual StatusCodes.OK
+      _rev.getOption(json).value shouldEqual expectedRev
+    }
+
+  "Updating a resource" should {
+
+    "create a new revision if the payload has changed" in {
+      for {
+        payload        <- SimpleResource.sourcePayload(42)
+        resourceId     <- createResource(schema, payload)
+        updatedPayload <- SimpleResource.sourcePayload(100)
+        _              <- updateResource(resourceId, 1, schema, updatedPayload)
+        _              <- assertRev(resourceId, 2)
+      } yield succeed
+    }
+
+    "not create a new revision if the payload and the schema haven't changed" in {
+      for {
+        payload    <- SimpleResource.sourcePayload(42)
+        resourceId <- createResource(schema, payload)
+        _          <- updateResource(resourceId, 1, "test-schema", payload)
+        _          <- assertRev(resourceId, 1)
+      } yield succeed
+    }
+
+    "create a new revision if the schema has changed" in {
+      for {
+        payload    <- SimpleResource.sourcePayload(42)
+        resourceId <- createResource(schema, payload)
+        _          <- updateResource(resourceId, 1, unconstrained, payload)
+        _          <- assertRev(resourceId, 2)
+      } yield succeed
+    }
+  }
+
+  "Checking for update changes for a large resource" should {
+    "succeed" in {
+      val tpe                = "Random"
+      val largeRandomPayload = {
+        val entry = Json.obj(
+          "array"  := (1 to 100).toList,
+          "string" := "some-value"
+        )
+        (1 to 500).foldLeft(JsonObject("@type" := tpe)) { case (acc, index) =>
+          acc.add(s"prop$index", entry)
+        }
+      }.asJson
+
+      for {
+        resourceId <- createResource(unconstrained, largeRandomPayload)
+        _          <- updateResource(resourceId, 1, unconstrained, largeRandomPayload)
+        _          <- assertRev(resourceId, 1)
+      } yield succeed
+    }
+  }
+
+}