[EDIFIKANA] Make createProperty atomic via RPC (DS-PROP-010)

edifikana/back-end/docs/datastore-test-catalog.md

@@ -112,6 +112,14 @@ datastore query bodies, and mapper functions side by side — not assumptions fr
    left behind. At minimum, confirm the current `IllegalStateException` fallback
    (`?: throw IllegalStateException(...)`) only covers the case where the insert returns zero rows
    silently, not the case where it throws. → **DS-PROP-010**
+
+   **Fixed:** `createProperty` now performs both inserts inside a single atomic
+   `create_property_with_owner` RPC (`20260726000000_create_property_with_owner_function.sql`),
+   mirroring the `upsert_rent_config`/`transfer_ownership` pattern — Postgres executes one RPC
+   call as one transaction, so a failure on the owner-mapping insert rolls back the property
+   insert too. Confirmed live: `DS-PROP-010b` fabricates a nonexistent `creatorUserId` (an FK
+   violation on `user_property_mapping_user_id_fkey`), and a raw, untyped select on `properties`
+   confirms no orphaned row survives.
 4. **Six datastores skip the `deleted_at IS NULL` filter on their update path, unlike the rest —
    soft-deleted rows can be silently mutated.** Contrast the consistent group (CommonArea,
    Document, Occupant, PaymentRecord, Task, Unit — all filter `deletedAt isExact null` in their
@@ -395,7 +403,7 @@ datastore query bodies, and mapper functions side by side — not assumptions fr
 
 | ID | Priority | Case | Expected |
 |---|---|---|---|
-| DS-PROP-010 | **P0** | Non-atomic two-insert `createProperty` | Finding #3 |
+| DS-PROP-010 | **P0** | Non-atomic two-insert `createProperty` | Fixed — both inserts now run inside a single atomic `create_property_with_owner` RPC; `DS-PROP-010b` confirms no orphaned property row survives an owner-mapping FK failure |
 | DS-PROP-011 | **P0** | `updateProperty` on a soft-deleted property | Fixed — guard now returns `Result.failure` (zero rows matched), entry left unmutated |
 | DS-PROP-001 | P1 | `getProperties(userId)` via `v_user_properties` — confirm it excludes soft-deleted properties | Confirmed via migration that `v_user_properties` is defined to exclude soft-deleted properties (`20260317000004...:20-25`) — expected to pass; still worth a regression test given the correctness lives entirely in the view definition (same reasoning as DS-EMP-004) |
 | DS-PROP-002 | P1 | `getProperties` for a user with zero property mappings | `emptyList()` |

edifikana/back-end/src/integTest/kotlin/com/cramsan/edifikana/server/datastore/supabase/SupabasePropertyDatastoreIntegrationTest.kt

@@ -6,6 +6,7 @@ import com.cramsan.edifikana.lib.model.property.PropertyId
 import com.cramsan.edifikana.lib.model.user.UserId
 import com.cramsan.edifikana.server.service.models.Property
 import com.cramsan.framework.utils.uuid.UUID
+import io.github.jan.supabase.postgrest.postgrest
 import kotlin.test.BeforeTest
 import kotlin.test.Test
 import kotlin.test.assertEquals
@@ -31,11 +32,10 @@ class SupabasePropertyDatastoreIntegrationTest : SupabaseIntegrationTest() {
     @Test
     fun `DS-PROP-010 createProperty should create both the property and its owning user_property_mapping`(): Unit =
         runBlocking {
-            // Arrange & Act - the two inserts (property, then user_property_mapping) are not
-            // wrapped in a transaction; confirm the happy-path shape: the property is
-            // immediately visible via getProperties(creatorUserId), proving the mapping row
-            // landed alongside it. True mid-sequence failure injection (to prove the orphan
-            // scenario) isn't reachable via the public datastore API alone.
+            // Arrange & Act - createProperty now performs both inserts atomically via the
+            // create_property_with_owner RPC; confirm the happy-path shape still works: the
+            // property is immediately visible via getProperties(creatorUserId), proving the
+            // mapping row landed alongside it.
             val result = propertyDatastore.createProperty(
                 name = "${test_prefix}_AtomicityCheck",
                 address = "1 Atomic Way",
@@ -51,6 +51,35 @@ class SupabasePropertyDatastoreIntegrationTest : SupabaseIntegrationTest() {
             assertTrue(ownedProperties.any { it.id == property.id })
         }
 
+    @Test
+    fun `DS-PROP-010b createProperty should not leave an orphaned property row when the owner mapping insert fails`(): Unit =
+        runBlocking {
+            // Arrange - a fabricated userId that doesn't exist in `users` violates
+            // user_property_mapping_user_id_fkey. Before this fix, the property insert (the RPC's
+            // first statement) would have already committed by the time this FK violation fires;
+            // now both inserts run inside one RPC call/transaction, so the whole thing rolls back.
+            val nonExistentUserId = UserId(UUID.random())
+            val propertyName = "${test_prefix}_OrphanCheck"
+
+            // Act
+            val result = propertyDatastore.createProperty(
+                name = propertyName,
+                address = "1 Orphan Ln",
+                creatorUserId = nonExistentUserId,
+                organizationId = testOrg!!,
+                imageUrl = null,
+            )
+
+            // Assert - the call fails, and no orphaned property row was left behind. A raw,
+            // untyped select is required here (rather than getProperty/getProperties) since those
+            // only prove a property is unreachable for a user, not that no row exists at all.
+            assertTrue(result.isFailure)
+            val orphanedRows = supabase.postgrest.from("properties").select {
+                filter { eq("name", propertyName) }
+            }.decodeList<kotlinx.serialization.json.JsonObject>()
+            assertTrue(orphanedRows.isEmpty())
+        }
+
     @Test
     fun `DS-PROP-011 updateProperty should fail and not mutate a soft-deleted property`(): Unit =
         runBlocking {

edifikana/back-end/src/main/kotlin/com/cramsan/edifikana/server/datastore/PropertyDatastore.kt

@@ -12,7 +12,10 @@ import com.cramsan.framework.annotations.BackendDatastore
 @BackendDatastore
 interface PropertyDatastore {
     /**
-     * Creates a new property. Returns the [Result] of the operation with the created [Property].
+     * Creates a new property and associates it with its creator in a single atomic operation
+     * (via the `create_property_with_owner` RPC) - if the owner association fails, the property
+     * insert is rolled back too, so no ownerless property is ever left behind. Returns the
+     * [Result] of the operation with the created [Property].
      */
     suspend fun createProperty(
         name: String,

edifikana/back-end/src/main/kotlin/com/cramsan/edifikana/server/datastore/supabase/SupabaseMappers.kt

@@ -225,24 +225,6 @@ fun EmployeeEntity.toEmployee(): Employee {
     )
 }
 
-/**
- * Maps a [CreatePropertyRequest] to the [PropertyEntity.CreatePropertyEntity] model.
- */
-
-fun CreatePropertyEntity(
-    name: String,
-    address: String,
-    organizationId: OrganizationId,
-    imageUrl: String? = null,
-): PropertyEntity.CreatePropertyEntity {
-    return PropertyEntity.CreatePropertyEntity(
-        name = name,
-        address = address,
-        organizationId = organizationId,
-        imageUrl = imageUrl?.let { Url(it) },
-    )
-}
-
 /**
  * Maps a [PropertyEntity] to the [Property] model.
  */

edifikana/back-end/src/main/kotlin/com/cramsan/edifikana/server/datastore/supabase/SupabasePropertyDatastore.kt

@@ -7,14 +7,17 @@ import com.cramsan.edifikana.lib.model.user.UserId
 import com.cramsan.edifikana.server.datastore.PropertyDatastore
 import com.cramsan.edifikana.server.datastore.supabase.models.OrganizationEntity
 import com.cramsan.edifikana.server.datastore.supabase.models.PropertyEntity
-import com.cramsan.edifikana.server.datastore.supabase.models.UserPropertyMappingEntity
 import com.cramsan.edifikana.server.datastore.supabase.models.UserPropertyViewEntity
 import com.cramsan.edifikana.server.service.models.Property
 import com.cramsan.framework.annotations.BackendDatastore
 import com.cramsan.framework.core.runSuspendCatching
 import com.cramsan.framework.logging.logD
 import com.cramsan.framework.utils.exceptions.ClientRequestExceptions
 import io.github.jan.supabase.postgrest.Postgrest
+import kotlinx.serialization.SerialName
+import kotlinx.serialization.Serializable
+import kotlinx.serialization.json.Json
+import kotlinx.serialization.json.jsonObject
 import kotlin.time.Clock
 
 /**
@@ -47,36 +50,19 @@ class SupabasePropertyDatastore(private val postgrest: Postgrest, private val cl
                 "No organization with id $organizationId found in database",
             )
 
-            val requestEntity: PropertyEntity.CreatePropertyEntity =
-                CreatePropertyEntity(
-                    name = name,
-                    address = address,
-                    organizationId = organizationId,
-                    imageUrl = imageUrl,
+            // Both inserts (properties + user_property_mapping) happen inside a single RPC call,
+            // which Postgres executes as one transaction - if the owner-mapping insert fails,
+            // the property insert is rolled back too, instead of leaving an orphaned property.
+            val params =
+                CreatePropertyWithOwnerRpcParams(
+                    pName = name,
+                    pAddress = address,
+                    pOrganizationId = organizationId.id,
+                    pImageUrl = imageUrl,
+                    pCreatorUserId = creatorUserId.userId,
                 )
-
-            // Insert the property into the database and select the created entity
-            val createdProperty =
-                postgrest
-                    .from(PropertyEntity.COLLECTION)
-                    .insert(requestEntity) {
-                        select()
-                    }.decodeSingle<PropertyEntity>()
-
-            // Plain insert is safe: createdProperty.id is a freshly generated UUID, so no
-            // concurrent call can produce the same (userId, propertyId) pair.
-            postgrest
-                .from(UserPropertyMappingEntity.COLLECTION)
-                .insert(
-                    UserPropertyMappingEntity.CreateUserPropertyMappingEntity(
-                        userId = creatorUserId,
-                        propertyId = createdProperty.id,
-                    ),
-                ) {
-                    select()
-                }.decodeSingleOrNull<UserPropertyMappingEntity>() ?: run {
-                throw IllegalStateException("Failed to associate property with user")
-            }
+            val jsonParams = Json.encodeToJsonElement(CreatePropertyWithOwnerRpcParams.serializer(), params).jsonObject
+            val createdProperty = postgrest.rpc(RPC_CREATE_PROPERTY_WITH_OWNER, jsonParams).decodeAs<PropertyEntity>()
 
             logD(TAG, "Property created propertyId: %s", createdProperty.id)
             createdProperty.toProperty()
@@ -215,8 +201,22 @@ class SupabasePropertyDatastore(private val postgrest: Postgrest, private val cl
             true
         }
 
+    /**
+     * Parameters for the `create_property_with_owner` RPC, which inserts the property and its
+     * owner mapping in a single transaction.
+     */
+    @Serializable
+    private data class CreatePropertyWithOwnerRpcParams(
+        @SerialName("p_name") val pName: String,
+        @SerialName("p_address") val pAddress: String,
+        @SerialName("p_organization_id") val pOrganizationId: String,
+        @SerialName("p_image_url") val pImageUrl: String?,
+        @SerialName("p_creator_user_id") val pCreatorUserId: String,
+    )
+
     companion object {
         const val TAG = "SupabasePropertyDatastore"
         const val VIEW_USER_PROPERTIES = "v_user_properties"
+        const val RPC_CREATE_PROPERTY_WITH_OWNER = "create_property_with_owner"
     }
 }

edifikana/back-end/src/main/kotlin/com/cramsan/edifikana/server/datastore/supabase/models/PropertyEntity.kt

@@ -27,18 +27,4 @@ data class PropertyEntity(
     companion object {
         const val COLLECTION = "properties"
     }
-
-    /**
-     * Entity representing a new property.
-     */
-    @Serializable
-    @DatabaseModel
-    data class CreatePropertyEntity(
-        val name: String,
-        val address: String,
-        @SerialName("organization_id")
-        val organizationId: OrganizationId,
-        @SerialName("image_url")
-        val imageUrl: Url? = null,
-    )
 }

edifikana/back-end/supabase/migrations/20260726000000_create_property_with_owner_function.sql

@@ -0,0 +1,48 @@
+-- ============================================================================
+-- Migration: Add create_property_with_owner RPC function
+-- Datastore test catalog: DS-PROP-010
+-- ============================================================================
+-- SupabasePropertyDatastore.createProperty previously issued two separate
+-- Postgrest calls: INSERT into properties, then INSERT into
+-- user_property_mapping linking the new property to its creator. If the
+-- second call failed for any reason (constraint violation, transient network
+-- failure, cancelled coroutine), the property from the first call was already
+-- committed - an orphaned, ownerless, permanently invisible-but-present
+-- property (v_user_properties, and therefore getProperties(userId), only
+-- surfaces properties with a user_property_mapping row).
+--
+-- This function performs both inserts inside a single plpgsql function body.
+-- Postgres executes one RPC call as one transaction, so if the second INSERT
+-- raises, the first is rolled back too - no partial state is ever visible.
+--
+-- SECURITY INVOKER + SET search_path = public follows the hardened shape of
+-- upsert_rent_config (20260429000000_revoke_function_execute_grants.sql).
+-- ============================================================================
+
+CREATE OR REPLACE FUNCTION public.create_property_with_owner(
+    p_name            TEXT,
+    p_address         TEXT,
+    p_organization_id UUID,
+    p_image_url       TEXT,
+    p_creator_user_id UUID
+)
+RETURNS properties
+LANGUAGE plpgsql
+SECURITY INVOKER
+SET search_path = public
+AS $$
+DECLARE
+    v_property properties;
+BEGIN
+    INSERT INTO properties (name, address, organization_id, image_url)
+    VALUES (p_name, p_address, p_organization_id, p_image_url)
+    RETURNING * INTO v_property;
+
+    INSERT INTO user_property_mapping (user_id, property_id)
+    VALUES (p_creator_user_id, v_property.id);
+
+    RETURN v_property;
+END;
+$$;
+
+GRANT EXECUTE ON FUNCTION public.create_property_with_owner(TEXT, TEXT, UUID, TEXT, UUID) TO service_role;