From c7628de78ef8b0964b75af7832873038364acbdd Mon Sep 17 00:00:00 2001 From: aman-bansal Date: Thu, 12 Oct 2023 10:06:21 +0530 Subject: [PATCH] refactor a bit --- .../config/service/ConfigServiceGrpcImpl.java | 9 +++++++-- .../config/service/store/ConfigStore.java | 2 +- .../service/store/DocumentConfigStore.java | 16 +++++++--------- .../service/ConfigServiceGrpcImplTest.java | 4 ++-- .../service/store/DocumentConfigStoreTest.java | 4 ++-- 5 files changed, 19 insertions(+), 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 index ef0c23f0..1dcbb976 100644 --- 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 configs = + Map> 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 index f1d1ea84..ffc94b48 100644 --- 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 getConfig(ConfigResourceContext configResourceCo * @return the configs * @throws IOException */ - Map getContextConfigs( + Map> getContextConfigs( Collection configResourceContexts) throws IOException; /** diff --git a/config-service-impl/src/main/java/org/hypertrace/config/service/store/DocumentConfigStore.java b/config-service-impl/src/main/java/org/hypertrace/config/service/store/DocumentConfigStore.java index 95640c20..410405cc 100644 --- a/config-service-impl/src/main/java/org/hypertrace/config/service/store/DocumentConfigStore.java +++ b/config-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 getConfig(ConfigResourceContext configRes } @Override - public Map getContextConfigs( + public Map> getContextConfigs( java.util.Collection configResourceContexts) throws IOException { Map> 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> getLatestVersionCon private Filter buildConfigResourceContextsFilter( java.util.Collection configResourceContexts) { + if (configResourceContexts.isEmpty()) { + throw new RuntimeException("Config resource contexts cannot be empty"); + } List childFilters = configResourceContexts.stream() .map(this::getConfigResourceFieldContextFilter) diff --git a/config-service-impl/src/test/java/org/hypertrace/config/service/ConfigServiceGrpcImplTest.java b/config-service-impl/src/test/java/org/hypertrace/config/service/ConfigServiceGrpcImplTest.java index 8f2b41fc..25612ca1 100644 --- a/config-service-impl/src/test/java/org/hypertrace/config/service/ConfigServiceGrpcImplTest.java +++ b/config-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/config-service-impl/src/test/java/org/hypertrace/config/service/store/DocumentConfigStoreTest.java b/config-service-impl/src/test/java/org/hypertrace/config/service/store/DocumentConfigStoreTest.java index 71a1c3c7..f4b3f2cf 100644 --- a/config-service-impl/src/test/java/org/hypertrace/config/service/store/DocumentConfigStoreTest.java +++ b/config-service-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 actualConfigs = + Map> actualConfigs = configStore.getContextConfigs(Set.of(configResourceContext)); - assertEquals(Map.of(configResourceContext, expectedConfig), actualConfigs); + assertEquals(Map.of(configResourceContext, Optional.of(expectedConfig)), actualConfigs); } @Test