From ce79ce11a1f46f69bc78a05b93bdcff92c1dec61 Mon Sep 17 00:00:00 2001 From: Devesh-Skyflow Date: Mon, 20 Jul 2026 12:31:30 +0530 Subject: [PATCH] SK-2955 align upsert/upsertType with Flow-DB changes (docs, tests, error-message cleanup) (#346) Flow-DB now defaults record-level upsert to UPDATE (was REPLACE) and owns upsert-placement validation server-side. The SDK already serializes upsertType correctly and omits updateType when unset, so this is docs, tests, and validation hygiene only. - README: document upsertType is optional, default is now UPDATE, and UPDATE vs REPLACE semantics. - Samples: drop redundant explicit upsertType(UPDATE) (now the default) in BulkMultiTableInsert{Sync,Async}; keep REPLACE samples as-is. - Validations: defer upsert-placement checks to the backend (authoritative messages); SDK only guards empty upsert columns. - Tests: cover default-UPDATE omission at request/record level, both-level serialization, and updated placement tests to reflect deferral. Co-authored-by: Claude Opus 4.8 (1M context) --- README.md | 7 +- samples/pom.xml | 2 +- .../vault/BulkMultiTableInsertAsync.java | 4 +- .../vault/BulkMultiTableInsertSync.java | 4 +- .../utils/validations/Validations.java | 29 ++------ .../java/com/skyflow/VaultClientTests.java | 69 +++++++++++++++++++ .../utils/validations/ValidationsTests.java | 12 ++-- .../com/skyflow/vault/data/InsertTests.java | 31 ++------- 8 files changed, 99 insertions(+), 59 deletions(-) diff --git a/README.md b/README.md index 938c7b89..fc4354d2 100644 --- a/README.md +++ b/README.md @@ -260,8 +260,11 @@ public class InsertSchema { **Note**: - The table name can be specified either at the request level `InsertRequest` or at the record level `InsertRecord`, but not both. - If table name is not specified at the request level `InsertRequest`, then it must be specified in all record objects. -- If table name is specified at the request level `InsertRequest`, then upsert must also be specified at the request level. -- If table name is specified at the record level `InsertRecord`, then upsert must also be specified at the record level `InsertRecord`. +- Upsert must be specified in the same place as the table name: if table name is specified at the request level `InsertRequest`, specify upsert at the request level; if table name is specified at the record level `InsertRecord`, specify upsert at the record level `InsertRecord`. +- `upsertType` is optional and can be set alongside `upsert` at either the request level or the record level (matching the table/upsert placement): + - `UpsertType.UPDATE` — updates only the columns provided in the request on the matched row; other existing columns are retained. + - `UpsertType.REPLACE` — replaces the matched row with the provided values; columns not provided are cleared. + - If `upsertType` is not specified, the vault applies the default of `UPDATE`. ### An [example](https://github.com/skyflowapi/skyflow-java/blob/v3/samples/src/main/java/com/example/vault/BulkInsertSync.java) of a sync bulkInsert call diff --git a/samples/pom.xml b/samples/pom.xml index ad4a1427..4675a641 100644 --- a/samples/pom.xml +++ b/samples/pom.xml @@ -18,7 +18,7 @@ com.skyflow skyflow-java - 3.0.0-beta.6 + 3.0.0-beta.11 diff --git a/samples/src/main/java/com/example/vault/BulkMultiTableInsertAsync.java b/samples/src/main/java/com/example/vault/BulkMultiTableInsertAsync.java index 1606cf70..99c5e99a 100644 --- a/samples/src/main/java/com/example/vault/BulkMultiTableInsertAsync.java +++ b/samples/src/main/java/com/example/vault/BulkMultiTableInsertAsync.java @@ -5,7 +5,6 @@ import com.skyflow.config.VaultConfig; import com.skyflow.enums.Env; import com.skyflow.enums.LogLevel; -import com.skyflow.enums.UpsertType; import com.skyflow.vault.data.InsertRecord; import com.skyflow.vault.data.InsertRequest; import com.skyflow.vault.data.InsertResponse; @@ -59,7 +58,8 @@ public static void main(String[] args) { .data(recordData1) .table("") .upsert(upsertColumns) - .upsertType(UpsertType.UPDATE) + // upsertType is optional; when omitted the vault defaults to UpsertType.UPDATE. + // Set .upsertType(UpsertType.REPLACE) to replace the matched row instead. .build(); // Step 5: Prepare second record for insertion diff --git a/samples/src/main/java/com/example/vault/BulkMultiTableInsertSync.java b/samples/src/main/java/com/example/vault/BulkMultiTableInsertSync.java index 95bf50dc..577cd142 100644 --- a/samples/src/main/java/com/example/vault/BulkMultiTableInsertSync.java +++ b/samples/src/main/java/com/example/vault/BulkMultiTableInsertSync.java @@ -5,7 +5,6 @@ import com.skyflow.config.VaultConfig; import com.skyflow.enums.Env; import com.skyflow.enums.LogLevel; -import com.skyflow.enums.UpsertType; import com.skyflow.errors.SkyflowException; import com.skyflow.vault.data.InsertRecord; import com.skyflow.vault.data.InsertRequest; @@ -58,7 +57,8 @@ public static void main(String[] args) { .data(recordData1) .table("") .upsert(upsertColumns) - .upsertType(UpsertType.UPDATE) + // upsertType is optional; when omitted the vault defaults to UpsertType.UPDATE. + // Set .upsertType(UpsertType.REPLACE) to replace the matched row instead. .build(); // Step 5: Prepare second record for insertion diff --git a/v3/src/main/java/com/skyflow/utils/validations/Validations.java b/v3/src/main/java/com/skyflow/utils/validations/Validations.java index 839c54ce..3e30f7a9 100644 --- a/v3/src/main/java/com/skyflow/utils/validations/Validations.java +++ b/v3/src/main/java/com/skyflow/utils/validations/Validations.java @@ -73,30 +73,15 @@ public static void validateInsertRequest(InsertRequest insertRequest) throws Sky } } } - // upsert check 1 - if (insertRequest.getTable() != null && !table.trim().isEmpty()){ // if table name specified at both place - for (InsertRecord record : records) { - if (record.getUpsert() != null && record.getUpsert().isEmpty()) { - LogUtil.printErrorLog(Utils.parameterizedString( - ErrorLogs.EMPTY_UPSERT_VALUES.getLog(), InterfaceName.INSERT.getName() - )); - throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.EmptyUpsertValues.getMessage()); - } - if (record.getUpsert() != null && !record.getUpsert().isEmpty()){ - LogUtil.printErrorLog(Utils.parameterizedString( - ErrorLogs.UPSERT_TABLE_REQUEST_AT_RECORD_LEVEL.getLog(), InterfaceName.INSERT.getName() - )); - throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.UpsertTableRequestAtRecordLevel.getMessage()); - } - } - } - // upsert check 2 - if (insertRequest.getTable() == null || table.trim().isEmpty()){ - if (insertRequest.getUpsert() != null && !insertRequest.getUpsert().isEmpty()){ + // Upsert placement (request level vs record level) is validated by the backend, + // which returns the authoritative error message. The SDK only guards against + // empty upsert column lists. + for (InsertRecord record : records) { + if (record.getUpsert() != null && record.getUpsert().isEmpty()) { LogUtil.printErrorLog(Utils.parameterizedString( - ErrorLogs.UPSERT_TABLE_REQUEST_AT_REQUEST_LEVEL.getLog(), InterfaceName.INSERT.getName() + ErrorLogs.EMPTY_UPSERT_VALUES.getLog(), InterfaceName.INSERT.getName() )); - throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.UpsertTableRequestAtRequestLevel.getMessage()); + throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.EmptyUpsertValues.getMessage()); } } diff --git a/v3/src/test/java/com/skyflow/VaultClientTests.java b/v3/src/test/java/com/skyflow/VaultClientTests.java index b52272dc..e9b5a65f 100644 --- a/v3/src/test/java/com/skyflow/VaultClientTests.java +++ b/v3/src/test/java/com/skyflow/VaultClientTests.java @@ -279,6 +279,75 @@ public void testMixedTableAndUpsertLevels() { Assert.assertEquals("col4", result.getUpsert().get().getUniqueColumns().get().get(0)); } + @Test + public void testUpsertAtRequestLevelWithoutUpsertTypeOmitsUpdateType() { + // When upsertType is not specified, the SDK must not send updateType so the + // backend applies its default (UPDATE). + Map data = new HashMap<>(); + data.put("key", "value"); + InsertRecord record = InsertRecord.builder().data(data).build(); + ArrayList records = new ArrayList<>(); + records.add(record); + + com.skyflow.vault.data.InsertRequest request = + com.skyflow.vault.data.InsertRequest.builder() + .records(records) + .upsert(Arrays.asList("col1")) + .build(); + + V1InsertRequest result = vaultClient.getBulkInsertRequestBody(request, vaultConfig); + Assert.assertNotNull(result.getUpsert()); + Assert.assertEquals("col1", result.getUpsert().get().getUniqueColumns().get().get(0)); + Assert.assertFalse(result.getUpsert().get().getUpdateType().isPresent()); + } + + @Test + public void testUpsertAtRecordLevelWithoutUpsertTypeOmitsUpdateType() { + // When upsertType is not specified at the record level, the SDK must not send + // updateType so the backend applies its default (UPDATE). + Map data = new HashMap<>(); + data.put("key", "value"); + InsertRecord record = InsertRecord.builder().data(data).upsert(Arrays.asList("col2")).build(); + ArrayList records = new ArrayList<>(); + records.add(record); + + com.skyflow.vault.data.InsertRequest request = + com.skyflow.vault.data.InsertRequest.builder() + .records(records) + .build(); + + V1InsertRequest result = vaultClient.getBulkInsertRequestBody(request, vaultConfig); + Assert.assertNotNull(result.getRecords().get().get(0).getUpsert()); + Assert.assertEquals("col2", result.getRecords().get().get(0).getUpsert().get().getUniqueColumns().get().get(0)); + Assert.assertFalse(result.getRecords().get().get(0).getUpsert().get().getUpdateType().isPresent()); + } + + @Test + public void testUpsertTypeAtBothRequestAndRecordLevelSerializesBoth() { + // The SDK serializes updateType wherever it is set; the backend rejects the + // both-levels combination. This asserts the SDK does not silently drop either. + Map data = new HashMap<>(); + data.put("key", "value"); + InsertRecord record = InsertRecord.builder() + .data(data) + .upsert(Arrays.asList("col2")) + .upsertType(UpsertType.UPDATE) + .build(); + ArrayList records = new ArrayList<>(); + records.add(record); + + com.skyflow.vault.data.InsertRequest request = + com.skyflow.vault.data.InsertRequest.builder() + .records(records) + .upsert(Arrays.asList("col1")) + .upsertType(UpsertType.REPLACE) + .build(); + + V1InsertRequest result = vaultClient.getBulkInsertRequestBody(request, vaultConfig); + Assert.assertEquals("REPLACE", result.getUpsert().get().getUpdateType().get().name()); + Assert.assertEquals("UPDATE", result.getRecords().get().get(0).getUpsert().get().getUpdateType().get().name()); + } + @Test public void testSetBearerTokenWhenTokenIsNull() throws Exception { Credentials credentials = new Credentials(); diff --git a/v3/src/test/java/com/skyflow/utils/validations/ValidationsTests.java b/v3/src/test/java/com/skyflow/utils/validations/ValidationsTests.java index 407ea055..cd4d7485 100644 --- a/v3/src/test/java/com/skyflow/utils/validations/ValidationsTests.java +++ b/v3/src/test/java/com/skyflow/utils/validations/ValidationsTests.java @@ -75,7 +75,8 @@ public void validateInsertRequest_tableMissing_throws() { } @Test - public void validateInsertRequest_upsertAtRecordLevel_throws() { + public void validateInsertRequest_upsertAtRecordLevelWithTableAtRequestLevel_deferredToBackend() throws SkyflowException { + // Upsert placement is no longer validated client-side; the backend owns the rule. ArrayList records = new ArrayList<>(); records.add(InsertRecord.builder() .table(null) @@ -85,12 +86,12 @@ public void validateInsertRequest_upsertAtRecordLevel_throws() { .table("requestTable") .records(records) .build(); - assertSkyflowException(() -> Validations.validateInsertRequest(request), - ErrorMessage.UpsertTableRequestAtRecordLevel.getMessage()); + Validations.validateInsertRequest(request); } @Test - public void validateInsertRequest_upsertAtRequestLevelWithoutTable_throws() { + public void validateInsertRequest_upsertAtRequestLevelWithTableAtRecordLevel_deferredToBackend() throws SkyflowException { + // Upsert placement is no longer validated client-side; the backend owns the rule. ArrayList records = new ArrayList<>(); records.add(InsertRecord.builder().table("recordTable").build()); InsertRequest request = InsertRequest.builder() @@ -98,8 +99,7 @@ public void validateInsertRequest_upsertAtRequestLevelWithoutTable_throws() { .upsert(Collections.singletonList("key")) .records(records) .build(); - assertSkyflowException(() -> Validations.validateInsertRequest(request), - ErrorMessage.UpsertTableRequestAtRequestLevel.getMessage()); + Validations.validateInsertRequest(request); } @Test diff --git a/v3/src/test/java/com/skyflow/vault/data/InsertTests.java b/v3/src/test/java/com/skyflow/vault/data/InsertTests.java index 7020fb19..2b12b73f 100644 --- a/v3/src/test/java/com/skyflow/vault/data/InsertTests.java +++ b/v3/src/test/java/com/skyflow/vault/data/InsertTests.java @@ -326,6 +326,9 @@ public void testTableSpecifiedAtBothRequestAndRecordLevel() { } } + // Upsert placement (request vs record level) is validated by the backend, not the SDK. + // The SDK must accept these combinations and let the backend return the authoritative error. + @Test public void testUpsertSpecifiedAtBothRequestAndRecordLevel() { InsertRecord record = InsertRecord.builder().data(valueMap).upsert(upsert).build(); @@ -333,13 +336,8 @@ public void testUpsertSpecifiedAtBothRequestAndRecordLevel() { InsertRequest request = InsertRequest.builder().table(table).records(values).upsert(upsert).build(); try { Validations.validateInsertRequest(request); - Assert.fail(EXCEPTION_NOT_THROWN); } catch (SkyflowException e) { - Assert.assertEquals(ErrorCode.INVALID_INPUT.getCode(), e.getHttpCode()); - Assert.assertEquals( - Utils.parameterizedString(ErrorMessage.UpsertTableRequestAtRecordLevel.getMessage(), Constants.SDK_PREFIX), - e.getMessage() - ); + Assert.fail(INVALID_EXCEPTION_THROWN); } } @@ -350,13 +348,8 @@ public void testUpsertAtRecordLevelWithTableAtRequestLevel() { InsertRequest request = InsertRequest.builder().table(table).records(values).build(); try { Validations.validateInsertRequest(request); - Assert.fail(EXCEPTION_NOT_THROWN); } catch (SkyflowException e) { - Assert.assertEquals(ErrorCode.INVALID_INPUT.getCode(), e.getHttpCode()); - Assert.assertEquals( - Utils.parameterizedString(ErrorMessage.UpsertTableRequestAtRecordLevel.getMessage(), Constants.SDK_PREFIX), - e.getMessage() - ); + Assert.fail(INVALID_EXCEPTION_THROWN); } } @@ -367,13 +360,8 @@ public void testUpsertAtRequestLevelWithNoTable() { InsertRequest request = InsertRequest.builder().records(values).upsert(upsert).build(); try { Validations.validateInsertRequest(request); - Assert.fail(EXCEPTION_NOT_THROWN); } catch (SkyflowException e) { - Assert.assertEquals(ErrorCode.INVALID_INPUT.getCode(), e.getHttpCode()); - Assert.assertEquals( - Utils.parameterizedString(ErrorMessage.UpsertTableRequestAtRequestLevel.getMessage(), Constants.SDK_PREFIX), - e.getMessage() - ); + Assert.fail(INVALID_EXCEPTION_THROWN); } } @@ -387,13 +375,8 @@ public void testUpsertAtRequestLevelWithEmptyTable() { InsertRequest request = InsertRequest.builder().table("").records(values).upsert(upsert).build(); try { Validations.validateInsertRequest(request); - Assert.fail(EXCEPTION_NOT_THROWN); } catch (SkyflowException e) { - Assert.assertEquals(ErrorCode.INVALID_INPUT.getCode(), e.getHttpCode()); - Assert.assertEquals( - Utils.parameterizedString(ErrorMessage.UpsertTableRequestAtRequestLevel.getMessage(), Constants.SDK_PREFIX), - e.getMessage() - ); + Assert.fail(INVALID_EXCEPTION_THROWN); } }