Added logic in HouseTableMapper to map storagetype

linkedin · Sep 11, 2024 · c848b37 · c848b37
1 parent 09ea3a6
commit c848b37
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 21 deletions.
diff --git a/...c/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperations.java b/...c/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperations.java
@@ -226,9 +226,7 @@ protected void doCommit(TableMetadata base, TableMetadata metadata) {
               TableMetadataParser.write(updatedMtDataRef, io().newOutputFile(newMetadataLocation)),
           InternalCatalogMetricsConstant.METADATA_UPDATE_LATENCY);
 
-      houseTable = houseTableMapper.toHouseTable(updatedMetadata);
-      // Set the storage type of table in house table before persisting in hts
-      houseTable.setStorageType(fileIOManager.getStorage(fileIO).getType().getValue());
+      houseTable = houseTableMapper.toHouseTable(updatedMetadata, fileIO);
       if (!isStageCreate) {
         houseTableRepository.save(houseTable);
       } else {

diff --git a/...atalog/src/main/java/com/linkedin/openhouse/internal/catalog/mapper/HouseTableMapper.java b/...atalog/src/main/java/com/linkedin/openhouse/internal/catalog/mapper/HouseTableMapper.java
@@ -4,22 +4,30 @@
 import static com.linkedin.openhouse.internal.catalog.mapper.HouseTableSerdeUtils.OPENHOUSE_NAMESPACE;
 
 import com.linkedin.openhouse.housetables.client.model.UserTable;
+import com.linkedin.openhouse.internal.catalog.fileio.FileIOManager;
 import com.linkedin.openhouse.internal.catalog.model.HouseTable;
 import java.util.HashMap;
 import java.util.Map;
 import org.apache.iceberg.TableMetadata;
+import org.apache.iceberg.io.FileIO;
 import org.mapstruct.Mapper;
 import org.mapstruct.Mapping;
 import org.mapstruct.Mappings;
+import org.springframework.beans.factory.annotation.Autowired;
 
 @Mapper(componentModel = "spring")
 public abstract class HouseTableMapper {
+  @Autowired FileIOManager fileIOManager;
+
   @Mapping(target = "lastModifiedTime", ignore = true)
   @Mapping(target = "creationTime", ignore = true)
-  public abstract HouseTable toHouseTable(Map<String, String> properties);
+  @Mapping(
+      target = "storageType",
+      expression = "java(fileIOManager.getStorage(fileIO).getType().getValue())")
+  public abstract HouseTable toHouseTable(Map<String, String> properties, FileIO fileIO);
 
-  public HouseTable toHouseTable(TableMetadata tableMetadata) {
-    return toHouseTable(extractRawHTSFields(tableMetadata.properties()));
+  public HouseTable toHouseTable(TableMetadata tableMetadata, FileIO fileIO) {
+    return toHouseTable(extractRawHTSFields(tableMetadata.properties()), fileIO);
   }
 
   @Mappings({@Mapping(target = "tableLocation", source = "userTable.metadataLocation")})

diff --git a/...ternalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/model/HouseTable.java b/...ternalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/model/HouseTable.java
@@ -9,7 +9,6 @@
 import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
-import lombok.Setter;
 
 /** Data Model for persisting Table Object in the HTS-Repository. */
 @Entity
@@ -47,7 +46,5 @@ public class HouseTable {
    * com.linkedin.openhouse.cluster.storage.StorageClient} implementation that is used to interact
    * with this table.
    */
-  // Fields that are not in table properties need to be set via setter explicitly
-  // see {@link com.linkedin.openhouse.internal.catalog.mapper.HouseTableMapper}
-  @Setter private String storageType;
+  private String storageType;
 }
diff --git a/...st/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperationsTest.java b/...st/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalTableOperationsTest.java
@@ -78,7 +78,7 @@ private static String getTempLocation() {
   @BeforeEach
   void setup() {
     MockitoAnnotations.openMocks(this);
-    Mockito.when(mockHouseTableMapper.toHouseTable(Mockito.any(TableMetadata.class)))
+    Mockito.when(mockHouseTableMapper.toHouseTable(Mockito.any(TableMetadata.class), Mockito.any()))
         .thenReturn(mockHouseTable);
     HadoopFileIO fileIO = new HadoopFileIO(new Configuration());
     openHouseInternalTableOperations =
@@ -111,7 +111,7 @@ void testDoCommitAppendSnapshotsInitialVersion() throws IOException {
 
       TableMetadata metadata = BASE_TABLE_METADATA.replaceProperties(properties);
       openHouseInternalTableOperations.doCommit(BASE_TABLE_METADATA, metadata);
-      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture());
+      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture(), Mockito.any());
 
       Map<String, String> updatedProperties = tblMetadataCaptor.getValue().properties();
       Assertions.assertEquals(
@@ -152,7 +152,7 @@ void testDoCommitAppendSnapshotsExistingVersion() throws IOException {
 
       TableMetadata metadata = base.replaceProperties(properties);
       openHouseInternalTableOperations.doCommit(base, metadata);
-      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture());
+      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture(), Mockito.any());
 
       Map<String, String> updatedProperties = tblMetadataCaptor.getValue().properties();
       Assertions.assertEquals(
@@ -202,7 +202,7 @@ void testDoCommitAppendAndDeleteSnapshots() throws IOException {
 
       TableMetadata metadata = base.replaceProperties(properties);
       openHouseInternalTableOperations.doCommit(base, metadata);
-      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture());
+      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture(), Mockito.any());
 
       Map<String, String> updatedProperties = tblMetadataCaptor.getValue().properties();
       Assertions.assertEquals(
@@ -256,7 +256,7 @@ void testDoCommitDeleteSnapshots() throws IOException {
 
       TableMetadata metadata = base.replaceProperties(properties);
       openHouseInternalTableOperations.doCommit(base, metadata);
-      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture());
+      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture(), Mockito.any());
 
       Map<String, String> updatedProperties = tblMetadataCaptor.getValue().properties();
       Assertions.assertEquals(
@@ -369,7 +369,7 @@ void testDoCommitAppendStageOnlySnapshotsInitialVersion() throws IOException {
 
       TableMetadata metadata = BASE_TABLE_METADATA.replaceProperties(properties);
       openHouseInternalTableOperations.doCommit(BASE_TABLE_METADATA, metadata);
-      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture());
+      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture(), Mockito.any());
       Map<String, String> updatedProperties = tblMetadataCaptor.getValue().properties();
 
       // verify snapshots are staged but not appended
@@ -414,7 +414,7 @@ void testDoCommitAppendStageOnlySnapshotsExistingVersion() throws IOException {
 
       TableMetadata metadata = base.replaceProperties(properties);
       openHouseInternalTableOperations.doCommit(base, metadata);
-      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture());
+      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture(), Mockito.any());
       Map<String, String> updatedProperties = tblMetadataCaptor.getValue().properties();
 
       // verify snapshots are staged but not appended
@@ -504,7 +504,7 @@ void testDoCommitCherryPickSnapshotBaseUnchanged() throws IOException {
 
       TableMetadata metadata = base.replaceProperties(properties);
       openHouseInternalTableOperations.doCommit(base, metadata);
-      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture());
+      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture(), Mockito.any());
       Map<String, String> updatedProperties = tblMetadataCaptor.getValue().properties();
 
       // verify the staged snapshot is cherry picked by use the existing one
@@ -545,7 +545,7 @@ void testDoCommitCherryPickSnapshotBaseChanged() throws IOException {
 
       TableMetadata metadata = base.replaceProperties(properties);
       openHouseInternalTableOperations.doCommit(base, metadata);
-      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture());
+      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture(), Mockito.any());
       Map<String, String> updatedProperties = tblMetadataCaptor.getValue().properties();
 
       // verify the staged snapshot is cherry picked by creating a new snapshot and append it
@@ -583,7 +583,7 @@ void testDoCommitCherryPickFirstSnapshot() throws IOException {
 
       TableMetadata metadata = base.replaceProperties(properties);
       openHouseInternalTableOperations.doCommit(base, metadata);
-      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture());
+      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture(), Mockito.any());
       Map<String, String> updatedProperties = tblMetadataCaptor.getValue().properties();
 
       // verify the staged snapshot is cherry picked by using the existing one
@@ -614,7 +614,7 @@ void testDoCommitDeleteLastStagedSnapshotWhenNoRefs() throws IOException {
 
       TableMetadata metadata = base.replaceProperties(properties);
       openHouseInternalTableOperations.doCommit(base, metadata);
-      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture());
+      Mockito.verify(mockHouseTableMapper).toHouseTable(tblMetadataCaptor.capture(), Mockito.any());
       Map<String, String> updatedProperties = tblMetadataCaptor.getValue().properties();
 
       // verify nothing happens