Add custom data mapping: per-column MIGRATE_AS_IS/MASK wizard with schema browsing#72
Conversation
…hema browsing Agent-Logs-Url: https://github.com/MaximumTrainer/OpenDataMask/sessions/78fb0a16-9359-4a9d-a8ae-075229759b79 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
…appingView, tests, docs Agent-Logs-Url: https://github.com/MaximumTrainer/OpenDataMask/sessions/78fb0a16-9359-4a9d-a8ae-075229759b79 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
Agent-Logs-Url: https://github.com/MaximumTrainer/OpenDataMask/sessions/78fb0a16-9359-4a9d-a8ae-075229759b79 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new “Custom Data Mapping” capability to configure per-column actions (MIGRATE_AS_IS vs MASK with FAKE/HASH/NULL) and exposes it through a 3-step frontend wizard backed by new REST endpoints, including live connection schema browsing.
Changes:
- Backend: introduce
CustomDataMappingpersistence model + CRUD/bulk APIs and abrowseConnectionSchemaendpoint on connections. - Frontend: add mapping types + API client + new
DataMappingViewwizard route and navigation entry points. - Tests/docs: add unit tests for the new services/types and document the new endpoints and bulk request format.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/views/tests/DataMappingView.test.ts | Adds vitest coverage for new mapping/schema types and basic structure expectations. |
| frontend/src/views/WorkspaceDetailView.vue | Adds “Data Mappings” tab and quick-action navigation entry. |
| frontend/src/views/DataMappingView.vue | Implements the 3-step mapping wizard UI, bulk save flow, and saved-mappings summary. |
| frontend/src/types/index.ts | Introduces MappingAction, MaskingStrategy, mapping DTO types, and schema browsing response types. |
| frontend/src/router/index.ts | Registers the new /workspaces/:id/mappings route. |
| frontend/src/api/mapping.ts | Adds API wrapper functions for mappings CRUD/bulk and schema browsing. |
| docs/user-guide.md | Documents the Custom Data Mapping feature and REST API including bulk example. |
| backend/src/test/kotlin/com/opendatamask/application/service/DataConnectionServiceTest.kt | Extends tests to cover schema browsing behavior. |
| backend/src/test/kotlin/com/opendatamask/application/service/CustomDataMappingServiceTest.kt | Adds unit tests covering mapping CRUD and bulk replace behavior. |
| backend/src/main/kotlin/com/opendatamask/domain/port/output/CustomDataMappingPort.kt | Defines the persistence port for mappings. |
| backend/src/main/kotlin/com/opendatamask/domain/port/input/dto/CustomDataMappingDto.kt | Adds request/response DTOs for mapping operations and connection schema responses. |
| backend/src/main/kotlin/com/opendatamask/domain/port/input/DataConnectionUseCase.kt | Extends the connection use case with schema browsing. |
| backend/src/main/kotlin/com/opendatamask/domain/port/input/CustomDataMappingUseCase.kt | Defines the mapping use case interface (CRUD + bulk). |
| backend/src/main/kotlin/com/opendatamask/domain/model/CustomDataMapping.kt | Adds the JPA entity + enums for mapping action and masking strategy. |
| backend/src/main/kotlin/com/opendatamask/application/service/DataConnectionService.kt | Implements schema browsing by calling connector table/column introspection. |
| backend/src/main/kotlin/com/opendatamask/application/service/CustomDataMappingService.kt | Implements mapping CRUD and bulk table-level replace logic. |
| backend/src/main/kotlin/com/opendatamask/adapter/output/persistence/CustomDataMappingRepository.kt | Adds Spring Data JPA repository implementing the new port. |
| backend/src/main/kotlin/com/opendatamask/adapter/input/rest/DataConnectionController.kt | Exposes GET /connections/{id}/schema. |
| backend/src/main/kotlin/com/opendatamask/adapter/input/rest/CustomDataMappingController.kt | Exposes mappings CRUD + bulk endpoints under /api/workspaces/{id}/mappings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setTimeout(() => { | ||
| saveSuccess.value = false | ||
| currentStep.value = 2 | ||
| }, 1500) |
There was a problem hiding this comment.
saveMappings() uses setTimeout to update step state after saving but doesn’t clear the timer on unmount. Navigating away quickly can trigger state updates after unmount. Consider storing the timer id and clearing it in onUnmounted() (or avoiding the timer).
| :class="{ active: columnMappings[col.name]?.action === 'MIGRATE_AS_IS' }" | ||
| @click="setAction(col.name, MappingAction.MIGRATE_AS_IS)" |
There was a problem hiding this comment.
The template compares enum-typed fields against string literals (e.g. action === 'MIGRATE_AS_IS'). Prefer MappingAction.MIGRATE_AS_IS / MappingAction.MASK in comparisons to avoid duplicating enum values and to make refactors safer.
| @Mock private lateinit var customDataMappingRepository: CustomDataMappingRepository | ||
|
|
||
| @InjectMocks | ||
| private lateinit var service: CustomDataMappingService |
There was a problem hiding this comment.
This unit test mocks CustomDataMappingRepository (persistence adapter) even though the service depends only on CustomDataMappingPort. Mocking the port interface directly will keep the test focused on service behavior and reduce coupling to the Spring Data adapter.
| action = entry.action, | ||
| maskingStrategy = if (entry.action == MappingAction.MASK) entry.maskingStrategy else null, | ||
| fakeGeneratorType = if (entry.action == MappingAction.MASK) entry.fakeGeneratorType else null | ||
| ) |
There was a problem hiding this comment.
saveBulkMappings() will persist entries with action = MASK even when maskingStrategy is null (or FAKE without a generator), which allows invalid mappings to be stored. Add per-entry validation (DTO @AssertTrue / explicit checks) before saving.
| @Table(name = "custom_data_mappings") | ||
| class CustomDataMapping( | ||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| val id: Long = 0, | ||
|
|
||
| @Column(nullable = false) | ||
| var workspaceId: Long, | ||
|
|
||
| @Column(nullable = false) | ||
| var connectionId: Long, | ||
|
|
||
| @Column(nullable = false) | ||
| var tableName: String, | ||
|
|
||
| @Column(nullable = false) |
There was a problem hiding this comment.
CustomDataMapping has no uniqueness constraint for (workspaceId, connectionId, tableName, columnName), so creating/updating via the single-item endpoints can introduce duplicate rows for the same column. Adding a DB-level unique constraint (and handling conflicts with a 409/400) would prevent hard-to-debug duplication in the wizard and downstream masking logic.
| @Table(name = "custom_data_mappings") | |
| class CustomDataMapping( | |
| @Id | |
| @GeneratedValue(strategy = GenerationType.IDENTITY) | |
| val id: Long = 0, | |
| @Column(nullable = false) | |
| var workspaceId: Long, | |
| @Column(nullable = false) | |
| var connectionId: Long, | |
| @Column(nullable = false) | |
| var tableName: String, | |
| @Column(nullable = false) | |
| @Table( | |
| name = "custom_data_mappings", | |
| uniqueConstraints = [ | |
| UniqueConstraint( | |
| name = "uk_custom_data_mappings_workspace_connection_table_column", | |
| columnNames = ["workspace_id", "connection_id", "table_name", "column_name"] | |
| ) | |
| ] | |
| ) | |
| class CustomDataMapping( | |
| @Id | |
| @GeneratedValue(strategy = GenerationType.IDENTITY) | |
| val id: Long = 0, | |
| @Column(name = "workspace_id", nullable = false) | |
| var workspaceId: Long, | |
| @Column(name = "connection_id", nullable = false) | |
| var connectionId: Long, | |
| @Column(name = "table_name", nullable = false) | |
| var tableName: String, | |
| @Column(name = "column_name", nullable = false) |
| val tables = connector.listTables().map { tableName -> | ||
| val columns = connector.listColumns(tableName).map { col -> | ||
| ConnectionSchemaResponse.ColumnSchemaInfo( | ||
| name = col.name, | ||
| type = col.type, |
There was a problem hiding this comment.
browseConnectionSchema() maps over all tables and calls connector.listColumns(tableName) for each one. For JDBC connectors, listColumns() opens a new DB connection per call, so schemas with many tables can cause a connection storm. Consider adding a connector API that returns tables+columns in one call / reusing a single connection, or limiting the number of tables returned.
| @@ -108,6 +109,33 @@ class DataConnectionService( | |||
| } | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
browseConnectionSchema() is missing a @Transactional(readOnly = true) annotation even though it reads from the repository (and other read methods in this service are marked read-only). Adding it keeps behavior consistent and avoids unnecessary transaction semantics.
| @Transactional(readOnly = true) |
|
|
||
| @field:NotNull(message = "Column mappings are required") | ||
| val columnMappings: List<ColumnMappingEntry> | ||
| ) { |
There was a problem hiding this comment.
BulkCustomDataMappingRequest.columnMappings is not annotated with @Valid, so the @NotBlank/@NotNull constraints on ColumnMappingEntry will not be applied to the list elements. Add @field:Valid (and optionally @field:NotEmpty if empty payloads should be rejected) to ensure invalid entries are caught at the API boundary.
| columnName = request.columnName, | ||
| action = request.action, | ||
| maskingStrategy = if (request.action == MappingAction.MASK) request.maskingStrategy else null, | ||
| fakeGeneratorType = if (request.action == MappingAction.MASK) request.fakeGeneratorType else null | ||
| ) |
There was a problem hiding this comment.
The service persists action = MASK even when maskingStrategy/fakeGeneratorType are missing in the request, which allows invalid mappings to be stored (e.g., MASK with null strategy). Add cross-field validation (DTO @AssertTrue or explicit checks) to enforce valid combinations before saving.
| val result = if (connectionId != null && tableName != null) { | ||
| customDataMappingService.listMappingsForTable(workspaceId, connectionId, tableName) | ||
| } else { | ||
| customDataMappingService.listMappings(workspaceId) | ||
| } |
There was a problem hiding this comment.
GET /mappings falls back to listMappings(workspaceId) unless both connectionId and tableName are provided, so supplying only one filter is silently ignored. Consider returning 400 for partial filters, or supporting independent filtering explicitly.
|
@copilot apply changes based on the comments in this thread |
, enum comparisons, timer cleanup, bulk delete Agent-Logs-Url: https://github.com/MaximumTrainer/OpenDataMask/sessions/2758ddbd-e5a0-47b1-a915-5fcdf275497d Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
…tomDataMappingPort Agent-Logs-Url: https://github.com/MaximumTrainer/OpenDataMask/sessions/2758ddbd-e5a0-47b1-a915-5fcdf275497d Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
Applied all review feedback across commits Backend:
Frontend:
|
CustomDataMappingdomain entity withMappingAction(MIGRATE_AS_IS, MASK) andMaskingStrategy(FAKE, HASH, NULL) enumsCustomDataMappingPortoutput port andCustomDataMappingUseCaseinput port with DTOsCustomDataMappingRepositoryJPA repositoryCustomDataMappingServiceapplication serviceDataConnectionController(GET /connections/{id}/schema)CustomDataMappingControllerREST controllerCustomDataMappingServiceTestunit tests (RED → GREEN, 16 tests total)browseConnectionSchematests inDataConnectionServiceTestfrontend/src/types/index.tsmapping.tsAPI module and schema browsing endpointDataMappingView.vuewizard (select connection → select table → configure per-column actions)/workspaces/:id/mappingsand navigation tab/link in workspace detaildocs/user-guide.mdwith Custom Data Mapping section@Transactional(readOnly = true)tobrowseConnectionSchema()@field:ValidtoBulkCustomDataMappingRequest.columnMappingsCustomDataMappingPortinterface (renamed from repository), not the adaptersetTimeoutinonUnmounted()to prevent stale state updatesMappingAction.*/MaskingStrategy.*enum constants instead of string literalsGeneratorTypeand remove FQN invalidateMaskingCombination; rename mock variable tocustomDataMappingPort