From 986f02ce64aaa7eb5db436ef8023643f9a27bd2d Mon Sep 17 00:00:00 2001 From: zichun Date: Thu, 9 Apr 2026 09:54:47 +0800 Subject: [PATCH] refactor(metadata): HasExt marker + applyTo/parseExt helpers on ExtJsonValidator --- CLAUDE.md | 2 +- PROGRESS.md | 6 +++--- api/api-v1/src/main/kotlin/org/vibeerp/api/v1/entity/HasExt.kt | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/application/LocationService.kt | 45 ++++++++++++++++----------------------------- pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/domain/Location.kt | 12 ++++++++++-- pbc/pbc-inventory/src/test/kotlin/org/vibeerp/pbc/inventory/application/LocationServiceTest.kt | 9 ++++++++- pbc/pbc-orders-purchase/src/main/kotlin/org/vibeerp/pbc/orders/purchase/application/PurchaseOrderService.kt | 34 +++++++++++----------------------- pbc/pbc-orders-purchase/src/main/kotlin/org/vibeerp/pbc/orders/purchase/domain/PurchaseOrder.kt | 12 ++++++++++-- pbc/pbc-orders-purchase/src/test/kotlin/org/vibeerp/pbc/orders/purchase/application/PurchaseOrderServiceTest.kt | 4 +++- pbc/pbc-orders-sales/src/main/kotlin/org/vibeerp/pbc/orders/sales/application/SalesOrderService.kt | 34 +++++++++++----------------------- pbc/pbc-orders-sales/src/main/kotlin/org/vibeerp/pbc/orders/sales/domain/SalesOrder.kt | 12 ++++++++++-- pbc/pbc-orders-sales/src/test/kotlin/org/vibeerp/pbc/orders/sales/application/SalesOrderServiceTest.kt | 6 +++++- pbc/pbc-partners/src/main/kotlin/org/vibeerp/pbc/partners/application/PartnerService.kt | 69 ++++++++++++++++++++++----------------------------------------------- pbc/pbc-partners/src/main/kotlin/org/vibeerp/pbc/partners/domain/Partner.kt | 12 ++++++++++-- pbc/pbc-partners/src/test/kotlin/org/vibeerp/pbc/partners/application/PartnerServiceTest.kt | 10 +++++++--- platform/platform-metadata/src/main/kotlin/org/vibeerp/platform/metadata/customfield/ExtJsonValidator.kt | 46 ++++++++++++++++++++++++++++++++++++++++++++++ platform/platform-metadata/src/test/kotlin/org/vibeerp/platform/metadata/customfield/ExtJsonValidatorTest.kt | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 17 files changed, 307 insertions(+), 141 deletions(-) create mode 100644 api/api-v1/src/main/kotlin/org/vibeerp/api/v1/entity/HasExt.kt diff --git a/CLAUDE.md b/CLAUDE.md index 2c44f03..48507b2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -96,7 +96,7 @@ plugins (incl. ref) depend on: api/api-v1 only **Foundation complete; first business surface in place.** As of the latest commit: - **18 Gradle subprojects** built and tested. The dependency rule (PBCs never import each other; plug-ins only see `api.v1`) is enforced at configuration time by the root `build.gradle.kts`. -- **240 unit tests across 18 modules**, all green. `./gradlew build` is the canonical full build. +- **246 unit tests across 18 modules**, all green. `./gradlew build` is the canonical full build. - **All 9 cross-cutting platform services live and smoke-tested end-to-end** against real Postgres: auth (P4.1), authorization with `@RequirePermission` + JWT roles claim + AOP enforcement (P4.3), plug-in HTTP endpoints (P1.3), plug-in linter (P1.2), plug-in-owned Liquibase + typed SQL (P1.4), event bus + transactional outbox (P1.7), metadata loader + custom-field validation (P1.5 + P3.4), ICU4J translator with per-plug-in locale chain (P1.6), plus the audit/principal context bridge. - **8 of 10 core PBCs implemented end-to-end** (`pbc-identity`, `pbc-catalog`, `pbc-partners`, `pbc-inventory`, `pbc-orders-sales`, `pbc-orders-purchase`, `pbc-finance`, `pbc-production`). **The full buy-sell-make loop works**: a purchase order receives stock via `PURCHASE_RECEIPT`, a sales order ships stock via `SALES_SHIPMENT`, and a work order produces stock via `PRODUCTION_RECEIPT`. All three PBCs feed the same `inventory__stock_movement` ledger via the same `InventoryApi.recordMovement` facade. - **Event-driven cross-PBC integration is live in BOTH directions and the consumer reacts to the full lifecycle.** Six typed events under `org.vibeerp.api.v1.event.orders.*` are published from `SalesOrderService` and `PurchaseOrderService` inside the same `@Transactional` method as their state changes. **pbc-finance** subscribes to ALL SIX of them via the api.v1 `EventBus.subscribe(eventType, listener)` typed-class overload: confirm events POST AR/AP rows; ship/receive events SETTLE them; cancel events REVERSE them. Each transition is idempotent under at-least-once delivery — re-applying the same destination status is a no-op. Cancel-from-DRAFT is a clean no-op because no `*ConfirmedEvent` was ever published. pbc-finance has no source dependency on pbc-orders-*; it reaches them only through events. diff --git a/PROGRESS.md b/PROGRESS.md index 922800d..3879afd 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -10,11 +10,11 @@ | | | |---|---| -| **Latest version** | v0.19.0 (pbc-production v2 — IN_PROGRESS + BOM + scrap) | -| **Latest commit** | `75a75ba feat(production): P5.7 v2 — IN_PROGRESS state + BOM auto-issue + scrap flow` | +| **Latest version** | v0.19.1 (HasExt marker interface + ExtJsonValidator helpers) | +| **Latest commit** | `TBD refactor(metadata): HasExt marker + applyTo/parseExt helpers on ExtJsonValidator` | | **Repo** | https://github.com/reporkey/vibe-erp | | **Modules** | 18 | -| **Unit tests** | 240, all green | +| **Unit tests** | 246, all green | | **End-to-end smoke runs** | An SO confirmed with 2 lines auto-spawns 2 draft work orders via `SalesOrderConfirmedSubscriber`; completing one credits the finished-good stock via `PRODUCTION_RECEIPT`, cancelling the other flips its status, and a manual WO can still be created with no source SO. All in one run: 6 outbox rows DISPATCHED across `orders_sales.SalesOrder` and `production.WorkOrder` topics; pbc-finance still writes its AR row for the underlying SO; the `inventory__stock_movement` ledger carries the production receipt tagged `WO:`. First PBC that REACTS to another PBC's events by creating new business state (not just derived reporting state). | | **Real PBCs implemented** | 8 of 10 (`pbc-identity`, `pbc-catalog`, `pbc-partners`, `pbc-inventory`, `pbc-orders-sales`, `pbc-orders-purchase`, `pbc-finance`, `pbc-production`) | | **Plug-ins serving HTTP** | 1 (reference printing-shop, 7 endpoints + own DB schema + own metadata + own i18n bundles) | diff --git a/api/api-v1/src/main/kotlin/org/vibeerp/api/v1/entity/HasExt.kt b/api/api-v1/src/main/kotlin/org/vibeerp/api/v1/entity/HasExt.kt new file mode 100644 index 0000000..f022e89 --- /dev/null +++ b/api/api-v1/src/main/kotlin/org/vibeerp/api/v1/entity/HasExt.kt @@ -0,0 +1,64 @@ +package org.vibeerp.api.v1.entity + +/** + * Marker interface for every entity that participates in Tier 1 + * customization — i.e. any entity with a JSONB `ext` column that + * must be validated against [CustomField] declarations on save and + * decoded for response DTOs on read. + * + * **Why this is in api.v1, not platform.metadata:** plug-ins define + * their own entities and need to opt into the same validation and + * serialization path as the framework's core PBCs. Keeping the + * marker in api.v1 means a plug-in's `Plate` or `InkRecipe` can + * implement `HasExt` and automatically get: + * - validation against its `metadata__custom_field` declarations, + * - canonical JSON serialization into the column, + * - safe parse-back into a `Map` for response DTOs, + * all through the same `ExtJsonValidator.applyTo(...)` / + * `.parseExt(...)` helpers that the core PBCs use. No plug-in code + * should need to reach into `platform.metadata.internal.*` for ext + * handling. + * + * **Interface surface is deliberately tiny** — two members. + * + * - [extEntityName] is the stable string key used to look up + * declared fields in the custom field registry. The convention is + * `.` (e.g. `"partners.Partner"`, `"inventory.Location"`). + * Plug-ins are expected to namespace theirs under the plug-in id + * (e.g. `"printing-shop.Plate"`). + * - [ext] is the serialized JSONB column. Entities should back this + * with a `@Column(columnDefinition = "jsonb") var ext: String = + * "{}"`. The `ExtJsonValidator.applyTo(...)` helper writes the + * canonical form here; `ExtJsonValidator.parseExt(...)` reads it + * back. + * + * **Why `ext` is a `String`, not a `Map`:** Hibernate writes the JSON + * column as a string (or its `@JdbcTypeCode(SqlTypes.JSON)` wrapper + * — effectively the same thing). Exposing it as a Map here would + * force plug-ins to depend on Jackson, which `api.v1` explicitly + * does NOT. Keeping it a String means the only Jackson touchpoint + * lives inside `ExtJsonValidator`, which is already in + * `platform.metadata`. + * + * **Does NOT extend [Entity].** Not every Entity has custom fields; + * not every HasExt needs an Id (though in practice they always do). + * Keeping the two marker interfaces orthogonal means a plug-in can + * choose which behaviors it opts into without inheritance bloat. + */ +public interface HasExt { + /** + * The stable entity-name key under which this entity's custom + * fields are declared in `metadata__custom_field`. Convention: + * `.` for core; `.` for plug-ins. + */ + public val extEntityName: String + + /** + * The serialized JSONB column. Managed by + * `ExtJsonValidator.applyTo(...)` on save and decoded by + * `ExtJsonValidator.parseExt(...)` on read. Should default to + * `"{}"` on the entity class so an un-customized row still + * round-trips through JSON cleanly. + */ + public var ext: String +} diff --git a/pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/application/LocationService.kt b/pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/application/LocationService.kt index 0276b09..d9c1ce1 100644 --- a/pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/application/LocationService.kt +++ b/pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/application/LocationService.kt @@ -1,7 +1,5 @@ package org.vibeerp.pbc.inventory.application -import com.fasterxml.jackson.databind.ObjectMapper -import com.fasterxml.jackson.module.kotlin.registerKotlinModule import org.springframework.stereotype.Service import org.springframework.transaction.annotation.Transactional import org.vibeerp.pbc.inventory.domain.Location @@ -32,8 +30,6 @@ class LocationService( private val extValidator: ExtJsonValidator, ) { - private val jsonMapper: ObjectMapper = ObjectMapper().registerKotlinModule() - @Transactional(readOnly = true) fun list(): List = locations.findAll() @@ -47,17 +43,14 @@ class LocationService( require(!locations.existsByCode(command.code)) { "location code '${command.code}' is already taken" } - val canonicalExt = extValidator.validate(ENTITY_NAME, command.ext) - return locations.save( - Location( - code = command.code, - name = command.name, - type = command.type, - active = command.active, - ).also { - it.ext = jsonMapper.writeValueAsString(canonicalExt) - }, + val location = Location( + code = command.code, + name = command.name, + type = command.type, + active = command.active, ) + extValidator.applyTo(location, command.ext) + return locations.save(location) } fun update(id: UUID, command: UpdateLocationCommand): Location { @@ -67,10 +60,7 @@ class LocationService( command.name?.let { location.name = it } command.type?.let { location.type = it } command.active?.let { location.active = it } - if (command.ext != null) { - val canonicalExt = extValidator.validate(ENTITY_NAME, command.ext) - location.ext = jsonMapper.writeValueAsString(canonicalExt) - } + extValidator.applyTo(location, command.ext) return location } @@ -81,17 +71,14 @@ class LocationService( location.active = false } - @Suppress("UNCHECKED_CAST") - fun parseExt(location: Location): Map = try { - if (location.ext.isBlank()) emptyMap() - else jsonMapper.readValue(location.ext, Map::class.java) as Map - } catch (ex: Throwable) { - emptyMap() - } - - companion object { - const val ENTITY_NAME: String = "Location" - } + /** + * Convenience passthrough for response mappers — delegates to + * [ExtJsonValidator.parseExt]. Kept here as a thin wrapper so + * the controller's mapper doesn't have to inject the validator + * directly. + */ + fun parseExt(location: Location): Map = + extValidator.parseExt(location) } data class CreateLocationCommand( diff --git a/pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/domain/Location.kt b/pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/domain/Location.kt index bbed567..f9ad1f0 100644 --- a/pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/domain/Location.kt +++ b/pbc/pbc-inventory/src/main/kotlin/org/vibeerp/pbc/inventory/domain/Location.kt @@ -7,6 +7,7 @@ import jakarta.persistence.Enumerated import jakarta.persistence.Table import org.hibernate.annotations.JdbcTypeCode import org.hibernate.type.SqlTypes +import org.vibeerp.api.v1.entity.HasExt import org.vibeerp.platform.persistence.audit.AuditedJpaEntity /** @@ -50,7 +51,7 @@ class Location( name: String, type: LocationType, active: Boolean = true, -) : AuditedJpaEntity() { +) : AuditedJpaEntity(), HasExt { @Column(name = "code", nullable = false, length = 64) var code: String = code @@ -67,10 +68,17 @@ class Location( @Column(name = "ext", nullable = false, columnDefinition = "jsonb") @JdbcTypeCode(SqlTypes.JSON) - var ext: String = "{}" + override var ext: String = "{}" + + override val extEntityName: String get() = ENTITY_NAME override fun toString(): String = "Location(id=$id, code='$code', type=$type, active=$active)" + + companion object { + /** Key under which Location's custom fields are declared in `metadata__custom_field`. */ + const val ENTITY_NAME: String = "Location" + } } /** diff --git a/pbc/pbc-inventory/src/test/kotlin/org/vibeerp/pbc/inventory/application/LocationServiceTest.kt b/pbc/pbc-inventory/src/test/kotlin/org/vibeerp/pbc/inventory/application/LocationServiceTest.kt index db1948e..2cd90db 100644 --- a/pbc/pbc-inventory/src/test/kotlin/org/vibeerp/pbc/inventory/application/LocationServiceTest.kt +++ b/pbc/pbc-inventory/src/test/kotlin/org/vibeerp/pbc/inventory/application/LocationServiceTest.kt @@ -6,12 +6,15 @@ import assertk.assertions.hasMessage import assertk.assertions.isEqualTo import assertk.assertions.isFalse import assertk.assertions.isInstanceOf +import io.mockk.Runs import io.mockk.every +import io.mockk.just import io.mockk.mockk import io.mockk.slot import io.mockk.verify import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import org.vibeerp.api.v1.entity.HasExt import org.vibeerp.pbc.inventory.domain.Location import org.vibeerp.pbc.inventory.domain.LocationType import org.vibeerp.pbc.inventory.infrastructure.LocationJpaRepository @@ -29,7 +32,11 @@ class LocationServiceTest { fun setUp() { locations = mockk() extValidator = mockk() - every { extValidator.validate(any(), any()) } answers { secondArg() ?: emptyMap() } + // applyTo is a void method with side effects on the entity; + // for unit tests we treat it as a no-op. Tests that exercise + // the ext path directly can stub concrete behavior. + every { extValidator.applyTo(any(), any()) } just Runs + every { extValidator.parseExt(any()) } returns emptyMap() service = LocationService(locations, extValidator) } diff --git a/pbc/pbc-orders-purchase/src/main/kotlin/org/vibeerp/pbc/orders/purchase/application/PurchaseOrderService.kt b/pbc/pbc-orders-purchase/src/main/kotlin/org/vibeerp/pbc/orders/purchase/application/PurchaseOrderService.kt index 01a7274..2d72dc7 100644 --- a/pbc/pbc-orders-purchase/src/main/kotlin/org/vibeerp/pbc/orders/purchase/application/PurchaseOrderService.kt +++ b/pbc/pbc-orders-purchase/src/main/kotlin/org/vibeerp/pbc/orders/purchase/application/PurchaseOrderService.kt @@ -1,7 +1,5 @@ package org.vibeerp.pbc.orders.purchase.application -import com.fasterxml.jackson.databind.ObjectMapper -import com.fasterxml.jackson.module.kotlin.registerKotlinModule import org.springframework.stereotype.Service import org.springframework.transaction.annotation.Transactional import org.vibeerp.api.v1.event.EventBus @@ -55,8 +53,6 @@ class PurchaseOrderService( private val eventBus: EventBus, ) { - private val jsonMapper: ObjectMapper = ObjectMapper().registerKotlinModule() - @Transactional(readOnly = true) fun list(): List = orders.findAll() @@ -112,8 +108,6 @@ class PurchaseOrderService( acc + (line.quantity * line.unitPrice) } - val canonicalExt = extValidator.validate(ENTITY_NAME, command.ext) - val order = PurchaseOrder( code = command.code, partnerCode = command.partnerCode, @@ -122,9 +116,8 @@ class PurchaseOrderService( expectedDate = command.expectedDate, currencyCode = command.currencyCode, totalAmount = total, - ).also { - it.ext = jsonMapper.writeValueAsString(canonicalExt) - } + ) + extValidator.applyTo(order, command.ext) for (line in command.lines) { order.lines += PurchaseOrderLine( purchaseOrder = order, @@ -160,9 +153,8 @@ class PurchaseOrderService( command.expectedDate?.let { order.expectedDate = it } command.currencyCode?.let { order.currencyCode = it } - if (command.ext != null) { - order.ext = jsonMapper.writeValueAsString(extValidator.validate(ENTITY_NAME, command.ext)) - } + // applyTo() is null-safe — a null command.ext is a no-op. + extValidator.applyTo(order, command.ext) if (command.lines != null) { require(command.lines.isNotEmpty()) { @@ -309,17 +301,13 @@ class PurchaseOrderService( return order } - @Suppress("UNCHECKED_CAST") - fun parseExt(order: PurchaseOrder): Map = try { - if (order.ext.isBlank()) emptyMap() - else jsonMapper.readValue(order.ext, Map::class.java) as Map - } catch (ex: Throwable) { - emptyMap() - } - - companion object { - const val ENTITY_NAME: String = "PurchaseOrder" - } + /** + * Convenience passthrough for response mappers — delegates to + * [ExtJsonValidator.parseExt]. Returns an empty map on an empty + * or unparseable column so response rendering never 500s. + */ + fun parseExt(order: PurchaseOrder): Map = + extValidator.parseExt(order) } data class CreatePurchaseOrderCommand( diff --git a/pbc/pbc-orders-purchase/src/main/kotlin/org/vibeerp/pbc/orders/purchase/domain/PurchaseOrder.kt b/pbc/pbc-orders-purchase/src/main/kotlin/org/vibeerp/pbc/orders/purchase/domain/PurchaseOrder.kt index 79e66ab..1efa288 100644 --- a/pbc/pbc-orders-purchase/src/main/kotlin/org/vibeerp/pbc/orders/purchase/domain/PurchaseOrder.kt +++ b/pbc/pbc-orders-purchase/src/main/kotlin/org/vibeerp/pbc/orders/purchase/domain/PurchaseOrder.kt @@ -11,6 +11,7 @@ import jakarta.persistence.OrderBy import jakarta.persistence.Table import org.hibernate.annotations.JdbcTypeCode import org.hibernate.type.SqlTypes +import org.vibeerp.api.v1.entity.HasExt import org.vibeerp.platform.persistence.audit.AuditedJpaEntity import java.math.BigDecimal import java.time.LocalDate @@ -70,7 +71,7 @@ class PurchaseOrder( expectedDate: LocalDate? = null, currencyCode: String, totalAmount: BigDecimal = BigDecimal.ZERO, -) : AuditedJpaEntity() { +) : AuditedJpaEntity(), HasExt { @Column(name = "code", nullable = false, length = 64) var code: String = code @@ -96,7 +97,9 @@ class PurchaseOrder( @Column(name = "ext", nullable = false, columnDefinition = "jsonb") @JdbcTypeCode(SqlTypes.JSON) - var ext: String = "{}" + override var ext: String = "{}" + + override val extEntityName: String get() = ENTITY_NAME @OneToMany( mappedBy = "purchaseOrder", @@ -109,6 +112,11 @@ class PurchaseOrder( override fun toString(): String = "PurchaseOrder(id=$id, code='$code', supplier='$partnerCode', status=$status, total=$totalAmount $currencyCode)" + + companion object { + /** Key under which PurchaseOrder's custom fields are declared in `metadata__custom_field`. */ + const val ENTITY_NAME: String = "PurchaseOrder" + } } /** diff --git a/pbc/pbc-orders-purchase/src/test/kotlin/org/vibeerp/pbc/orders/purchase/application/PurchaseOrderServiceTest.kt b/pbc/pbc-orders-purchase/src/test/kotlin/org/vibeerp/pbc/orders/purchase/application/PurchaseOrderServiceTest.kt index 409df05..f93b2d2 100644 --- a/pbc/pbc-orders-purchase/src/test/kotlin/org/vibeerp/pbc/orders/purchase/application/PurchaseOrderServiceTest.kt +++ b/pbc/pbc-orders-purchase/src/test/kotlin/org/vibeerp/pbc/orders/purchase/application/PurchaseOrderServiceTest.kt @@ -16,6 +16,7 @@ import io.mockk.verify import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.vibeerp.api.v1.core.Id +import org.vibeerp.api.v1.entity.HasExt import org.vibeerp.api.v1.event.EventBus import org.vibeerp.api.v1.event.orders.PurchaseOrderCancelledEvent import org.vibeerp.api.v1.event.orders.PurchaseOrderConfirmedEvent @@ -54,7 +55,8 @@ class PurchaseOrderServiceTest { inventoryApi = mockk() extValidator = mockk() eventBus = mockk() - every { extValidator.validate(any(), any()) } answers { secondArg() ?: emptyMap() } + every { extValidator.applyTo(any(), any()) } just runs + every { extValidator.parseExt(any()) } returns emptyMap() every { orders.existsByCode(any()) } returns false every { orders.save(any()) } answers { firstArg() } every { eventBus.publish(any()) } just runs diff --git a/pbc/pbc-orders-sales/src/main/kotlin/org/vibeerp/pbc/orders/sales/application/SalesOrderService.kt b/pbc/pbc-orders-sales/src/main/kotlin/org/vibeerp/pbc/orders/sales/application/SalesOrderService.kt index 0a55ba4..7787b07 100644 --- a/pbc/pbc-orders-sales/src/main/kotlin/org/vibeerp/pbc/orders/sales/application/SalesOrderService.kt +++ b/pbc/pbc-orders-sales/src/main/kotlin/org/vibeerp/pbc/orders/sales/application/SalesOrderService.kt @@ -1,7 +1,5 @@ package org.vibeerp.pbc.orders.sales.application -import com.fasterxml.jackson.databind.ObjectMapper -import com.fasterxml.jackson.module.kotlin.registerKotlinModule import org.springframework.stereotype.Service import org.springframework.transaction.annotation.Transactional import org.vibeerp.api.v1.event.EventBus @@ -78,8 +76,6 @@ class SalesOrderService( private val eventBus: EventBus, ) { - private val jsonMapper: ObjectMapper = ObjectMapper().registerKotlinModule() - @Transactional(readOnly = true) fun list(): List = orders.findAll() @@ -138,8 +134,6 @@ class SalesOrderService( acc + (line.quantity * line.unitPrice) } - val canonicalExt = extValidator.validate(ENTITY_NAME, command.ext) - val order = SalesOrder( code = command.code, partnerCode = command.partnerCode, @@ -147,9 +141,8 @@ class SalesOrderService( orderDate = command.orderDate, currencyCode = command.currencyCode, totalAmount = total, - ).also { - it.ext = jsonMapper.writeValueAsString(canonicalExt) - } + ) + extValidator.applyTo(order, command.ext) for (line in command.lines) { order.lines += SalesOrderLine( salesOrder = order, @@ -184,9 +177,8 @@ class SalesOrderService( command.orderDate?.let { order.orderDate = it } command.currencyCode?.let { order.currencyCode = it } - if (command.ext != null) { - order.ext = jsonMapper.writeValueAsString(extValidator.validate(ENTITY_NAME, command.ext)) - } + // applyTo() is null-safe — a null command.ext is a no-op. + extValidator.applyTo(order, command.ext) if (command.lines != null) { require(command.lines.isNotEmpty()) { @@ -347,17 +339,13 @@ class SalesOrderService( return order } - @Suppress("UNCHECKED_CAST") - fun parseExt(order: SalesOrder): Map = try { - if (order.ext.isBlank()) emptyMap() - else jsonMapper.readValue(order.ext, Map::class.java) as Map - } catch (ex: Throwable) { - emptyMap() - } - - companion object { - const val ENTITY_NAME: String = "SalesOrder" - } + /** + * Convenience passthrough for response mappers — delegates to + * [ExtJsonValidator.parseExt]. Returns an empty map on an empty + * or unparseable column so response rendering never 500s. + */ + fun parseExt(order: SalesOrder): Map = + extValidator.parseExt(order) } data class CreateSalesOrderCommand( diff --git a/pbc/pbc-orders-sales/src/main/kotlin/org/vibeerp/pbc/orders/sales/domain/SalesOrder.kt b/pbc/pbc-orders-sales/src/main/kotlin/org/vibeerp/pbc/orders/sales/domain/SalesOrder.kt index 005078c..fdf49c9 100644 --- a/pbc/pbc-orders-sales/src/main/kotlin/org/vibeerp/pbc/orders/sales/domain/SalesOrder.kt +++ b/pbc/pbc-orders-sales/src/main/kotlin/org/vibeerp/pbc/orders/sales/domain/SalesOrder.kt @@ -12,6 +12,7 @@ import jakarta.persistence.OrderBy import jakarta.persistence.Table import org.hibernate.annotations.JdbcTypeCode import org.hibernate.type.SqlTypes +import org.vibeerp.api.v1.entity.HasExt import org.vibeerp.platform.persistence.audit.AuditedJpaEntity import java.math.BigDecimal import java.time.LocalDate @@ -76,7 +77,7 @@ class SalesOrder( orderDate: LocalDate, currencyCode: String, totalAmount: BigDecimal = BigDecimal.ZERO, -) : AuditedJpaEntity() { +) : AuditedJpaEntity(), HasExt { @Column(name = "code", nullable = false, length = 64) var code: String = code @@ -99,7 +100,9 @@ class SalesOrder( @Column(name = "ext", nullable = false, columnDefinition = "jsonb") @JdbcTypeCode(SqlTypes.JSON) - var ext: String = "{}" + override var ext: String = "{}" + + override val extEntityName: String get() = ENTITY_NAME /** * The order's line items, eagerly loaded. Cascade ALL because @@ -119,6 +122,11 @@ class SalesOrder( override fun toString(): String = "SalesOrder(id=$id, code='$code', partner='$partnerCode', status=$status, total=$totalAmount $currencyCode)" + + companion object { + /** Key under which SalesOrder's custom fields are declared in `metadata__custom_field`. */ + const val ENTITY_NAME: String = "SalesOrder" + } } /** diff --git a/pbc/pbc-orders-sales/src/test/kotlin/org/vibeerp/pbc/orders/sales/application/SalesOrderServiceTest.kt b/pbc/pbc-orders-sales/src/test/kotlin/org/vibeerp/pbc/orders/sales/application/SalesOrderServiceTest.kt index a32f523..1219414 100644 --- a/pbc/pbc-orders-sales/src/test/kotlin/org/vibeerp/pbc/orders/sales/application/SalesOrderServiceTest.kt +++ b/pbc/pbc-orders-sales/src/test/kotlin/org/vibeerp/pbc/orders/sales/application/SalesOrderServiceTest.kt @@ -17,6 +17,7 @@ import io.mockk.verify import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.vibeerp.api.v1.core.Id +import org.vibeerp.api.v1.entity.HasExt import org.vibeerp.api.v1.event.EventBus import org.vibeerp.api.v1.event.orders.SalesOrderCancelledEvent import org.vibeerp.api.v1.event.orders.SalesOrderConfirmedEvent @@ -54,7 +55,10 @@ class SalesOrderServiceTest { inventoryApi = mockk() extValidator = mockk() eventBus = mockk() - every { extValidator.validate(any(), any()) } answers { secondArg() ?: emptyMap() } + // applyTo is a void with side effects; default to a no-op so + // tests that don't touch ext don't need to set expectations. + every { extValidator.applyTo(any(), any()) } just runs + every { extValidator.parseExt(any()) } returns emptyMap() every { orders.existsByCode(any()) } returns false every { orders.save(any()) } answers { firstArg() } // The bus is fire-and-forget from the service's perspective. diff --git a/pbc/pbc-partners/src/main/kotlin/org/vibeerp/pbc/partners/application/PartnerService.kt b/pbc/pbc-partners/src/main/kotlin/org/vibeerp/pbc/partners/application/PartnerService.kt index 114b3bd..0d2050f 100644 --- a/pbc/pbc-partners/src/main/kotlin/org/vibeerp/pbc/partners/application/PartnerService.kt +++ b/pbc/pbc-partners/src/main/kotlin/org/vibeerp/pbc/partners/application/PartnerService.kt @@ -1,7 +1,5 @@ package org.vibeerp.pbc.partners.application -import com.fasterxml.jackson.databind.ObjectMapper -import com.fasterxml.jackson.module.kotlin.registerKotlinModule import org.springframework.stereotype.Service import org.springframework.transaction.annotation.Transactional import org.vibeerp.pbc.partners.domain.Partner @@ -50,8 +48,6 @@ class PartnerService( private val extValidator: ExtJsonValidator, ) { - private val jsonMapper: ObjectMapper = ObjectMapper().registerKotlinModule() - @Transactional(readOnly = true) fun list(): List = partners.findAll() @@ -65,24 +61,22 @@ class PartnerService( require(!partners.existsByCode(command.code)) { "partner code '${command.code}' is already taken" } - // Validate the ext JSONB before save (P3.4 — Tier 1 customization). - // Throws IllegalArgumentException with all violations on failure; - // GlobalExceptionHandler maps it to 400 Bad Request. - val canonicalExt = extValidator.validate(ENTITY_NAME, command.ext) - return partners.save( - Partner( - code = command.code, - name = command.name, - type = command.type, - taxId = command.taxId, - website = command.website, - email = command.email, - phone = command.phone, - active = command.active, - ).also { - it.ext = jsonMapper.writeValueAsString(canonicalExt) - }, + // P3.4 — Tier 1 customization. applyTo() validates, canonicalises, + // and writes to partner.ext. Throws IllegalArgumentException with + // all violations joined if validation fails; GlobalExceptionHandler + // maps that to 400 Bad Request. + val partner = Partner( + code = command.code, + name = command.name, + type = command.type, + taxId = command.taxId, + website = command.website, + email = command.email, + phone = command.phone, + active = command.active, ) + extValidator.applyTo(partner, command.ext) + return partners.save(partner) } fun update(id: UUID, command: UpdatePartnerCommand): Partner { @@ -102,37 +96,18 @@ class PartnerService( // updates would force callers to merge with the existing JSON // before sending, which is the kind of foot-gun the framework // should not normalise. PATCH is for top-level columns only. - if (command.ext != null) { - val canonicalExt = extValidator.validate(ENTITY_NAME, command.ext) - partner.ext = jsonMapper.writeValueAsString(canonicalExt) - } + // applyTo() is null-safe — a null command.ext is a no-op. + extValidator.applyTo(partner, command.ext) return partner } /** - * Parse the entity's `ext` JSON string back into a Map for the - * REST response. Returns an empty map when the column is empty - * or unparseable — the latter shouldn't happen because every - * write goes through [extValidator], but cleaning up bad rows - * from old data isn't this method's responsibility. + * Convenience passthrough for response mappers — delegates to + * [ExtJsonValidator.parseExt]. Returns an empty map on an empty + * or unparseable column so response rendering never 500s. */ - @Suppress("UNCHECKED_CAST") - fun parseExt(partner: Partner): Map = try { - if (partner.ext.isBlank()) emptyMap() - else jsonMapper.readValue(partner.ext, Map::class.java) as Map - } catch (ex: Throwable) { - emptyMap() - } - - companion object { - /** - * The entity name the partner aggregate is registered under in - * `metadata__entity` (and therefore the key the [ExtJsonValidator] - * looks up). Kept as a constant so the controller and tests - * can reference the same string without hard-coding it twice. - */ - const val ENTITY_NAME: String = "Partner" - } + fun parseExt(partner: Partner): Map = + extValidator.parseExt(partner) /** * Deactivate the partner and all its contacts. Addresses are left diff --git a/pbc/pbc-partners/src/main/kotlin/org/vibeerp/pbc/partners/domain/Partner.kt b/pbc/pbc-partners/src/main/kotlin/org/vibeerp/pbc/partners/domain/Partner.kt index 288786a..5d5c2d2 100644 --- a/pbc/pbc-partners/src/main/kotlin/org/vibeerp/pbc/partners/domain/Partner.kt +++ b/pbc/pbc-partners/src/main/kotlin/org/vibeerp/pbc/partners/domain/Partner.kt @@ -7,6 +7,7 @@ import jakarta.persistence.Enumerated import jakarta.persistence.Table import org.hibernate.annotations.JdbcTypeCode import org.hibernate.type.SqlTypes +import org.vibeerp.api.v1.entity.HasExt import org.vibeerp.platform.persistence.audit.AuditedJpaEntity /** @@ -58,7 +59,7 @@ class Partner( email: String? = null, phone: String? = null, active: Boolean = true, -) : AuditedJpaEntity() { +) : AuditedJpaEntity(), HasExt { @Column(name = "code", nullable = false, length = 64) var code: String = code @@ -87,10 +88,17 @@ class Partner( @Column(name = "ext", nullable = false, columnDefinition = "jsonb") @JdbcTypeCode(SqlTypes.JSON) - var ext: String = "{}" + override var ext: String = "{}" + + override val extEntityName: String get() = ENTITY_NAME override fun toString(): String = "Partner(id=$id, code='$code', type=$type, active=$active)" + + companion object { + /** Key under which Partner's custom fields are declared in `metadata__custom_field`. */ + const val ENTITY_NAME: String = "Partner" + } } /** diff --git a/pbc/pbc-partners/src/test/kotlin/org/vibeerp/pbc/partners/application/PartnerServiceTest.kt b/pbc/pbc-partners/src/test/kotlin/org/vibeerp/pbc/partners/application/PartnerServiceTest.kt index 646f1a0..a8b8535 100644 --- a/pbc/pbc-partners/src/test/kotlin/org/vibeerp/pbc/partners/application/PartnerServiceTest.kt +++ b/pbc/pbc-partners/src/test/kotlin/org/vibeerp/pbc/partners/application/PartnerServiceTest.kt @@ -6,12 +6,15 @@ import assertk.assertions.hasMessage import assertk.assertions.isEqualTo import assertk.assertions.isFalse import assertk.assertions.isInstanceOf +import io.mockk.Runs import io.mockk.every +import io.mockk.just import io.mockk.mockk import io.mockk.slot import io.mockk.verify import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import org.vibeerp.api.v1.entity.HasExt import org.vibeerp.pbc.partners.domain.Contact import org.vibeerp.pbc.partners.domain.Partner import org.vibeerp.pbc.partners.domain.PartnerType @@ -36,9 +39,10 @@ class PartnerServiceTest { addresses = mockk() contacts = mockk() extValidator = mockk() - // Default: validator returns the input map unchanged. Tests - // that exercise ext directly override this expectation. - every { extValidator.validate(any(), any()) } answers { secondArg() ?: emptyMap() } + // Default: validator treats applyTo as a no-op. Tests that + // exercise ext directly override this expectation. + every { extValidator.applyTo(any(), any()) } just Runs + every { extValidator.parseExt(any()) } returns emptyMap() service = PartnerService(partners, addresses, contacts, extValidator) } diff --git a/platform/platform-metadata/src/main/kotlin/org/vibeerp/platform/metadata/customfield/ExtJsonValidator.kt b/platform/platform-metadata/src/main/kotlin/org/vibeerp/platform/metadata/customfield/ExtJsonValidator.kt index cd0634d..70624e9 100644 --- a/platform/platform-metadata/src/main/kotlin/org/vibeerp/platform/metadata/customfield/ExtJsonValidator.kt +++ b/platform/platform-metadata/src/main/kotlin/org/vibeerp/platform/metadata/customfield/ExtJsonValidator.kt @@ -1,8 +1,10 @@ package org.vibeerp.platform.metadata.customfield +import com.fasterxml.jackson.databind.ObjectMapper import org.springframework.stereotype.Component import org.vibeerp.api.v1.entity.CustomField import org.vibeerp.api.v1.entity.FieldType +import org.vibeerp.api.v1.entity.HasExt import java.math.BigDecimal import java.time.LocalDate import java.time.OffsetDateTime @@ -53,9 +55,53 @@ import java.util.UUID @Component class ExtJsonValidator( private val registry: CustomFieldRegistry, + private val jsonMapper: ObjectMapper, ) { /** + * Validate [ext], serialize the canonical form, and write it to + * [entity]'s ext column. Null [ext] is a no-op (the column keeps + * its prior value, which means PATCH-style "don't touch ext" + * works out of the box). + * + * This is the single entry point every core PBC service and + * every plug-in uses to stage an ext update. It replaces the + * four-line sequence (validate / writeValueAsString / assign to + * column) that used to live in every service — removing a lot + * of boilerplate and giving the framework one place to change + * the ext-save contract later (e.g. PII redaction, audit + * tagging, eventing on field-level changes). + * + * Throws [IllegalArgumentException] with all violations joined + * by `; ` if validation fails. The framework's + * `GlobalExceptionHandler` maps that to 400 Bad Request. + */ + fun applyTo(entity: HasExt, ext: Map?) { + if (ext == null) return + val canonical = validate(entity.extEntityName, ext) + entity.ext = jsonMapper.writeValueAsString(canonical) + } + + /** + * Parse an entity's serialized `ext` column back into a + * `Map` for response DTOs. Returns an empty map + * for any parse failure — response mappers should NOT fail a + * successful read because of a bad serialized ext value; if the + * column is corrupt, the fix is a save, not a 500 on read. + * + * This replaces the `parseExt(...)` helper that was duplicated + * verbatim in every core PBC service. + */ + @Suppress("UNCHECKED_CAST") + fun parseExt(entity: HasExt): Map = + try { + if (entity.ext.isBlank()) emptyMap() + else jsonMapper.readValue(entity.ext, Map::class.java) as Map + } catch (ex: Throwable) { + emptyMap() + } + + /** * Validate (and canonicalise) the [ext] map for [entityName]. * * @return a fresh map containing only the declared fields, with diff --git a/platform/platform-metadata/src/test/kotlin/org/vibeerp/platform/metadata/customfield/ExtJsonValidatorTest.kt b/platform/platform-metadata/src/test/kotlin/org/vibeerp/platform/metadata/customfield/ExtJsonValidatorTest.kt index 61b4884..d700d4b 100644 --- a/platform/platform-metadata/src/test/kotlin/org/vibeerp/platform/metadata/customfield/ExtJsonValidatorTest.kt +++ b/platform/platform-metadata/src/test/kotlin/org/vibeerp/platform/metadata/customfield/ExtJsonValidatorTest.kt @@ -9,12 +9,15 @@ import assertk.assertions.isEmpty import assertk.assertions.isEqualTo import assertk.assertions.isInstanceOf import assertk.assertions.messageContains +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.module.kotlin.registerKotlinModule import io.mockk.every import io.mockk.mockk import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.vibeerp.api.v1.entity.CustomField import org.vibeerp.api.v1.entity.FieldType +import org.vibeerp.api.v1.entity.HasExt class ExtJsonValidatorTest { @@ -24,7 +27,7 @@ class ExtJsonValidatorTest { @BeforeEach fun setUp() { registry = mockk() - validator = ExtJsonValidator(registry) + validator = ExtJsonValidator(registry, ObjectMapper().registerKotlinModule()) } private fun stub(vararg fields: CustomField) { @@ -170,6 +173,72 @@ class ExtJsonValidatorTest { assertThat(canonical.keys).hasSize(0) } + // ─── applyTo + parseExt (HasExt convenience helpers) ───────────── + + /** + * Minimal [HasExt] fixture. Services will mix this marker into + * their JPA entities; the test just needs something concrete. + */ + private class FakeEntity( + override val extEntityName: String = ENTITY, + override var ext: String = "{}", + ) : HasExt + + @Test + fun `applyTo with null ext is a no-op`() { + val fake = FakeEntity() + validator.applyTo(fake, null) + assertThat(fake.ext).isEqualTo("{}") + } + + @Test + fun `applyTo validates and writes canonical JSON to entity ext`() { + stub( + CustomField("label", ENTITY, FieldType.String(maxLength = 16)), + CustomField("count", ENTITY, FieldType.Integer), + ) + val fake = FakeEntity() + + validator.applyTo(fake, mapOf("label" to "ok", "count" to 3)) + + // Canonical form — label first (declared order), count as Long. + assertThat(fake.ext).isEqualTo("""{"label":"ok","count":3}""") + } + + @Test + fun `applyTo surfaces validation failures as IllegalArgumentException`() { + stub(CustomField("known", ENTITY, FieldType.Integer)) + val fake = FakeEntity() + + assertFailure { validator.applyTo(fake, mapOf("rogue" to "x")) } + .isInstanceOf(IllegalArgumentException::class) + .messageContains("undeclared") + // Ext was NOT touched on failure. + assertThat(fake.ext).isEqualTo("{}") + } + + @Test + fun `parseExt returns empty map for blank ext column`() { + val fake = FakeEntity(ext = "") + assertThat(validator.parseExt(fake)).isEmpty() + } + + @Test + fun `parseExt round-trips a previously-canonicalised ext value`() { + val fake = FakeEntity(ext = """{"label":"round-trip","count":42}""") + val parsed = validator.parseExt(fake) + assertThat(parsed.keys).contains("label") + assertThat(parsed.keys).contains("count") + assertThat(parsed["label"]).isEqualTo("round-trip") + } + + @Test + fun `parseExt swallows malformed JSON and returns empty map`() { + val fake = FakeEntity(ext = "this is not json") + // Response mappers should NOT 500 on a corrupt row. + assertThat(validator.parseExt(fake)).isEmpty() + } + companion object { const val ENTITY: String = "TestEntity" } -- libgit2 0.22.2