From 2b52eeb727a1476a6a5ab95029a1076c02cecf52 Mon Sep 17 00:00:00 2001 From: Jishnu J <jishnu.j@innovaturelabs.com> Date: Tue, 17 Dec 2024 18:05:04 +0530 Subject: [PATCH 01/10] Initial commit [skip ci] --- .../controlfile/ControlFileTable.java | 40 +++++ .../tablemetadata/TableMetadataException.java | 24 +++ .../tablemetadata/TableMetadataRequest.java | 27 +++ .../tablemetadata/TableMetadataService.java | 63 +++++++ .../core/util/TableMetadataUtil.java | 156 ++++++++++++++++++ 5 files changed, 310 insertions(+) create mode 100644 data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java create mode 100644 data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataException.java create mode 100644 data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java create mode 100644 data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java create mode 100644 data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java new file mode 100644 index 0000000000..d9308794fc --- /dev/null +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java @@ -0,0 +1,40 @@ +package com.scalar.db.dataloader.core.dataimport.controlfile; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import java.util.ArrayList; +import java.util.List; +import lombok.Getter; +import lombok.Setter; + +/** Represents the mapping for one table in the control file */ +@Getter +@Setter +public class ControlFileTable { + + @JsonProperty("namespace") + public String namespace; + + @JsonProperty("table_name") + public String tableName; + + @JsonProperty("mappings") + public List<ControlFileTableFieldMapping> mappings; + + /** Class constructor */ + public ControlFileTable(String namespace, String tableName) { + this.tableName = tableName; + this.namespace = namespace; + this.mappings = new ArrayList<>(); + } + + @JsonCreator + public ControlFileTable( + @JsonProperty("namespace") String namespace, + @JsonProperty("table_name") String tableName, + @JsonProperty("mappings") List<ControlFileTableFieldMapping> mappings) { + this.namespace = namespace; + this.tableName = tableName; + this.mappings = mappings; + } +} diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataException.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataException.java new file mode 100644 index 0000000000..31773a9b64 --- /dev/null +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataException.java @@ -0,0 +1,24 @@ +package com.scalar.db.dataloader.core.tablemetadata; + +/** A custom exception that encapsulates errors thrown by the TableMetaDataService */ +public class TableMetadataException extends Exception { + + /** + * Class constructor + * + * @param message error message + * @param cause reason for exception + */ + public TableMetadataException(String message, Throwable cause) { + super(message, cause); + } + + /** + * Class constructor + * + * @param message error message + */ + public TableMetadataException(String message) { + super(message); + } +} diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java new file mode 100644 index 0000000000..cb2c0fe7e5 --- /dev/null +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java @@ -0,0 +1,27 @@ +package com.scalar.db.dataloader.core.tablemetadata; + +/** Represents the request for metadata for a single ScalarDB table */ +public class TableMetadataRequest { + + private final String namespace; + private final String tableName; + + /** + * Class constructor + * + * @param namespace ScalarDB namespace + * @param tableName ScalarDB table name + */ + public TableMetadataRequest(String namespace, String tableName) { + this.namespace = namespace; + this.tableName = tableName; + } + + public String getNamespace() { + return namespace; + } + + public String getTableName() { + return tableName; + } +} diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java new file mode 100644 index 0000000000..88d60f778e --- /dev/null +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java @@ -0,0 +1,63 @@ +package com.scalar.db.dataloader.core.tablemetadata; + +import com.scalar.db.api.DistributedStorageAdmin; +import com.scalar.db.api.TableMetadata; +import com.scalar.db.dataloader.core.util.TableMetadataUtil; +import com.scalar.db.exception.storage.ExecutionException; +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; +import lombok.RequiredArgsConstructor; + +@RequiredArgsConstructor +public class TableMetadataService { + private static final String ERROR_MISSING_NAMESPACE_OR_TABLE = + "Missing namespace or table: %s, %s"; + + private final DistributedStorageAdmin storageAdmin; + + /** + * Returns the TableMetadata for the given namespace and table name. + * + * @param namespace ScalarDb namespace + * @param tableName ScalarDb table name + * @return TableMetadata + * @throws TableMetadataException if the namespace or table is missing + */ + public TableMetadata getTableMetadata(String namespace, String tableName) + throws TableMetadataException { + try { + TableMetadata tableMetadata = storageAdmin.getTableMetadata(namespace, tableName); + if (tableMetadata == null) { + throw new TableMetadataException( + String.format(ERROR_MISSING_NAMESPACE_OR_TABLE, namespace, tableName)); + } + return tableMetadata; + } catch (ExecutionException e) { + throw new TableMetadataException( + String.format(ERROR_MISSING_NAMESPACE_OR_TABLE, namespace, tableName), e.getCause()); + } + } + + /** + * Returns the TableMetadata for the given list of TableMetadataRequest. + * + * @param requests List of TableMetadataRequest + * @return Map of TableMetadata + * @throws TableMetadataException if the namespace or table is missing + */ + public Map<String, TableMetadata> getTableMetadata(Collection<TableMetadataRequest> requests) + throws TableMetadataException { + Map<String, TableMetadata> metadataMap = new HashMap<>(); + + for (TableMetadataRequest request : requests) { + String namespace = request.getNamespace(); + String tableName = request.getTableName(); + TableMetadata tableMetadata = getTableMetadata(namespace, tableName); + String key = TableMetadataUtil.getTableLookupKey(namespace, tableName); + metadataMap.put(key, tableMetadata); + } + + return metadataMap; + } +} diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java new file mode 100644 index 0000000000..7ce43a3f71 --- /dev/null +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java @@ -0,0 +1,156 @@ +package com.scalar.db.dataloader.core.util; + +import com.scalar.db.api.TableMetadata; +import com.scalar.db.dataloader.core.Constants; +import com.scalar.db.dataloader.core.dataimport.controlfile.ControlFileTable; +import com.scalar.db.io.DataType; +import com.scalar.db.transaction.consensuscommit.Attribute; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** Utils for ScalarDB table metadata */ +public class TableMetadataUtil { + + /** + * Check if the field is a metadata column or not + * + * @param columnName Table column name + * @param metadataColumns Fixed list of metadata columns + * @param columnNames List of all column names in a table + * @return The field is metadata or not + */ + public static boolean isMetadataColumn( + String columnName, Set<String> metadataColumns, Set<String> columnNames) { + // Skip field if it can be ignored + if (metadataColumns.contains(columnName)) { + return true; + } + + // Skip if the field is a "before_" field + return columnName.startsWith(Attribute.BEFORE_PREFIX) + && !columnNames.contains(Attribute.BEFORE_PREFIX + columnName); + } + + /** + * Check if the field is a metadata column or not + * + * @param columnName ScalarDB table column name5 + * @param tableMetadata Metadata for a single ScalarDB + * @return is the field a metadata column or not + */ + public static boolean isMetadataColumn(String columnName, TableMetadata tableMetadata) { + Set<String> metadataColumns = getMetadataColumns(); + LinkedHashSet<String> columnNames = tableMetadata.getColumnNames(); + + // Skip field if it can be ignored + if (metadataColumns.contains(columnName)) { + return true; + } + + // Skip if the field is a "before_" field + return columnName.startsWith(Attribute.BEFORE_PREFIX) + && !columnNames.contains(Attribute.BEFORE_PREFIX + columnName); + } + + /** + * Return a list of fixed metadata columns + * + * @return Set of columns + */ + public static Set<String> getMetadataColumns() { + return Stream.of( + Attribute.ID, + Attribute.STATE, + Attribute.VERSION, + Attribute.PREPARED_AT, + Attribute.COMMITTED_AT, + Attribute.BEFORE_ID, + Attribute.BEFORE_STATE, + Attribute.BEFORE_VERSION, + Attribute.BEFORE_PREPARED_AT, + Attribute.BEFORE_COMMITTED_AT) + .collect(Collectors.toCollection(HashSet::new)); + } + + /** + * Return a map with the data types for all columns in a ScalarDB table + * + * @param tableMetadata Metadata for a single ScalarDB table + * @return data types map + */ + public static Map<String, DataType> extractColumnDataTypes(TableMetadata tableMetadata) { + Map<String, DataType> definitions = new HashMap<>(); + for (String columnName : tableMetadata.getColumnNames()) { + definitions.put(columnName, tableMetadata.getColumnDataType(columnName)); + } + return definitions; + } + + /** + * Return lookup key for a table in a namespace + * + * @param namespace Namespace + * @param tableName Table name + * @return Table metadata lookup key + */ + public static String getTableLookupKey(String namespace, String tableName) { + return String.format(Constants.TABLE_LOOKUP_KEY_FORMAT, namespace, tableName); + } + + /** + * Return lookup key for a table in a namespace + * + * @param controlFileTable Control file data mapping + * @return Table metadata lookup key + */ + public static String getTableLookupKey(ControlFileTable controlFileTable) { + return String.format( + Constants.TABLE_LOOKUP_KEY_FORMAT, controlFileTable.namespace, controlFileTable.tableName); + } + + /** + * Populate the projection columns with metadata columns + * + * @param tableMetadata Metadata for a single ScalarDB table + * @param projections List of projection columns + * @return List of projection columns with metadata columns + */ + public static List<String> populateProjectionsWithMetadata( + TableMetadata tableMetadata, List<String> projections) { + List<String> projectionMetadata = new ArrayList<>(); + + // Add projection columns along with metadata columns + projections.forEach( + projection -> { + projectionMetadata.add(projection); + if (!isKeyColumn(projection, tableMetadata)) { + // Add metadata column before the projection if it's not a key column + projectionMetadata.add(Attribute.BEFORE_PREFIX + projection); + } + }); + + // Add fixed metadata columns + projectionMetadata.addAll(getMetadataColumns()); + + return projectionMetadata; + } + + /** + * Checks if a column is a key column (partition key or clustering key) in the table. + * + * @param column The column name to check. + * @param tableMetadata The metadata of the ScalarDB table. + * @return True if the column is a key column, false otherwise. + */ + private static boolean isKeyColumn(String column, TableMetadata tableMetadata) { + return tableMetadata.getPartitionKeyNames().contains(column) + || tableMetadata.getClusteringKeyNames().contains(column); + } +} From 90c41051e93e5df9928f13cef86611d61076f441 Mon Sep 17 00:00:00 2001 From: Jishnu J <jishnu.j@innovaturelabs.com> Date: Wed, 18 Dec 2024 16:33:02 +0530 Subject: [PATCH 02/10] Added a file [skip ci] --- .../ControlFileTableFieldMapping.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTableFieldMapping.java diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTableFieldMapping.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTableFieldMapping.java new file mode 100644 index 0000000000..78c16ed726 --- /dev/null +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTableFieldMapping.java @@ -0,0 +1,32 @@ +package com.scalar.db.dataloader.core.dataimport.controlfile; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import lombok.Getter; +import lombok.Setter; + +/** Represents the one field mapping for a table mapping in the control file */ +@Getter +@Setter +public class ControlFileTableFieldMapping { + + @JsonProperty("source_field") + public String sourceField; + + @JsonProperty("target_column") + public String targetColumn; + + /** + * Class constructor + * + * @param sourceField The data field in the provided json field + * @param targetColumn The column in the ScalarDB table + */ + @JsonCreator + public ControlFileTableFieldMapping( + @JsonProperty("source_field") String sourceField, + @JsonProperty("target_column") String targetColumn) { + this.sourceField = sourceField; + this.targetColumn = targetColumn; + } +} From 3d5d3e09f5031c5559c48c7d6f5800ff1099dd15 Mon Sep 17 00:00:00 2001 From: Jishnu J <jishnu.j@innovaturelabs.com> Date: Wed, 18 Dec 2024 16:40:14 +0530 Subject: [PATCH 03/10] Added unit test files [skip ci] --- .../TableMetadataServiceTest.java | 53 +++++++++++ .../core/util/TableMetadataUtilTest.java | 87 +++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 data-loader/core/src/test/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataServiceTest.java create mode 100644 data-loader/core/src/test/java/com/scalar/db/dataloader/core/util/TableMetadataUtilTest.java diff --git a/data-loader/core/src/test/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataServiceTest.java b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataServiceTest.java new file mode 100644 index 0000000000..2724ca5d31 --- /dev/null +++ b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataServiceTest.java @@ -0,0 +1,53 @@ +package com.scalar.db.dataloader.core.tablemetadata; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.scalar.db.api.DistributedStorageAdmin; +import com.scalar.db.api.TableMetadata; +import com.scalar.db.dataloader.core.UnitTestUtils; +import com.scalar.db.exception.storage.ExecutionException; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +public class TableMetadataServiceTest { + + DistributedStorageAdmin storageAdmin; + TableMetadataService tableMetadataService; + + @BeforeEach + public void setup() throws ExecutionException { + storageAdmin = Mockito.mock(DistributedStorageAdmin.class); + Mockito.when(storageAdmin.getTableMetadata("namespace", "table")) + .thenReturn(UnitTestUtils.createTestTableMetadata()); + // Mockito.when(storageAdmin.getTableMetadata("namespace1","table1")).thenReturn(null); + tableMetadataService = new TableMetadataService(storageAdmin); + } + + @Test + void getTableMetadata_withValidNamespaceAndTable_shouldReturnTableMetadataMap() + throws TableMetadataException { + + Map<String, TableMetadata> expected = new HashMap<>(); + expected.put("namespace.table", UnitTestUtils.createTestTableMetadata()); + TableMetadataRequest tableMetadataRequest = new TableMetadataRequest("namespace", "table"); + Map<String, TableMetadata> output = + tableMetadataService.getTableMetadata(Collections.singleton(tableMetadataRequest)); + Assertions.assertEquals(expected.get("namespace.table"), output.get("namespace.table")); + } + + @Test + void getTableMetadata_withInvalidNamespaceAndTable_shouldThrowException() + throws TableMetadataException { + TableMetadataRequest tableMetadataRequest = new TableMetadataRequest("namespace2", "table2"); + assertThatThrownBy( + () -> + tableMetadataService.getTableMetadata(Collections.singleton(tableMetadataRequest))) + .isInstanceOf(TableMetadataException.class) + .hasMessage("Missing namespace or table: namespace2, table2"); + } +} diff --git a/data-loader/core/src/test/java/com/scalar/db/dataloader/core/util/TableMetadataUtilTest.java b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/util/TableMetadataUtilTest.java new file mode 100644 index 0000000000..b1a9452ddd --- /dev/null +++ b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/util/TableMetadataUtilTest.java @@ -0,0 +1,87 @@ +package com.scalar.db.dataloader.core.util; + +import static com.scalar.db.dataloader.core.Constants.TABLE_LOOKUP_KEY_FORMAT; +import static org.assertj.core.api.Assertions.assertThat; + +import com.scalar.db.dataloader.core.dataimport.controlfile.ControlFileTable; +import com.scalar.db.transaction.consensuscommit.Attribute; +import java.util.HashSet; +import java.util.Set; +import org.junit.jupiter.api.Test; + +/** Unit tests for TableMetadataUtils */ +class TableMetadataUtilTest { + + private static final String NAMESPACE = "ns"; + private static final String TABLE_NAME = "table"; + + @Test + void isMetadataColumn_IsMetaDataColumn_ShouldReturnTrue() { + boolean isMetadataColumn = + TableMetadataUtil.isMetadataColumn( + Attribute.ID, TableMetadataUtil.getMetadataColumns(), new HashSet<>()); + assertThat(isMetadataColumn).isTrue(); + } + + @Test + void isMetadataColumn_IsNotMetadataColumn_ShouldReturnFalse() { + boolean isMetadataColumn = + TableMetadataUtil.isMetadataColumn( + "columnName", TableMetadataUtil.getMetadataColumns(), new HashSet<>()); + assertThat(isMetadataColumn).isFalse(); + } + + @Test + void isMetadataColumn_IsBeforePrefixColumn_ShouldReturnTrue() { + boolean isMetadataColumn = + TableMetadataUtil.isMetadataColumn( + Attribute.BEFORE_PREFIX + "columnName", + TableMetadataUtil.getMetadataColumns(), + new HashSet<>()); + assertThat(isMetadataColumn).isTrue(); + } + + @Test + void isMetadataColumn_IsNotBeforePrefixColumn_ShouldReturnFalse() { + Set<String> columnNames = new HashSet<>(); + columnNames.add("before_before_testing"); + boolean isMetadataColumn = + TableMetadataUtil.isMetadataColumn( + "before_testing", TableMetadataUtil.getMetadataColumns(), columnNames); + assertThat(isMetadataColumn).isFalse(); + } + + @Test + void getMetadataColumns_NoArgs_ShouldReturnSet() { + + Set<String> columns = new HashSet<>(); + columns.add(Attribute.ID); + columns.add(Attribute.STATE); + columns.add(Attribute.VERSION); + columns.add(Attribute.PREPARED_AT); + columns.add(Attribute.COMMITTED_AT); + columns.add(Attribute.BEFORE_ID); + columns.add(Attribute.BEFORE_STATE); + columns.add(Attribute.BEFORE_VERSION); + columns.add(Attribute.BEFORE_PREPARED_AT); + columns.add(Attribute.BEFORE_COMMITTED_AT); + + Set<String> metadataColumns = TableMetadataUtil.getMetadataColumns(); + assertThat(metadataColumns).containsExactlyInAnyOrder(columns.toArray(new String[0])); + } + + @Test + void getTableLookupKey_ValidStringArgs_ShouldReturnLookupKey() { + String actual = TableMetadataUtil.getTableLookupKey(NAMESPACE, TABLE_NAME); + String expected = String.format(TABLE_LOOKUP_KEY_FORMAT, NAMESPACE, TABLE_NAME); + assertThat(actual).isEqualTo(expected); + } + + @Test + void getTableLookupKey_ValidControlFileArg_ShouldReturnLookupKey() { + ControlFileTable controlFileTable = new ControlFileTable(NAMESPACE, TABLE_NAME); + String actual = TableMetadataUtil.getTableLookupKey(controlFileTable); + String expected = String.format(TABLE_LOOKUP_KEY_FORMAT, NAMESPACE, TABLE_NAME); + assertThat(actual).isEqualTo(expected); + } +} From 64952025b7b860c91450dce95d0d1e684d87cd9b Mon Sep 17 00:00:00 2001 From: Jishnu J <jishnu.j@innovaturelabs.com> Date: Thu, 19 Dec 2024 11:23:48 +0530 Subject: [PATCH 04/10] Spotbug fixes --- .../dataimport/controlfile/ControlFileTable.java | 12 +++++++++--- .../controlfile/ControlFileTableFieldMapping.java | 4 ++-- .../core/tablemetadata/TableMetadataRequest.java | 10 +++------- .../db/dataloader/core/util/TableMetadataUtil.java | 2 +- .../core/tablemetadata/TableMetadataServiceTest.java | 8 +++----- gradle/spotbugs-exclude.xml | 5 +++++ 6 files changed, 23 insertions(+), 18 deletions(-) diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java index d9308794fc..3216193bbf 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java @@ -13,13 +13,13 @@ public class ControlFileTable { @JsonProperty("namespace") - public String namespace; + private String namespace; @JsonProperty("table_name") - public String tableName; + private String tableName; @JsonProperty("mappings") - public List<ControlFileTableFieldMapping> mappings; + private final List<ControlFileTableFieldMapping> mappings; /** Class constructor */ public ControlFileTable(String namespace, String tableName) { @@ -28,6 +28,12 @@ public ControlFileTable(String namespace, String tableName) { this.mappings = new ArrayList<>(); } + /** + * Added for mapping data to control file table object from API request + * @param namespace namespace + * @param tableName table name + * @param mappings column name mapping from control file + */ @JsonCreator public ControlFileTable( @JsonProperty("namespace") String namespace, diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTableFieldMapping.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTableFieldMapping.java index 78c16ed726..064d5c430b 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTableFieldMapping.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTableFieldMapping.java @@ -11,10 +11,10 @@ public class ControlFileTableFieldMapping { @JsonProperty("source_field") - public String sourceField; + private String sourceField; @JsonProperty("target_column") - public String targetColumn; + private String targetColumn; /** * Class constructor diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java index cb2c0fe7e5..0ddd9ab686 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java @@ -1,6 +1,9 @@ package com.scalar.db.dataloader.core.tablemetadata; +import lombok.Getter; + /** Represents the request for metadata for a single ScalarDB table */ +@Getter public class TableMetadataRequest { private final String namespace; @@ -17,11 +20,4 @@ public TableMetadataRequest(String namespace, String tableName) { this.tableName = tableName; } - public String getNamespace() { - return namespace; - } - - public String getTableName() { - return tableName; - } } diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java index 7ce43a3f71..cd3c8d94d5 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java @@ -112,7 +112,7 @@ public static String getTableLookupKey(String namespace, String tableName) { */ public static String getTableLookupKey(ControlFileTable controlFileTable) { return String.format( - Constants.TABLE_LOOKUP_KEY_FORMAT, controlFileTable.namespace, controlFileTable.tableName); + Constants.TABLE_LOOKUP_KEY_FORMAT, controlFileTable.getNamespace(), controlFileTable.getTableName()); } /** diff --git a/data-loader/core/src/test/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataServiceTest.java b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataServiceTest.java index 2724ca5d31..52269a98e5 100644 --- a/data-loader/core/src/test/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataServiceTest.java +++ b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataServiceTest.java @@ -14,17 +14,16 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; -public class TableMetadataServiceTest { +class TableMetadataServiceTest { DistributedStorageAdmin storageAdmin; TableMetadataService tableMetadataService; @BeforeEach - public void setup() throws ExecutionException { + void setup() throws ExecutionException { storageAdmin = Mockito.mock(DistributedStorageAdmin.class); Mockito.when(storageAdmin.getTableMetadata("namespace", "table")) .thenReturn(UnitTestUtils.createTestTableMetadata()); - // Mockito.when(storageAdmin.getTableMetadata("namespace1","table1")).thenReturn(null); tableMetadataService = new TableMetadataService(storageAdmin); } @@ -41,8 +40,7 @@ void getTableMetadata_withValidNamespaceAndTable_shouldReturnTableMetadataMap() } @Test - void getTableMetadata_withInvalidNamespaceAndTable_shouldThrowException() - throws TableMetadataException { + void getTableMetadata_withInvalidNamespaceAndTable_shouldThrowException() { TableMetadataRequest tableMetadataRequest = new TableMetadataRequest("namespace2", "table2"); assertThatThrownBy( () -> diff --git a/gradle/spotbugs-exclude.xml b/gradle/spotbugs-exclude.xml index 05571f3fdb..1724740470 100644 --- a/gradle/spotbugs-exclude.xml +++ b/gradle/spotbugs-exclude.xml @@ -34,4 +34,9 @@ <Bug pattern="ODR_OPEN_DATABASE_RESOURCE"/> </Or> </Match> + <!-- Ignore mutable object exposure warnings(caused by Lombok) for all classes in dataloader.core --> + <Match> + <Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2"/> + <Package name="~com.scalar.db.dataloader.core.*"/> + </Match> </FindBugsFilter> From 30db9882df9f5cc94f08a71a365b142722e672e3 Mon Sep 17 00:00:00 2001 From: Jishnu J <jishnu.j@innovaturelabs.com> Date: Thu, 19 Dec 2024 15:45:44 +0530 Subject: [PATCH 05/10] Applied spotless --- .../core/dataimport/controlfile/ControlFileTable.java | 1 + .../dataloader/core/tablemetadata/TableMetadataRequest.java | 1 - .../com/scalar/db/dataloader/core/util/TableMetadataUtil.java | 4 +++- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java index 3216193bbf..c9b0626e8a 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java @@ -30,6 +30,7 @@ public ControlFileTable(String namespace, String tableName) { /** * Added for mapping data to control file table object from API request + * * @param namespace namespace * @param tableName table name * @param mappings column name mapping from control file diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java index 0ddd9ab686..c0e62f1c52 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java @@ -19,5 +19,4 @@ public TableMetadataRequest(String namespace, String tableName) { this.namespace = namespace; this.tableName = tableName; } - } diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java index cd3c8d94d5..9f10b9b3ea 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java @@ -112,7 +112,9 @@ public static String getTableLookupKey(String namespace, String tableName) { */ public static String getTableLookupKey(ControlFileTable controlFileTable) { return String.format( - Constants.TABLE_LOOKUP_KEY_FORMAT, controlFileTable.getNamespace(), controlFileTable.getTableName()); + Constants.TABLE_LOOKUP_KEY_FORMAT, + controlFileTable.getNamespace(), + controlFileTable.getTableName()); } /** From ccb1ace5e8aa61ae82c5d27cf2a0a162bdea398a Mon Sep 17 00:00:00 2001 From: Peckstadt Yves <peckstadt.yves@gmail.com> Date: Fri, 20 Dec 2024 09:02:09 +0900 Subject: [PATCH 06/10] Improve javadocs --- .../controlfile/ControlFileTable.java | 31 ++++++-- .../ControlFileTableFieldMapping.java | 15 +++- .../tablemetadata/TableMetadataService.java | 30 +++++--- .../core/util/TableMetadataUtil.java | 77 ++++++++----------- 4 files changed, 86 insertions(+), 67 deletions(-) diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java index c9b0626e8a..e1d7c6a9d0 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java @@ -7,33 +7,50 @@ import lombok.Getter; import lombok.Setter; -/** Represents the mapping for one table in the control file */ +/** + * Represents the configuration for a single table in the control file, including its namespace, + * table name, and field mappings. This class is used to define how data from a control file maps to + * a specific table in ScalarDB. + */ @Getter @Setter public class ControlFileTable { + /** The namespace of the table in ScalarDB. */ @JsonProperty("namespace") private String namespace; + /** The name of the table in ScalarDB. */ @JsonProperty("table_name") private String tableName; + /** + * A list of mappings defining the correspondence between control file fields and table columns. + */ @JsonProperty("mappings") private final List<ControlFileTableFieldMapping> mappings; - /** Class constructor */ + /** + * Creates a new {@code ControlFileTable} instance with the specified namespace and table name. + * The mappings list is initialized as an empty list. + * + * @param namespace The namespace of the table in ScalarDB. + * @param tableName The name of the table in ScalarDB. + */ public ControlFileTable(String namespace, String tableName) { - this.tableName = tableName; this.namespace = namespace; + this.tableName = tableName; this.mappings = new ArrayList<>(); } /** - * Added for mapping data to control file table object from API request + * Constructs a {@code ControlFileTable} instance using data from a serialized JSON object. This + * constructor is used for deserialization of API requests or control files. * - * @param namespace namespace - * @param tableName table name - * @param mappings column name mapping from control file + * @param namespace The namespace of the table in ScalarDB. + * @param tableName The name of the table in ScalarDB. + * @param mappings A list of mappings that define the relationship between control file fields and + * table columns. */ @JsonCreator public ControlFileTable( diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTableFieldMapping.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTableFieldMapping.java index 064d5c430b..1068573304 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTableFieldMapping.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTableFieldMapping.java @@ -5,22 +5,29 @@ import lombok.Getter; import lombok.Setter; -/** Represents the one field mapping for a table mapping in the control file */ +/** + * Represents the mapping of a single field in the control file to a column in a ScalarDB table. + * This class defines how data from a specific field in the input source should be mapped to the + * corresponding column in the database. + */ @Getter @Setter public class ControlFileTableFieldMapping { + /** The name of the field in the input source (e.g., JSON or CSV). */ @JsonProperty("source_field") private String sourceField; + /** The name of the column in the ScalarDB table that the field maps to. */ @JsonProperty("target_column") private String targetColumn; /** - * Class constructor + * Constructs a {@code ControlFileTableFieldMapping} instance using data from a serialized JSON + * object. This constructor is primarily used for deserialization of control file mappings. * - * @param sourceField The data field in the provided json field - * @param targetColumn The column in the ScalarDB table + * @param sourceField The name of the field in the input source (e.g., JSON or CSV). + * @param targetColumn The name of the corresponding column in the ScalarDB table. */ @JsonCreator public ControlFileTableFieldMapping( diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java index 88d60f778e..4eea38a95d 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java @@ -9,20 +9,26 @@ import java.util.Map; import lombok.RequiredArgsConstructor; +/** + * Service for retrieving {@link TableMetadata} from ScalarDB. Provides methods to fetch metadata + * for individual tables or a collection of tables. + */ @RequiredArgsConstructor public class TableMetadataService { + private static final String ERROR_MISSING_NAMESPACE_OR_TABLE = "Missing namespace or table: %s, %s"; private final DistributedStorageAdmin storageAdmin; /** - * Returns the TableMetadata for the given namespace and table name. + * Retrieves the {@link TableMetadata} for a specific namespace and table name. * - * @param namespace ScalarDb namespace - * @param tableName ScalarDb table name - * @return TableMetadata - * @throws TableMetadataException if the namespace or table is missing + * @param namespace The ScalarDB namespace. + * @param tableName The name of the table within the specified namespace. + * @return The {@link TableMetadata} object containing schema details of the specified table. + * @throws TableMetadataException If the table or namespace does not exist, or if an error occurs + * while fetching the metadata. */ public TableMetadata getTableMetadata(String namespace, String tableName) throws TableMetadataException { @@ -40,11 +46,17 @@ public TableMetadata getTableMetadata(String namespace, String tableName) } /** - * Returns the TableMetadata for the given list of TableMetadataRequest. + * Retrieves the {@link TableMetadata} for a collection of table metadata requests. + * + * <p>Each request specifies a namespace and table name. The method consolidates the metadata into + * a map keyed by a unique lookup key generated for each table. * - * @param requests List of TableMetadataRequest - * @return Map of TableMetadata - * @throws TableMetadataException if the namespace or table is missing + * @param requests A collection of {@link TableMetadataRequest} objects specifying the tables to + * retrieve metadata for. + * @return A map where the keys are unique lookup keys (namespace + table name) and the values are + * the corresponding {@link TableMetadata} objects. + * @throws TableMetadataException If any of the requested tables or namespaces are missing, or if + * an error occurs while fetching the metadata. */ public Map<String, TableMetadata> getTableMetadata(Collection<TableMetadataRequest> requests) throws TableMetadataException { diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java index 9f10b9b3ea..0b165494d0 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java @@ -15,54 +15,43 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -/** Utils for ScalarDB table metadata */ +/** Utility class for handling ScalarDB table metadata operations. */ public class TableMetadataUtil { /** - * Check if the field is a metadata column or not + * Determines whether a given column is a metadata column based on predefined criteria. * - * @param columnName Table column name - * @param metadataColumns Fixed list of metadata columns - * @param columnNames List of all column names in a table - * @return The field is metadata or not + * @param columnName The name of the table column to check. + * @param metadataColumns A set of predefined metadata columns. + * @param columnNames A set of all column names in the table. + * @return {@code true} if the column is a metadata column; {@code false} otherwise. */ public static boolean isMetadataColumn( String columnName, Set<String> metadataColumns, Set<String> columnNames) { - // Skip field if it can be ignored if (metadataColumns.contains(columnName)) { return true; } - - // Skip if the field is a "before_" field return columnName.startsWith(Attribute.BEFORE_PREFIX) && !columnNames.contains(Attribute.BEFORE_PREFIX + columnName); } /** - * Check if the field is a metadata column or not + * Determines whether a given column is a metadata column using table metadata. * - * @param columnName ScalarDB table column name5 - * @param tableMetadata Metadata for a single ScalarDB - * @return is the field a metadata column or not + * @param columnName The name of the ScalarDB table column to check. + * @param tableMetadata The metadata of the table. + * @return {@code true} if the column is a metadata column; {@code false} otherwise. */ public static boolean isMetadataColumn(String columnName, TableMetadata tableMetadata) { Set<String> metadataColumns = getMetadataColumns(); LinkedHashSet<String> columnNames = tableMetadata.getColumnNames(); - - // Skip field if it can be ignored - if (metadataColumns.contains(columnName)) { - return true; - } - - // Skip if the field is a "before_" field - return columnName.startsWith(Attribute.BEFORE_PREFIX) - && !columnNames.contains(Attribute.BEFORE_PREFIX + columnName); + return isMetadataColumn(columnName, metadataColumns, columnNames); } /** - * Return a list of fixed metadata columns + * Retrieves a set of fixed metadata column names used in ScalarDB. * - * @return Set of columns + * @return A set of predefined metadata column names. */ public static Set<String> getMetadataColumns() { return Stream.of( @@ -80,10 +69,10 @@ public static Set<String> getMetadataColumns() { } /** - * Return a map with the data types for all columns in a ScalarDB table + * Extracts a mapping of column names to their data types from the table metadata. * - * @param tableMetadata Metadata for a single ScalarDB table - * @return data types map + * @param tableMetadata The metadata of the ScalarDB table. + * @return A map where keys are column names and values are their corresponding {@link DataType}. */ public static Map<String, DataType> extractColumnDataTypes(TableMetadata tableMetadata) { Map<String, DataType> definitions = new HashMap<>(); @@ -94,21 +83,21 @@ public static Map<String, DataType> extractColumnDataTypes(TableMetadata tableMe } /** - * Return lookup key for a table in a namespace + * Generates a unique lookup key for a table within a namespace. * - * @param namespace Namespace - * @param tableName Table name - * @return Table metadata lookup key + * @param namespace The namespace of the table. + * @param tableName The name of the table. + * @return A formatted string representing the table lookup key. */ public static String getTableLookupKey(String namespace, String tableName) { return String.format(Constants.TABLE_LOOKUP_KEY_FORMAT, namespace, tableName); } /** - * Return lookup key for a table in a namespace + * Generates a unique lookup key for a table using control file table data. * - * @param controlFileTable Control file data mapping - * @return Table metadata lookup key + * @param controlFileTable The control file table object containing namespace and table name. + * @return A formatted string representing the table lookup key. */ public static String getTableLookupKey(ControlFileTable controlFileTable) { return String.format( @@ -118,38 +107,32 @@ public static String getTableLookupKey(ControlFileTable controlFileTable) { } /** - * Populate the projection columns with metadata columns + * Adds metadata columns to a list of projection columns for a ScalarDB table. * - * @param tableMetadata Metadata for a single ScalarDB table - * @param projections List of projection columns - * @return List of projection columns with metadata columns + * @param tableMetadata The metadata of the ScalarDB table. + * @param projections A list of projection column names. + * @return A new list containing projection columns along with metadata columns. */ public static List<String> populateProjectionsWithMetadata( TableMetadata tableMetadata, List<String> projections) { List<String> projectionMetadata = new ArrayList<>(); - - // Add projection columns along with metadata columns projections.forEach( projection -> { projectionMetadata.add(projection); if (!isKeyColumn(projection, tableMetadata)) { - // Add metadata column before the projection if it's not a key column projectionMetadata.add(Attribute.BEFORE_PREFIX + projection); } }); - - // Add fixed metadata columns projectionMetadata.addAll(getMetadataColumns()); - return projectionMetadata; } /** - * Checks if a column is a key column (partition key or clustering key) in the table. + * Checks whether a column is a key column (partition key or clustering key) in the table. * - * @param column The column name to check. + * @param column The name of the column to check. * @param tableMetadata The metadata of the ScalarDB table. - * @return True if the column is a key column, false otherwise. + * @return {@code true} if the column is a key column; {@code false} otherwise. */ private static boolean isKeyColumn(String column, TableMetadata tableMetadata) { return tableMetadata.getPartitionKeyNames().contains(column) From a374f1a57d95d0cc10e845e5a96c9fa9d32bce9e Mon Sep 17 00:00:00 2001 From: Peckstadt Yves <peckstadt.yves@gmail.com> Date: Fri, 20 Dec 2024 09:02:35 +0900 Subject: [PATCH 07/10] Add private constructor to TableMetadataUtil --- .../com/scalar/db/dataloader/core/util/TableMetadataUtil.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java index 0b165494d0..0c5daedd52 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java @@ -5,6 +5,9 @@ import com.scalar.db.dataloader.core.dataimport.controlfile.ControlFileTable; import com.scalar.db.io.DataType; import com.scalar.db.transaction.consensuscommit.Attribute; +import lombok.AccessLevel; +import lombok.NoArgsConstructor; + import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -16,6 +19,7 @@ import java.util.stream.Stream; /** Utility class for handling ScalarDB table metadata operations. */ +@NoArgsConstructor(access = AccessLevel.PRIVATE) public class TableMetadataUtil { /** From a65c9b5ba91d8d523198eb57e63e265ef8f6bfd3 Mon Sep 17 00:00:00 2001 From: Peckstadt Yves <peckstadt.yves@gmail.com> Date: Fri, 20 Dec 2024 09:24:08 +0900 Subject: [PATCH 08/10] Apply spotless fix --- .../scalar/db/dataloader/core/util/TableMetadataUtil.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java index 0c5daedd52..acfd509d0f 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java @@ -5,9 +5,6 @@ import com.scalar.db.dataloader.core.dataimport.controlfile.ControlFileTable; import com.scalar.db.io.DataType; import com.scalar.db.transaction.consensuscommit.Attribute; -import lombok.AccessLevel; -import lombok.NoArgsConstructor; - import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -17,6 +14,8 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; +import lombok.AccessLevel; +import lombok.NoArgsConstructor; /** Utility class for handling ScalarDB table metadata operations. */ @NoArgsConstructor(access = AccessLevel.PRIVATE) From 3b766534c4dcc60a142022a039b08bf11cd0831b Mon Sep 17 00:00:00 2001 From: Jishnu J <jishnu.j@innovaturelabs.com> Date: Thu, 16 Jan 2025 17:01:29 +0530 Subject: [PATCH 09/10] Changes --- .../controlfile/ControlFileTable.java | 16 ++--- .../tablemetadata/TableMetadataRequest.java | 8 +-- .../tablemetadata/TableMetadataService.java | 2 +- .../core/util/TableMetadataUtil.java | 42 +------------- .../core/util/TableMetadataUtilTest.java | 58 ------------------- 5 files changed, 16 insertions(+), 110 deletions(-) diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java index e1d7c6a9d0..efcfb0bc00 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileTable.java @@ -21,8 +21,8 @@ public class ControlFileTable { private String namespace; /** The name of the table in ScalarDB. */ - @JsonProperty("table_name") - private String tableName; + @JsonProperty("table") + private String table; /** * A list of mappings defining the correspondence between control file fields and table columns. @@ -35,11 +35,11 @@ public class ControlFileTable { * The mappings list is initialized as an empty list. * * @param namespace The namespace of the table in ScalarDB. - * @param tableName The name of the table in ScalarDB. + * @param table The name of the table in ScalarDB. */ - public ControlFileTable(String namespace, String tableName) { + public ControlFileTable(String namespace, String table) { this.namespace = namespace; - this.tableName = tableName; + this.table = table; this.mappings = new ArrayList<>(); } @@ -48,17 +48,17 @@ public ControlFileTable(String namespace, String tableName) { * constructor is used for deserialization of API requests or control files. * * @param namespace The namespace of the table in ScalarDB. - * @param tableName The name of the table in ScalarDB. + * @param table The name of the table in ScalarDB. * @param mappings A list of mappings that define the relationship between control file fields and * table columns. */ @JsonCreator public ControlFileTable( @JsonProperty("namespace") String namespace, - @JsonProperty("table_name") String tableName, + @JsonProperty("table") String table, @JsonProperty("mappings") List<ControlFileTableFieldMapping> mappings) { this.namespace = namespace; - this.tableName = tableName; + this.table = table; this.mappings = mappings; } } diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java index c0e62f1c52..8e79da3d6b 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataRequest.java @@ -7,16 +7,16 @@ public class TableMetadataRequest { private final String namespace; - private final String tableName; + private final String table; /** * Class constructor * * @param namespace ScalarDB namespace - * @param tableName ScalarDB table name + * @param table ScalarDB table name */ - public TableMetadataRequest(String namespace, String tableName) { + public TableMetadataRequest(String namespace, String table) { this.namespace = namespace; - this.tableName = tableName; + this.table = table; } } diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java index 4eea38a95d..d0b3b25e5d 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java @@ -64,7 +64,7 @@ public Map<String, TableMetadata> getTableMetadata(Collection<TableMetadataReque for (TableMetadataRequest request : requests) { String namespace = request.getNamespace(); - String tableName = request.getTableName(); + String tableName = request.getTable(); TableMetadata tableMetadata = getTableMetadata(namespace, tableName); String key = TableMetadataUtil.getTableLookupKey(namespace, tableName); metadataMap.put(key, tableMetadata); diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java index acfd509d0f..2ced747be1 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java @@ -5,15 +5,12 @@ import com.scalar.db.dataloader.core.dataimport.controlfile.ControlFileTable; import com.scalar.db.io.DataType; import com.scalar.db.transaction.consensuscommit.Attribute; +import com.scalar.db.transaction.consensuscommit.ConsensusCommitUtils; import java.util.ArrayList; import java.util.HashMap; -import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; -import java.util.stream.Stream; import lombok.AccessLevel; import lombok.NoArgsConstructor; @@ -38,39 +35,6 @@ public static boolean isMetadataColumn( && !columnNames.contains(Attribute.BEFORE_PREFIX + columnName); } - /** - * Determines whether a given column is a metadata column using table metadata. - * - * @param columnName The name of the ScalarDB table column to check. - * @param tableMetadata The metadata of the table. - * @return {@code true} if the column is a metadata column; {@code false} otherwise. - */ - public static boolean isMetadataColumn(String columnName, TableMetadata tableMetadata) { - Set<String> metadataColumns = getMetadataColumns(); - LinkedHashSet<String> columnNames = tableMetadata.getColumnNames(); - return isMetadataColumn(columnName, metadataColumns, columnNames); - } - - /** - * Retrieves a set of fixed metadata column names used in ScalarDB. - * - * @return A set of predefined metadata column names. - */ - public static Set<String> getMetadataColumns() { - return Stream.of( - Attribute.ID, - Attribute.STATE, - Attribute.VERSION, - Attribute.PREPARED_AT, - Attribute.COMMITTED_AT, - Attribute.BEFORE_ID, - Attribute.BEFORE_STATE, - Attribute.BEFORE_VERSION, - Attribute.BEFORE_PREPARED_AT, - Attribute.BEFORE_COMMITTED_AT) - .collect(Collectors.toCollection(HashSet::new)); - } - /** * Extracts a mapping of column names to their data types from the table metadata. * @@ -106,7 +70,7 @@ public static String getTableLookupKey(ControlFileTable controlFileTable) { return String.format( Constants.TABLE_LOOKUP_KEY_FORMAT, controlFileTable.getNamespace(), - controlFileTable.getTableName()); + controlFileTable.getTable()); } /** @@ -126,7 +90,7 @@ public static List<String> populateProjectionsWithMetadata( projectionMetadata.add(Attribute.BEFORE_PREFIX + projection); } }); - projectionMetadata.addAll(getMetadataColumns()); + projectionMetadata.addAll(ConsensusCommitUtils.getTransactionMetaColumns().keySet()); return projectionMetadata; } diff --git a/data-loader/core/src/test/java/com/scalar/db/dataloader/core/util/TableMetadataUtilTest.java b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/util/TableMetadataUtilTest.java index b1a9452ddd..7c75c02658 100644 --- a/data-loader/core/src/test/java/com/scalar/db/dataloader/core/util/TableMetadataUtilTest.java +++ b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/util/TableMetadataUtilTest.java @@ -4,9 +4,6 @@ import static org.assertj.core.api.Assertions.assertThat; import com.scalar.db.dataloader.core.dataimport.controlfile.ControlFileTable; -import com.scalar.db.transaction.consensuscommit.Attribute; -import java.util.HashSet; -import java.util.Set; import org.junit.jupiter.api.Test; /** Unit tests for TableMetadataUtils */ @@ -15,61 +12,6 @@ class TableMetadataUtilTest { private static final String NAMESPACE = "ns"; private static final String TABLE_NAME = "table"; - @Test - void isMetadataColumn_IsMetaDataColumn_ShouldReturnTrue() { - boolean isMetadataColumn = - TableMetadataUtil.isMetadataColumn( - Attribute.ID, TableMetadataUtil.getMetadataColumns(), new HashSet<>()); - assertThat(isMetadataColumn).isTrue(); - } - - @Test - void isMetadataColumn_IsNotMetadataColumn_ShouldReturnFalse() { - boolean isMetadataColumn = - TableMetadataUtil.isMetadataColumn( - "columnName", TableMetadataUtil.getMetadataColumns(), new HashSet<>()); - assertThat(isMetadataColumn).isFalse(); - } - - @Test - void isMetadataColumn_IsBeforePrefixColumn_ShouldReturnTrue() { - boolean isMetadataColumn = - TableMetadataUtil.isMetadataColumn( - Attribute.BEFORE_PREFIX + "columnName", - TableMetadataUtil.getMetadataColumns(), - new HashSet<>()); - assertThat(isMetadataColumn).isTrue(); - } - - @Test - void isMetadataColumn_IsNotBeforePrefixColumn_ShouldReturnFalse() { - Set<String> columnNames = new HashSet<>(); - columnNames.add("before_before_testing"); - boolean isMetadataColumn = - TableMetadataUtil.isMetadataColumn( - "before_testing", TableMetadataUtil.getMetadataColumns(), columnNames); - assertThat(isMetadataColumn).isFalse(); - } - - @Test - void getMetadataColumns_NoArgs_ShouldReturnSet() { - - Set<String> columns = new HashSet<>(); - columns.add(Attribute.ID); - columns.add(Attribute.STATE); - columns.add(Attribute.VERSION); - columns.add(Attribute.PREPARED_AT); - columns.add(Attribute.COMMITTED_AT); - columns.add(Attribute.BEFORE_ID); - columns.add(Attribute.BEFORE_STATE); - columns.add(Attribute.BEFORE_VERSION); - columns.add(Attribute.BEFORE_PREPARED_AT); - columns.add(Attribute.BEFORE_COMMITTED_AT); - - Set<String> metadataColumns = TableMetadataUtil.getMetadataColumns(); - assertThat(metadataColumns).containsExactlyInAnyOrder(columns.toArray(new String[0])); - } - @Test void getTableLookupKey_ValidStringArgs_ShouldReturnLookupKey() { String actual = TableMetadataUtil.getTableLookupKey(NAMESPACE, TABLE_NAME); From d984eea2916df7728f4c97563864f0b4031ae0f1 Mon Sep 17 00:00:00 2001 From: Jishnu J <jishnu.j@innovaturelabs.com> Date: Mon, 20 Jan 2025 10:14:04 +0530 Subject: [PATCH 10/10] Changes update --- .../com/scalar/db/common/error/CoreError.java | 2 ++ .../tablemetadata/TableMetadataService.java | 9 ++++----- .../core/util/TableMetadataUtil.java | 18 ------------------ .../TableMetadataServiceTest.java | 4 +++- 4 files changed, 9 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/com/scalar/db/common/error/CoreError.java b/core/src/main/java/com/scalar/db/common/error/CoreError.java index 397ac6ac01..ceb1d5e5c5 100644 --- a/core/src/main/java/com/scalar/db/common/error/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/error/CoreError.java @@ -718,6 +718,8 @@ public enum CoreError implements ScalarDbError { "The provided partition key order does not match the table schema. Required order: %s", "", ""), + DATA_LOADER_MISSING_NAMESPACE_OR_TABLE( + Category.USER_ERROR, "0158", "Missing namespace or table: %s, %s", "", ""), // // Errors for the concurrency error category diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java index d0b3b25e5d..62552a6db1 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataService.java @@ -2,6 +2,7 @@ import com.scalar.db.api.DistributedStorageAdmin; import com.scalar.db.api.TableMetadata; +import com.scalar.db.common.error.CoreError; import com.scalar.db.dataloader.core.util.TableMetadataUtil; import com.scalar.db.exception.storage.ExecutionException; import java.util.Collection; @@ -16,9 +17,6 @@ @RequiredArgsConstructor public class TableMetadataService { - private static final String ERROR_MISSING_NAMESPACE_OR_TABLE = - "Missing namespace or table: %s, %s"; - private final DistributedStorageAdmin storageAdmin; /** @@ -36,12 +34,13 @@ public TableMetadata getTableMetadata(String namespace, String tableName) TableMetadata tableMetadata = storageAdmin.getTableMetadata(namespace, tableName); if (tableMetadata == null) { throw new TableMetadataException( - String.format(ERROR_MISSING_NAMESPACE_OR_TABLE, namespace, tableName)); + CoreError.DATA_LOADER_MISSING_NAMESPACE_OR_TABLE.buildMessage(namespace, tableName)); } return tableMetadata; } catch (ExecutionException e) { throw new TableMetadataException( - String.format(ERROR_MISSING_NAMESPACE_OR_TABLE, namespace, tableName), e.getCause()); + CoreError.DATA_LOADER_MISSING_NAMESPACE_OR_TABLE.buildMessage(namespace, tableName), + e.getCause()); } } diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java index 2ced747be1..1dc9231b43 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/TableMetadataUtil.java @@ -10,7 +10,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Set; import lombok.AccessLevel; import lombok.NoArgsConstructor; @@ -18,23 +17,6 @@ @NoArgsConstructor(access = AccessLevel.PRIVATE) public class TableMetadataUtil { - /** - * Determines whether a given column is a metadata column based on predefined criteria. - * - * @param columnName The name of the table column to check. - * @param metadataColumns A set of predefined metadata columns. - * @param columnNames A set of all column names in the table. - * @return {@code true} if the column is a metadata column; {@code false} otherwise. - */ - public static boolean isMetadataColumn( - String columnName, Set<String> metadataColumns, Set<String> columnNames) { - if (metadataColumns.contains(columnName)) { - return true; - } - return columnName.startsWith(Attribute.BEFORE_PREFIX) - && !columnNames.contains(Attribute.BEFORE_PREFIX + columnName); - } - /** * Extracts a mapping of column names to their data types from the table metadata. * diff --git a/data-loader/core/src/test/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataServiceTest.java b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataServiceTest.java index 52269a98e5..9bcd06bf9b 100644 --- a/data-loader/core/src/test/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataServiceTest.java +++ b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/tablemetadata/TableMetadataServiceTest.java @@ -4,6 +4,7 @@ import com.scalar.db.api.DistributedStorageAdmin; import com.scalar.db.api.TableMetadata; +import com.scalar.db.common.error.CoreError; import com.scalar.db.dataloader.core.UnitTestUtils; import com.scalar.db.exception.storage.ExecutionException; import java.util.Collections; @@ -46,6 +47,7 @@ void getTableMetadata_withInvalidNamespaceAndTable_shouldThrowException() { () -> tableMetadataService.getTableMetadata(Collections.singleton(tableMetadataRequest))) .isInstanceOf(TableMetadataException.class) - .hasMessage("Missing namespace or table: namespace2, table2"); + .hasMessage( + CoreError.DATA_LOADER_MISSING_NAMESPACE_OR_TABLE.buildMessage("namespace2", "table2")); } }