refactor a bit

hypertrace · Oct 12, 2023 · c7628de · c7628de
1 parent c579760
commit c7628de
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 16 deletions.
diff --git a/config-service-impl/src/main/java/org/hypertrace/config/service/ConfigServiceGrpcImpl.java b/config-service-impl/src/main/java/org/hypertrace/config/service/ConfigServiceGrpcImpl.java
@@ -168,12 +168,17 @@ public void deleteConfigs(
           request.getConfigsList().stream()
               .map(this::getConfigResourceContext)
               .collect(Collectors.toUnmodifiableSet());
-      Map<ConfigResourceContext, ContextSpecificConfig> configs =
+      Map<ConfigResourceContext, Optional<ContextSpecificConfig>> configs =
           configStore.getContextConfigs(configResourceContexts);
       // delete the configs for the specified config resources.
       configStore.deleteConfigs(configResourceContexts);
       responseObserver.onNext(
-          DeleteConfigsResponse.newBuilder().addAllDeletedConfigs(configs.values()).build());
+          DeleteConfigsResponse.newBuilder()
+              .addAllDeletedConfigs(
+                  configs.values().stream()
+                      .flatMap(Optional::stream)
+                      .collect(Collectors.toUnmodifiableList()))
+              .build());
       responseObserver.onCompleted();
     } catch (Exception e) {
       log.error("Delete configs failed for request: {}", request, e);

diff --git a/config-service-impl/src/main/java/org/hypertrace/config/service/store/ConfigStore.java b/config-service-impl/src/main/java/org/hypertrace/config/service/store/ConfigStore.java
@@ -53,7 +53,7 @@ Optional<ContextSpecificConfig> getConfig(ConfigResourceContext configResourceCo
    * @return the configs
    * @throws IOException
    */
-  Map<ConfigResourceContext, ContextSpecificConfig> getContextConfigs(
+  Map<ConfigResourceContext, Optional<ContextSpecificConfig>> getContextConfigs(
       Collection<ConfigResourceContext> configResourceContexts) throws IOException;
 
   /**

diff --git a/...g-service-impl/src/main/java/org/hypertrace/config/service/store/DocumentConfigStore.java b/...g-service-impl/src/main/java/org/hypertrace/config/service/store/DocumentConfigStore.java
@@ -7,6 +7,7 @@
 import static org.hypertrace.config.service.store.ConfigDocument.TENANT_ID_FIELD_NAME;
 import static org.hypertrace.config.service.store.ConfigDocument.VERSION_FIELD_NAME;
 
+import com.google.common.collect.Maps;
 import com.google.protobuf.Value;
 import com.typesafe.config.Config;
 import io.grpc.Status;
@@ -19,7 +20,6 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -174,17 +174,12 @@ public Optional<ContextSpecificConfig> getConfig(ConfigResourceContext configRes
   }
 
   @Override
-  public Map<ConfigResourceContext, ContextSpecificConfig> getContextConfigs(
+  public Map<ConfigResourceContext, Optional<ContextSpecificConfig>> getContextConfigs(
       java.util.Collection<ConfigResourceContext> configResourceContexts) throws IOException {
     Map<ConfigResourceContext, Optional<ConfigDocument>> configDocs =
         getLatestVersionConfigDocs(configResourceContexts);
-    return configDocs.entrySet().stream()
-        .filter(entry -> entry.getValue().isPresent())
-        .collect(
-            Collectors.toUnmodifiableMap(
-                Entry::getKey,
-                entry ->
-                    entry.getValue().flatMap(this::convertToContextSpecificConfig).orElseThrow()));
+    return Maps.transformValues(
+        configDocs, maybeConfigDoc -> maybeConfigDoc.flatMap(this::convertToContextSpecificConfig));
   }
 
   @Override
@@ -265,6 +260,9 @@ private Map<ConfigResourceContext, Optional<ConfigDocument>> getLatestVersionCon
 
   private Filter buildConfigResourceContextsFilter(
       java.util.Collection<ConfigResourceContext> configResourceContexts) {
+    if (configResourceContexts.isEmpty()) {
+      throw new RuntimeException("Config resource contexts cannot be empty");
+    }
     List<Filter> childFilters =
         configResourceContexts.stream()
             .map(this::getConfigResourceFieldContextFilter)

diff --git a/...g-service-impl/src/test/java/org/hypertrace/config/service/ConfigServiceGrpcImplTest.java b/...g-service-impl/src/test/java/org/hypertrace/config/service/ConfigServiceGrpcImplTest.java
@@ -230,9 +230,9 @@ void deleteConfigs() throws IOException {
         .thenReturn(
             Map.of(
                 getConfigResourceContext(context1),
-                    buildContextSpecificConfig(context1, config1, 10L, 20L),
+                    Optional.of(buildContextSpecificConfig(context1, config1, 10L, 20L)),
                 getConfigResourceContext(context2),
-                    buildContextSpecificConfig(context2, config2, 10L, 20L)));
+                    Optional.of(buildContextSpecificConfig(context2, config2, 10L, 20L))));
     Runnable runnable =
         () -> configServiceGrpc.deleteConfigs(deleteConfigsRequest, responseObserver);
     RequestContext.forTenantId(TENANT_ID).run(runnable);

diff --git a/...rvice-impl/src/test/java/org/hypertrace/config/service/store/DocumentConfigStoreTest.java b/...rvice-impl/src/test/java/org/hypertrace/config/service/store/DocumentConfigStoreTest.java
@@ -123,9 +123,9 @@ void getConfigs() throws IOException {
             .setCreationTimestamp(TIMESTAMP1)
             .setUpdateTimestamp(TIMESTAMP2)
             .build();
-    Map<ConfigResourceContext, ContextSpecificConfig> actualConfigs =
+    Map<ConfigResourceContext, Optional<ContextSpecificConfig>> actualConfigs =
         configStore.getContextConfigs(Set.of(configResourceContext));
-    assertEquals(Map.of(configResourceContext, expectedConfig), actualConfigs);
+    assertEquals(Map.of(configResourceContext, Optional.of(expectedConfig)), actualConfigs);
   }
 
   @Test