Skip to content

Commit

Permalink
feat: bulk delete configs support (#149)
Browse files Browse the repository at this point in the history 
* bulk delete configs api

* address review comments

* chore: add impl

* nit

* fix onError
  • Loading branch information
@SrikarMannepalli
SrikarMannepalli authored Jan 10, 2023
1 parent 948c3a3 commit 018652b
Show file tree
Hide file tree
Showing 4 changed files with 301 additions and 41 deletions.
20 changes: 20 additions & 0 deletions config-service-api/src/main/proto/org/hypertrace/config/service/v1/config_service.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ service ConfigService {
rpc DeleteConfig(DeleteConfigRequest) returns (DeleteConfigResponse) {}

rpc UpsertAllConfigs(UpsertAllConfigsRequest) returns (UpsertAllConfigsResponse) {}

rpc DeleteConfigs(DeleteConfigsRequest) returns (DeleteConfigsResponse) {}
}

message UpsertConfigRequest {
Expand Down Expand Up @@ -119,6 +121,24 @@ message DeleteConfigResponse {
ContextSpecificConfig deleted_config = 1;
}

message DeleteConfigsRequest {
repeated ConfigToDelete configs = 1;

message ConfigToDelete {
// required - name of the resource associated with the config
string resource_name = 1;
// required - namespace with which the config resource is associated
string resource_namespace = 2;
// optional - only required if config applies to a specific context.
// If empty, specified config is associated with a default context.
string context = 3;
}
}

message DeleteConfigsResponse {
repeated ContextSpecificConfig deleted_configs = 1;
}

message UpsertAllConfigsRequest {
repeated ConfigToUpsert configs = 1;

Expand Down
71 changes: 63 additions & 8 deletions config-service-impl/src/main/java/org/hypertrace/config/service/ConfigServiceGrpcImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,16 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.stream.Collectors;
import lombok.extern.slf4j.Slf4j;
import org.hypertrace.config.service.store.ConfigStore;
import org.hypertrace.config.service.v1.ConfigServiceGrpc;
import org.hypertrace.config.service.v1.ContextSpecificConfig;
import org.hypertrace.config.service.v1.DeleteConfigRequest;
import org.hypertrace.config.service.v1.DeleteConfigResponse;
import org.hypertrace.config.service.v1.DeleteConfigsRequest;
import org.hypertrace.config.service.v1.DeleteConfigsRequest.ConfigToDelete;
import org.hypertrace.config.service.v1.DeleteConfigsResponse;
import org.hypertrace.config.service.v1.GetAllConfigsRequest;
import org.hypertrace.config.service.v1.GetAllConfigsResponse;
import org.hypertrace.config.service.v1.GetConfigRequest;
Expand Down Expand Up @@ -140,6 +144,50 @@ public void deleteConfig(
}
}

@Override
public void deleteConfigs(
DeleteConfigsRequest request, StreamObserver<DeleteConfigsResponse> responseObserver) {
try {
if (request.getConfigsCount() == 0) {
responseObserver.onError(
Status.INVALID_ARGUMENT
.withDescription("List of configs to delete provided is empty")
.asException());
return;
}
Map<ConfigResourceContext, Value> valuesByContext =
request.getConfigsList().stream()
.map(
requestedDelete ->
Map.entry(this.getConfigResourceContext(requestedDelete), emptyValue()))
.collect(ImmutableMap.toImmutableMap(Entry::getKey, Entry::getValue));

List<UpsertedConfig> deletedConfigs =
configStore.writeAllConfigs(valuesByContext, getUserId());

responseObserver.onNext(
DeleteConfigsResponse.newBuilder()
.addAllDeletedConfigs(
deletedConfigs.stream()
.map(this::buildDeletedContextSpecificConfig)
.collect(Collectors.toUnmodifiableList()))
.build());
responseObserver.onCompleted();
} catch (Exception e) {
log.error("Delete configs failed for request: {}", request, e);
responseObserver.onError(e);
}
}

private ContextSpecificConfig buildDeletedContextSpecificConfig(UpsertedConfig deletedConfig) {
return ContextSpecificConfig.newBuilder()
.setContext(deletedConfig.getContext())
.setCreationTimestamp(deletedConfig.getCreationTimestamp())
.setUpdateTimestamp(deletedConfig.getUpdateTimestamp())
.setConfig(deletedConfig.getPrevConfig())
.build();
}

@Override
public void upsertAllConfigs(
UpsertAllConfigsRequest request, StreamObserver<UpsertAllConfigsResponse> responseObserver) {
Expand Down Expand Up @@ -179,6 +227,15 @@ private ConfigResourceContext getConfigResourceContext(UpsertConfigRequest upser
upsertConfigRequest.getContext());
}

private ConfigResourceContext getConfigResourceContext(DeleteConfigRequest upsertConfigRequest) {
return new ConfigResourceContext(
new ConfigResource(
upsertConfigRequest.getResourceName(),
upsertConfigRequest.getResourceNamespace(),
getTenantId()),
upsertConfigRequest.getContext());
}

private ConfigResourceContext getConfigResourceContext(GetConfigRequest getConfigRequest) {
return new ConfigResourceContext(
new ConfigResource(
Expand All @@ -197,20 +254,18 @@ private ConfigResourceContext getConfigResourceContext(
context);
}

private ConfigResourceContext getConfigResourceContext(DeleteConfigRequest deleteConfigRequest) {
private ConfigResourceContext getConfigResourceContext(ConfigToUpsert configToUpsert) {
return new ConfigResourceContext(
new ConfigResource(
deleteConfigRequest.getResourceName(),
deleteConfigRequest.getResourceNamespace(),
getTenantId()),
deleteConfigRequest.getContext());
configToUpsert.getResourceName(), configToUpsert.getResourceNamespace(), getTenantId()),
configToUpsert.getContext());
}

private ConfigResourceContext getConfigResourceContext(ConfigToUpsert configToUpsert) {
private ConfigResourceContext getConfigResourceContext(ConfigToDelete configToDelete) {
return new ConfigResourceContext(
new ConfigResource(
configToUpsert.getResourceName(), configToUpsert.getResourceNamespace(), getTenantId()),
configToUpsert.getContext());
configToDelete.getResourceName(), configToDelete.getResourceNamespace(), getTenantId()),
configToDelete.getContext());
}

private String getTenantId() {
Expand Down
86 changes: 86 additions & 0 deletions ...g-service-impl/src/test/java/org/hypertrace/config/service/ConfigServiceGrpcImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,14 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import org.hypertrace.config.service.store.ConfigStore;
import org.hypertrace.config.service.v1.ContextSpecificConfig;
import org.hypertrace.config.service.v1.DeleteConfigRequest;
import org.hypertrace.config.service.v1.DeleteConfigResponse;
import org.hypertrace.config.service.v1.DeleteConfigsRequest;
import org.hypertrace.config.service.v1.DeleteConfigsRequest.ConfigToDelete;
import org.hypertrace.config.service.v1.DeleteConfigsResponse;
import org.hypertrace.config.service.v1.GetAllConfigsRequest;
import org.hypertrace.config.service.v1.GetAllConfigsResponse;
import org.hypertrace.config.service.v1.GetConfigRequest;
Expand Down Expand Up @@ -197,6 +201,55 @@ void deleteConfig() throws IOException {
verify(responseObserver, never()).onError(any(Throwable.class));
}

@Test
void deleteConfigs() throws IOException {
String context1 = "context1";
String context2 = "context2";
ConfigStore configStore = mock(ConfigStore.class);
ConfigServiceGrpcImpl configServiceGrpc = new ConfigServiceGrpcImpl(configStore);
StreamObserver<DeleteConfigsResponse> responseObserver = mock(StreamObserver.class);

DeleteConfigsRequest deleteConfigsRequest =
DeleteConfigsRequest.newBuilder()
.addConfigs(getConfigToDelete(context1))
.addConfigs(getConfigToDelete(context2))
.build();

Map<ConfigResourceContext, Value> writeAllConfigsRequest =
Map.of(
getConfigResourceContext(context1),
emptyValue(),
getConfigResourceContext(context2),
emptyValue());

when(configStore.writeAllConfigs(writeAllConfigsRequest, ""))
.thenReturn(
List.of(
buildUpsertedConfig(context1, emptyValue(), config1, 10L, 20L),
buildUpsertedConfig(context2, emptyValue(), config2, 10L, 20L)));
Runnable runnable =
() -> configServiceGrpc.deleteConfigs(deleteConfigsRequest, responseObserver);
RequestContext.forTenantId(TENANT_ID).run(runnable);
verify(configStore, times(1)).writeAllConfigs(eq(writeAllConfigsRequest), eq(""));
verify(responseObserver, times(1))
.onNext(
DeleteConfigsResponse.newBuilder()
.addAllDeletedConfigs(
List.of(
buildContextSpecificConfig(context1, config1, 10L, 20L),
buildContextSpecificConfig(context2, config2, 10L, 20L)))
.build());
verify(responseObserver, times(1)).onCompleted();
verify(responseObserver, never()).onError(any(Throwable.class));

runnable =
() ->
configServiceGrpc.deleteConfigs(
DeleteConfigsRequest.getDefaultInstance(), responseObserver);
RequestContext.forTenantId(TENANT_ID).run(runnable);
verify(responseObserver, times(1)).onError(any(Throwable.class));
}

@Test
void deleteDefaultContextConfig() throws IOException {
ConfigStore configStore = mock(ConfigStore.class);
Expand Down Expand Up @@ -282,4 +335,37 @@ private DeleteConfigRequest getDefaultContextDeleteConfigRequest() {
.setResourceNamespace(RESOURCE_NAMESPACE)
.build();
}

private ConfigToDelete getConfigToDelete(String context) {
return ConfigToDelete.newBuilder()
.setResourceName(RESOURCE_NAME)
.setResourceNamespace(RESOURCE_NAMESPACE)
.setContext(context)
.build();
}

private UpsertedConfig buildUpsertedConfig(
String context,
Value config,
Value prevConfig,
long creationTimestamp,
long updateTimestamp) {
return UpsertedConfig.newBuilder()
.setContext(context)
.setConfig(config)
.setPrevConfig(prevConfig)
.setCreationTimestamp(creationTimestamp)
.setUpdateTimestamp(updateTimestamp)
.build();
}

private ContextSpecificConfig buildContextSpecificConfig(
String context, Value config, long creationTimestamp, long updateTimestamp) {
return ContextSpecificConfig.newBuilder()
.setContext(context)
.setConfig(config)
.setCreationTimestamp(creationTimestamp)
.setUpdateTimestamp(updateTimestamp)
.build();
}
}
Loading

0 comments on commit 018652b

Please sign in to comment.