Store user details in config objects in mongo

config-service-impl/src/main/java/org/hypertrace/config/service/ConfigServiceGrpcImpl.java

@@ -39,6 +39,8 @@
 @Slf4j
 public class ConfigServiceGrpcImpl extends ConfigServiceGrpc.ConfigServiceImplBase {
 
+  private static String DEFAULT_USER_ID = "Unknown";
+  private static String DEFAULT_USER_EMAIL = "Unknown";
   private final ConfigStore configStore;
 
   public ConfigServiceGrpcImpl(ConfigStore configStore) {
@@ -51,7 +53,8 @@ public void upsertConfig(
     try {
       ConfigResourceContext configResourceContext = getConfigResourceContext(request);
       UpsertedConfig upsertedConfig =
-          configStore.writeConfig(configResourceContext, getUserId(), request.getConfig());
+          configStore.writeConfig(
+              configResourceContext, getUserId(), request.getConfig(), getUserEmail());
       UpsertConfigResponse.Builder builder = UpsertConfigResponse.newBuilder();
       builder.setConfig(request.getConfig());
       builder.setCreationTimestamp(upsertedConfig.getCreationTimestamp());
@@ -201,7 +204,7 @@ public void upsertAllConfigs(
                           requestedUpsert.getConfig()))
               .collect(ImmutableMap.toImmutableMap(Entry::getKey, Entry::getValue));
       List<UpsertedConfig> upsertedConfigs =
-          configStore.writeAllConfigs(valuesByContext, getUserId());
+          configStore.writeAllConfigs(valuesByContext, getUserId(), getUserEmail());
       responseObserver.onNext(
           UpsertAllConfigsResponse.newBuilder().addAllUpsertedConfigs(upsertedConfigs).build());
       responseObserver.onCompleted();
@@ -270,8 +273,11 @@ private String getTenantId() {
                 ::asRuntimeException);
   }
 
-  // TODO : get the userId from the context
+  private String getUserEmail() {
+    return RequestContext.CURRENT.get().getEmail().orElse(DEFAULT_USER_EMAIL);
+  }
+
   private String getUserId() {
-    return "";
+    return RequestContext.CURRENT.get().getUserId().orElse(DEFAULT_USER_ID);
   }
 }

config-service-impl/src/main/java/org/hypertrace/config/service/store/ConfigDocument.java

@@ -17,6 +17,7 @@
 import com.google.protobuf.Value;
 import com.google.protobuf.util.JsonFormat;
 import java.io.IOException;
+import java.util.Optional;
 import lombok.extern.slf4j.Slf4j;
 import org.hypertrace.config.service.ConfigServiceUtils;
 import org.hypertrace.core.documentstore.Document;
@@ -31,12 +32,15 @@ public class ConfigDocument implements Document {
   private static final ObjectMapper OBJECT_MAPPER =
       new ObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
 
+  public static final String DEFAULT_LATEST_UPDATED_USER_ID = "Unknown";
+  public static final String DEFAULT_LATEST_UPDATED_USER_EMAIL = "Unknown";
   public static final String RESOURCE_FIELD_NAME = "resourceName";
   public static final String RESOURCE_NAMESPACE_FIELD_NAME = "resourceNamespace";
   public static final String TENANT_ID_FIELD_NAME = "tenantId";
   public static final String CONTEXT_FIELD_NAME = "context";
   public static final String VERSION_FIELD_NAME = "configVersion";
-  public static final String USER_ID_FIELD_NAME = "userId";
+  public static final String LAST_UPDATED_USER_ID_FIELD_NAME = "lastUpdateUserId";
+  public static final String LAST_UPDATED_USER_EMAIL_FIELD_NAME = "lastUpdatedUserEmail";
   public static final String CONFIG_FIELD_NAME = "config";
   public static final String CREATION_TIMESTAMP_FIELD_NAME = "creationTimestamp";
   public static final String UPDATE_TIMESTAMP_FIELD_NAME = "updateTimestamp";
@@ -56,8 +60,11 @@ public class ConfigDocument implements Document {
   @JsonProperty(value = VERSION_FIELD_NAME)
   long configVersion;
 
-  @JsonProperty(value = USER_ID_FIELD_NAME)
-  String userId;
+  @JsonProperty(value = LAST_UPDATED_USER_ID_FIELD_NAME)
+  String lastUpdatedUserId;
+
+  @JsonProperty(value = LAST_UPDATED_USER_EMAIL_FIELD_NAME)
+  String lastUpdatedUserEmail;
 
   @JsonSerialize(using = ValueSerializer.class)
   @JsonDeserialize(using = ValueDeserializer.class)
@@ -77,7 +84,8 @@ public ConfigDocument(
       @JsonProperty(TENANT_ID_FIELD_NAME) String tenantId,
       @JsonProperty(CONTEXT_FIELD_NAME) String context,
       @JsonProperty(VERSION_FIELD_NAME) long configVersion,
-      @JsonProperty(USER_ID_FIELD_NAME) String userId,
+      @JsonProperty(LAST_UPDATED_USER_ID_FIELD_NAME) String lastUpdatedUserId,
+      @JsonProperty(LAST_UPDATED_USER_EMAIL_FIELD_NAME) String lastUpdatedUserEmail,
       @JsonProperty(CONFIG_FIELD_NAME) Value config,
       @JsonProperty(CREATION_TIMESTAMP_FIELD_NAME) long creationTimestamp,
       @JsonProperty(UPDATE_TIMESTAMP_FIELD_NAME) long updateTimestamp) {
@@ -86,7 +94,10 @@ public ConfigDocument(
     this.tenantId = tenantId;
     this.context = context;
     this.configVersion = configVersion;
-    this.userId = userId;
+    this.lastUpdatedUserId =
+        Optional.ofNullable(lastUpdatedUserId).orElse(DEFAULT_LATEST_UPDATED_USER_ID);
+    this.lastUpdatedUserEmail =
+        Optional.ofNullable(lastUpdatedUserEmail).orElse(DEFAULT_LATEST_UPDATED_USER_EMAIL);
     this.config = config;
     this.creationTimestamp = creationTimestamp;
     this.updateTimestamp = updateTimestamp;

config-service-impl/src/main/java/org/hypertrace/config/service/store/ConfigStore.java

@@ -20,12 +20,13 @@ public interface ConfigStore {
    * Write the config value associated with the specified config resource to the store.
    *
    * @param configResourceContext
-   * @param userId
    * @param config
+   * @param userEmail
    * @return the config written to the store
    */
   UpsertedConfig writeConfig(
-      ConfigResourceContext configResourceContext, String userId, Value config) throws IOException;
+      ConfigResourceContext configResourceContext, String userId, Value config, String userEmail)
+      throws IOException;
 
   /**
    * Get the config with the latest version for the specified resource.
@@ -63,10 +64,12 @@ Map<ConfigResourceContext, ContextSpecificConfig> getContextConfigs(
    *
    * @param resourceContextValueMap
    * @param userId
+   * @param userEmail
    * @return the upserted configs
    */
   List<UpsertedConfig> writeAllConfigs(
-      Map<ConfigResourceContext, Value> resourceContextValueMap, String userId) throws IOException;
+      Map<ConfigResourceContext, Value> resourceContextValueMap, String userId, String userEmail)
+      throws IOException;
 
   /**
    * delete the config values associated with the specified config resources.

config-service-impl/src/main/java/org/hypertrace/config/service/store/DocumentConfigStore.java

@@ -55,15 +55,23 @@ public DocumentConfigStore(Clock clock, Datastore datastore) {
 
   @Override
   public UpsertedConfig writeConfig(
-      ConfigResourceContext configResourceContext, String userId, Value latestConfig)
+      ConfigResourceContext configResourceContext,
+      String lastUpdatedUserId,
+      Value latestConfig,
+      String lastUpdatedUserEmail)
       throws IOException {
     Optional<ConfigDocument> previousConfigDoc = getLatestVersionConfigDoc(configResourceContext);
     Optional<ContextSpecificConfig> optionalPreviousConfig =
         previousConfigDoc.flatMap(this::convertToContextSpecificConfig);
 
     Key latestDocKey = new ConfigDocumentKey(configResourceContext);
     ConfigDocument latestConfigDocument =
-        buildConfigDocument(configResourceContext, latestConfig, userId, previousConfigDoc);
+        buildConfigDocument(
+            configResourceContext,
+            latestConfig,
+            lastUpdatedUserId,
+            previousConfigDoc,
+            lastUpdatedUserEmail);
 
     collection.upsert(latestDocKey, latestConfigDocument);
     return optionalPreviousConfig
@@ -72,7 +80,8 @@ public UpsertedConfig writeConfig(
   }
 
   private List<UpsertedConfig> writeConfigs(
-      Map<ConfigResourceContext, Value> resourceContextValueMap, String userId) throws IOException {
+      Map<ConfigResourceContext, Value> resourceContextValueMap, String userId, String userEmail)
+      throws IOException {
     Map<ConfigResourceContext, Optional<ConfigDocument>> previousConfigDocs =
         getLatestVersionConfigDocs(resourceContextValueMap.keySet());
     Map<Key, Document> documentsToBeUpserted = new LinkedHashMap<>();
@@ -83,7 +92,7 @@ private List<UpsertedConfig> writeConfigs(
           }
           documentsToBeUpserted.put(
               new ConfigDocumentKey(key),
-              buildConfigDocument(key, resourceContextValueMap.get(key), userId, value));
+              buildConfigDocument(key, resourceContextValueMap.get(key), userId, value, userEmail));
         });
 
     boolean successfulBulkUpsertDocuments = collection.bulkUpsert(documentsToBeUpserted);
@@ -108,8 +117,9 @@ private List<UpsertedConfig> writeConfigs(
   private ConfigDocument buildConfigDocument(
       ConfigResourceContext configResourceContext,
       Value latestConfig,
-      String userId,
-      Optional<ConfigDocument> previousConfigDoc) {
+      String lastUpdatedUserId,
+      Optional<ConfigDocument> previousConfigDoc,
+      String lastUpdatedUserEmail) {
     long updateTimestamp = clock.millis();
     long creationTimestamp =
         previousConfigDoc
@@ -127,16 +137,18 @@ private ConfigDocument buildConfigDocument(
         configResourceContext.getConfigResource().getTenantId(),
         configResourceContext.getContext(),
         newVersion,
-        userId,
+        lastUpdatedUserId,
+        lastUpdatedUserEmail,
         latestConfig,
         creationTimestamp,
         updateTimestamp);
   }
 
   @Override
   public List<UpsertedConfig> writeAllConfigs(
-      Map<ConfigResourceContext, Value> resourceContextValueMap, String userId) throws IOException {
-    return this.writeConfigs(resourceContextValueMap, userId);
+      Map<ConfigResourceContext, Value> resourceContextValueMap, String userId, String userEmail)
+      throws IOException {
+    return this.writeConfigs(resourceContextValueMap, userId, userEmail);
   }
 
   @Override

config-service-impl/src/test/java/org/hypertrace/config/service/ConfigServiceGrpcImplTest.java

@@ -16,6 +16,7 @@
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -51,6 +52,8 @@
 
 class ConfigServiceGrpcImplTest {
 
+  private static final String USER_ID = "userId";
+  private static final String USER_EMAIL = "[email protected]";
   private static Value config1 = getConfig1();
   private static Value config2 = getConfig2();
   private static Value mergedConfig = getExpectedMergedConfig();
@@ -61,7 +64,8 @@ class ConfigServiceGrpcImplTest {
   @Test
   void upsertConfig() throws IOException {
     ConfigStore configStore = mock(ConfigStore.class);
-    when(configStore.writeConfig(any(ConfigResourceContext.class), anyString(), any(Value.class)))
+    when(configStore.writeConfig(
+            any(ConfigResourceContext.class), anyString(), any(Value.class), anyString()))
         .thenAnswer(
             invocation -> {
               Value config = invocation.getArgument(2, Value.class);
@@ -79,14 +83,17 @@ void upsertConfig() throws IOException {
         () -> configServiceGrpc.upsertConfig(getUpsertConfigRequest("", config1), responseObserver);
     Runnable runnableWithoutContext2 =
         () -> configServiceGrpc.upsertConfig(getUpsertConfigRequest("", config2), responseObserver);
-    RequestContext.forTenantId(TENANT_ID).run(runnableWithoutContext1);
-    RequestContext.forTenantId(TENANT_ID).run(runnableWithoutContext2);
+    RequestContext requestContext = spy(RequestContext.forTenantId(TENANT_ID));
+    when(requestContext.getUserId()).thenReturn(Optional.of(USER_ID));
+    when(requestContext.getEmail()).thenReturn(Optional.of(USER_EMAIL));
+    requestContext.run(runnableWithoutContext1);
+    requestContext.run(runnableWithoutContext2);
 
     Runnable runnableWithContext =
         () ->
             configServiceGrpc.upsertConfig(
                 getUpsertConfigRequest(CONTEXT1, config2), responseObserver);
-    RequestContext.forTenantId(TENANT_ID).run(runnableWithContext);
+    requestContext.run(runnableWithContext);
 
     ArgumentCaptor<UpsertConfigResponse> upsertConfigResponseCaptor =
         ArgumentCaptor.forClass(UpsertConfigResponse.class);
@@ -293,7 +300,7 @@ void deletingNonExistingConfigShouldThrowError() throws IOException {
     assertEquals(Status.NOT_FOUND, ((StatusException) throwable).getStatus());
 
     verify(configStore, never())
-        .writeConfig(any(ConfigResourceContext.class), anyString(), any(Value.class));
+        .writeConfig(any(ConfigResourceContext.class), anyString(), any(Value.class), anyString());
     verify(responseObserver, never()).onNext(any(DeleteConfigResponse.class));
     verify(responseObserver, never()).onCompleted();
   }

config-service-impl/src/test/java/org/hypertrace/config/service/store/ConfigDocumentTest.java

@@ -4,6 +4,7 @@
 import static org.hypertrace.config.service.TestUtils.RESOURCE_NAMESPACE;
 import static org.hypertrace.config.service.TestUtils.TENANT_ID;
 import static org.hypertrace.config.service.TestUtils.getConfig1;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import com.google.common.io.Resources;
@@ -27,6 +28,7 @@ void convertToAndFromJson() throws IOException {
             "context",
             15,
             "user1",
+            "[email protected]",
             getConfig1(),
             timestamp,
             timestamp);
@@ -45,6 +47,7 @@ void convertDocumentContainingNullValue() throws IOException {
             "context",
             15,
             "user1",
+            "[email protected]",
             nullValue,
             timestamp,
             timestamp);
@@ -68,4 +71,41 @@ void convertJsonStringWithoutTimestamp() throws IOException {
     assertEquals(0, configDocument.getCreationTimestamp());
     assertEquals(0, configDocument.getUpdateTimestamp());
   }
+
+  @Test
+  @DisplayName("Test compatibility of json string when new field is added to ConfigDocument")
+  void testConfigDocumentCompatibility() throws IOException {
+    ConfigDocument configDocument =
+        new ConfigDocument(
+            "exampleResourceName",
+            "exampleResourceNamespace",
+            "exampleTenantId",
+            "exampleContext",
+            1,
+            null,
+            null,
+            Value.newBuilder().build(),
+            1622547800000L,
+            1625149800000L);
+    assertDoesNotThrow(() -> configDocument.toJson());
+    assertEquals("Unknown", configDocument.getLastUpdatedUserId());
+    assertEquals("Unknown", configDocument.getLastUpdatedUserEmail());
+
+    String jsonString =
+        "{"
+            + "\"resourceName\":\"exampleResourceName\","
+            + "\"userId\":\"userId\","
+            + "\"resourceNamespace\":\"exampleResourceNamespace\","
+            + "\"tenantId\":\"exampleTenantId\","
+            + "\"context\":\"exampleContext\","
+            + "\"configVersion\":1,"
+            + "\"config\":null,"
+            + "\"creationTimestamp\":1622547800000,"
+            + "\"updateTimestamp\":1625149800000}";
+
+    ConfigDocument configDocument1 = ConfigDocument.fromJson(jsonString);
+
+    assertEquals("Unknown", configDocument1.getLastUpdatedUserId());
+    assertEquals("Unknown", configDocument1.getLastUpdatedUserEmail());
+  }
 }

config-service-impl/src/test/java/org/hypertrace/config/service/store/DocumentConfigStoreTest.java

@@ -44,6 +44,7 @@ class DocumentConfigStoreTest {
 
   private static final long CONFIG_VERSION = 5;
   private static final String USER_ID = "user1";
+  private static final String USER_EMAIL = "[email protected]";
   private static final long TIMESTAMP1 = 100L;
   private static final long TIMESTAMP2 = 200L;
   private static final long TIMESTAMP3 = 300L;
@@ -75,7 +76,7 @@ void writeConfig() throws IOException {
     when(collection.search(any(Query.class))).thenReturn(iterator);
 
     UpsertedConfig upsertedConfig =
-        configStore.writeConfig(configResourceContext, USER_ID, config1);
+        configStore.writeConfig(configResourceContext, USER_ID, config1, USER_EMAIL);
     assertEquals(config1, upsertedConfig.getConfig());
     assertEquals(TIMESTAMP1, upsertedConfig.getCreationTimestamp());
 
@@ -191,7 +192,9 @@ void writeAllConfigs() throws IOException {
     List<UpsertedConfig> upsertedConfigs =
         // Swap configs between contexts as an update
         configStore.writeAllConfigs(
-            ImmutableMap.of(resourceContext1, config2, resourceContext2, config1), USER_ID);
+            ImmutableMap.of(resourceContext1, config2, resourceContext2, config1),
+            USER_ID,
+            USER_EMAIL);
 
     assertEquals(
         List.of(
@@ -239,6 +242,7 @@ private static Document getConfigDocument(
         context,
         version,
         USER_ID,
+        USER_EMAIL,
         config,
         creationTimestamp,
         updateTimestamp);