Move id converter to router phase 3 (#2895)

Co-authored-by: Sophie Guo <[email protected]>

ambry-account/src/test/java/com/github/ambry/account/MockRouter.java

@@ -16,6 +16,7 @@
 import com.github.ambry.commons.ByteBufferReadableStreamChannel;
 import com.github.ambry.commons.ReadableStreamChannelInputStream;
 import com.github.ambry.config.RouterConfig;
+import com.github.ambry.frontend.IdConverter;
 import com.github.ambry.messageformat.BlobInfo;
 import com.github.ambry.messageformat.BlobProperties;
 import com.github.ambry.commons.Callback;
@@ -176,6 +177,11 @@ public Future<Void> undeleteBlob(String blobId, String serviceId, Callback<Void>
     throw new UnsupportedOperationException("Undelete is currently unsupported");
   }
 
+  @Override
+  public IdConverter getIdConverter() {
+    return null;
+  }
+
   @Override
   public RouterConfig getRouterConfig() {
     return null;

ambry-api/src/main/java/com/github/ambry/frontend/IdConverter.java

@@ -16,8 +16,10 @@
 import com.github.ambry.commons.CallbackUtils;
 import com.github.ambry.messageformat.BlobInfo;
 import com.github.ambry.messageformat.BlobProperties;
+import com.github.ambry.named.NamedBlobDb;
 import com.github.ambry.rest.RestRequest;
 import com.github.ambry.commons.Callback;
+import com.github.ambry.rest.RestServiceException;
 import java.io.Closeable;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Future;
@@ -76,4 +78,10 @@ default Future<String> convert(RestRequest restRequest, String input, BlobProper
   default CompletableFuture<String> convert(RestRequest restRequest, String input, BlobProperties blobProperties) {
     return convert(restRequest, input);
   }
+
+  /**
+   * Get the named blob db from IdConverter.
+   * @return {@link NamedBlobDb}
+   */
+  NamedBlobDb getNamedBlobDb() throws RestServiceException;
 }

ambry-api/src/main/java/com/github/ambry/router/Router.java

@@ -16,6 +16,7 @@
 import com.github.ambry.commons.Callback;
 import com.github.ambry.commons.CallbackUtils;
 import com.github.ambry.config.RouterConfig;
+import com.github.ambry.frontend.IdConverter;
 import com.github.ambry.messageformat.BlobInfo;
 import com.github.ambry.messageformat.BlobProperties;
 import com.github.ambry.quota.QuotaChargeCallback;
@@ -241,6 +242,11 @@ default CompletableFuture<Void> undeleteBlob(String blobId, String serviceId) {
     return future;
   }
 
+  /**
+   * @return the {@link IdConverter} in router.
+   */
+  IdConverter getIdConverter();
+
   /**
    * @return RouterConfig object for the {@link Router}.
    */

ambry-frontend/src/integration-test/java/com/github/ambry/frontend/FrontendQuotaIntegrationTest.java

@@ -184,6 +184,7 @@ private static Properties buildFrontendVProps(File trustStoreFile, boolean enabl
     properties.setProperty("clustermap.host.name", HOST_NAME);
     properties.setProperty("clustermap.port", String.valueOf(PORT));
     properties.setProperty(FrontendConfig.ENABLE_UNDELETE, Boolean.toString(enableUndelete));
+    properties.setProperty(FrontendConfig.NAMED_BLOB_DB_FACTORY, "com.github.ambry.frontend.TestNamedBlobDbFactory");
     return properties;
   }
 

ambry-frontend/src/main/java/com/github/ambry/frontend/AmbryIdConverterFactory.java

@@ -263,7 +263,8 @@ private String signIdIfRequired(RestRequest restRequest, String blobId) throws R
       return metadata != null ? idSigningService.getSignedId(blobId, metadata) : blobId;
     }
 
-    private NamedBlobDb getNamedBlobDb() throws RestServiceException {
+    @Override
+    public NamedBlobDb getNamedBlobDb() throws RestServiceException {
       if (namedBlobDb == null) {
         throw new RestServiceException("Named blob support not enabled", RestServiceErrorCode.BadRequest);
       }

ambry-frontend/src/main/java/com/github/ambry/frontend/FrontendRestRequestService.java

@@ -181,7 +181,7 @@ public void start() throws InstantiationException {
     } catch (Exception e) {
       throw new InstantiationException("FrontendRestRequestService Instantiation failed due to: " + e.getMessage());
     }
-    idConverter = idConverterFactory.getIdConverter();
+    idConverter = router.getIdConverter();
     securityService = securityServiceFactory.getSecurityService();
     getPeersHandler = new GetPeersHandler(clusterMap, securityService, frontendMetrics);
     getSignedUrlHandler =
@@ -270,10 +270,6 @@ public void shutdown() {
         securityService.close();
         securityService = null;
       }
-      if (idConverter != null) {
-        idConverter.close();
-        idConverter = null;
-      }
       if (accountStatsStore != null) {
         accountStatsStore.shutdown();
       }

ambry-frontend/src/main/java/com/github/ambry/frontend/FrontendRestRequestServiceFactory.java

@@ -104,7 +104,7 @@ public RestRequestService getRestRequestService() {
           Utils.getObj(frontendConfig.securityServiceFactory, verifiableProperties, clusterMap, accountService,
               urlSigningService, idSigningService, accountAndContainerInjector, quotaManager);
       return new FrontendRestRequestService(frontendConfig, frontendMetrics, router, clusterMap, idConverterFactory,
-          securityServiceFactory, urlSigningService, idSigningService, namedBlobDb, accountService,
+          securityServiceFactory, urlSigningService, idSigningService, router.getIdConverter().getNamedBlobDb(), accountService,
           accountAndContainerInjector, clusterMapConfig.clusterMapDatacenterName, clusterMapConfig.clusterMapHostName,
           clusterMapConfig.clusterMapClusterName, accountStatsStore, quotaManager);
     } catch (Exception e) {

ambry-frontend/src/test/java/com/github/ambry/frontend/FrontendRestRequestServiceFactoryTest.java

@@ -74,10 +74,11 @@ public void getFrontendRestRequestServiceTest() throws Exception {
     properties.setProperty("clustermap.datacenter.name", "Datacenter-Name");
     properties.setProperty("clustermap.host.name", "localhost");
     VerifiableProperties verifiableProperties = new VerifiableProperties(properties);
+    FrontendTestIdConverterFactory converterFactory = new FrontendTestIdConverterFactory();
 
     FrontendRestRequestServiceFactory frontendRestRequestServiceFactory =
         new FrontendRestRequestServiceFactory(verifiableProperties, new MockClusterMap(),
-            new InMemoryRouter(verifiableProperties, new MockClusterMap()), new InMemAccountService(false, true));
+            new InMemoryRouter(verifiableProperties, new MockClusterMap(), converterFactory), new InMemAccountService(false, true));
     RestRequestService ambryRestRequestService = frontendRestRequestServiceFactory.getRestRequestService();
     assertNotNull("No RestRequestService returned", ambryRestRequestService);
     assertEquals("Did not receive an FrontendRestRequestService instance",
@@ -94,7 +95,8 @@ public void getFrontendRestRequestServiceFactoryWithBadInputTest() throws Except
     Properties properties = new Properties();
     VerifiableProperties verifiableProperties = new VerifiableProperties(properties);
     ClusterMap clusterMap = new MockClusterMap();
-    Router router = new InMemoryRouter(verifiableProperties, clusterMap);
+    FrontendTestIdConverterFactory converterFactory = new FrontendTestIdConverterFactory();
+    Router router = new InMemoryRouter(verifiableProperties, clusterMap, converterFactory);
     AccountService accountService = new InMemAccountService(false, true);
 
     // VerifiableProperties null.

ambry-frontend/src/test/java/com/github/ambry/frontend/FrontendRestRequestServiceTest.java

@@ -164,10 +164,10 @@ public class FrontendRestRequestServiceTest {
   private final Properties configProps = new Properties();
   private final MetricRegistry metricRegistry = new MetricRegistry();
   private final FrontendMetrics frontendMetrics;
-  private final IdConverterFactory idConverterFactory;
+  private IdConverterFactory idConverterFactory;
   private final SecurityServiceFactory securityServiceFactory;
   private final FrontendTestResponseHandler responseHandler;
-  private final InMemoryRouter router;
+  private InMemoryRouter router;
   private final MockClusterMap clusterMap;
   private final AccountStatsStore accountStatsStore;
   private final BlobId referenceBlobId;
@@ -237,7 +237,7 @@ public FrontendRestRequestServiceTest() throws Exception {
       }
     }
     blobIdVersion = CommonTestUtils.getCurrentBlobIdVersion();
-    router = new InMemoryRouter(verifiableProperties, clusterMap);
+    router = new InMemoryRouter(verifiableProperties, clusterMap, idConverterFactory);
     accountStatsStore = mock(AccountStatsStore.class);
     responseHandler = new FrontendTestResponseHandler();
     frontendRestRequestService = getFrontendRestRequestService();
@@ -1830,7 +1830,7 @@ public void oldStyleUserMetadataTest() throws Exception {
    * @throws JSONException
    */
   @Test
-  public void misbehavingIdConverterTest() throws InstantiationException, JSONException {
+  public void misbehavingIdConverterTest() throws Exception {
     FrontendTestIdConverterFactory converterFactory = new FrontendTestIdConverterFactory();
     String exceptionMsg = TestUtils.getRandomString(10);
     converterFactory.exceptionToThrow = new IllegalStateException(exceptionMsg);
@@ -1843,7 +1843,7 @@ public void misbehavingIdConverterTest() throws InstantiationException, JSONExce
    * @throws JSONException
    */
   @Test
-  public void idConverterExceptionPipelineTest() throws InstantiationException, JSONException {
+  public void idConverterExceptionPipelineTest() throws Exception {
     FrontendTestIdConverterFactory converterFactory = new FrontendTestIdConverterFactory();
     String exceptionMsg = TestUtils.getRandomString(10);
     converterFactory.exceptionToReturn = new IllegalStateException(exceptionMsg);
@@ -1856,7 +1856,7 @@ public void idConverterExceptionPipelineTest() throws InstantiationException, JS
    * @throws JSONException
    */
   @Test
-  public void misbehavingSecurityServiceTest() throws InstantiationException, JSONException {
+  public void misbehavingSecurityServiceTest() throws Exception {
     FrontendTestSecurityServiceFactory securityFactory = new FrontendTestSecurityServiceFactory();
     String exceptionMsg = TestUtils.getRandomString(10);
     securityFactory.exceptionToThrow = new IllegalStateException(exceptionMsg);
@@ -1869,7 +1869,7 @@ public void misbehavingSecurityServiceTest() throws InstantiationException, JSON
    * @throws JSONException
    */
   @Test
-  public void securityServiceExceptionPipelineTest() throws InstantiationException, JSONException {
+  public void securityServiceExceptionPipelineTest() throws Exception {
     FrontendTestSecurityServiceFactory securityFactory = new FrontendTestSecurityServiceFactory();
     String exceptionMsg = TestUtils.getRandomString(10);
     securityFactory.exceptionToReturn = new IllegalStateException(exceptionMsg);
@@ -1882,7 +1882,7 @@ public void securityServiceExceptionPipelineTest() throws InstantiationException
    */
   @Test
   public void misbehavingRouterTest() throws Exception {
-    FrontendTestRouter testRouter = new FrontendTestRouter();
+    FrontendTestRouter testRouter = new FrontendTestRouter(idConverterFactory);
     String exceptionMsg = TestUtils.getRandomString(10);
     testRouter.exceptionToThrow = new IllegalStateException(exceptionMsg);
     doRouterExceptionPipelineTest(testRouter, exceptionMsg);
@@ -1895,7 +1895,7 @@ public void misbehavingRouterTest() throws Exception {
    */
   @Test
   public void routerExceptionPipelineTest() throws Exception {
-    FrontendTestRouter testRouter = new FrontendTestRouter();
+    FrontendTestRouter testRouter = new FrontendTestRouter(idConverterFactory);
     String exceptionMsg = TestUtils.getRandomString(10);
     testRouter.exceptionToReturn = new RouterException(exceptionMsg, RouterErrorCode.UnexpectedInternalError);
     doRouterExceptionPipelineTest(testRouter, exceptionMsg + " Error: " + RouterErrorCode.UnexpectedInternalError);
@@ -1928,7 +1928,7 @@ public void badPutRequestWithProhibitedHeadersTest() throws Exception {
    */
   @Test
   public void deleteServiceIdTest() throws Exception {
-    FrontendTestRouter testRouter = new FrontendTestRouter();
+    FrontendTestRouter testRouter = new FrontendTestRouter(idConverterFactory);
     frontendRestRequestService =
         new FrontendRestRequestService(frontendConfig, frontendMetrics, testRouter, clusterMap, idConverterFactory,
             securityServiceFactory, urlSigningService, idSigningService, null, accountService,
@@ -2417,13 +2417,13 @@ public void optionsTest() throws Exception {
    */
   @Test
   public void updateTtlRejectedTest() throws Exception {
-    FrontendTestRouter testRouter = new FrontendTestRouter();
+    FrontendTestRouter testRouter = new FrontendTestRouter(idConverterFactory);
     String exceptionMsg = TestUtils.getRandomString(10);
     testRouter.exceptionToReturn = new RouterException(exceptionMsg, RouterErrorCode.BlobUpdateNotAllowed);
     testRouter.exceptionOpType = FrontendTestRouter.OpType.UpdateBlobTtl;
     frontendRestRequestService =
         new FrontendRestRequestService(frontendConfig, frontendMetrics, testRouter, clusterMap, idConverterFactory,
-            securityServiceFactory, urlSigningService, idSigningService, null, accountService,
+            securityServiceFactory, urlSigningService, idSigningService, namedBlobDb, accountService,
             accountAndContainerInjector, datacenterName, hostname, clusterName, accountStatsStore, QUOTA_MANAGER);
     frontendRestRequestService.setupResponseHandler(responseHandler);
     frontendRestRequestService.start();
@@ -3503,10 +3503,11 @@ private void checkCommonGetHeadHeaders(MockRestResponseChannel restResponseChann
    * @throws JSONException
    */
   private void doIdConverterExceptionTest(FrontendTestIdConverterFactory converterFactory, String expectedExceptionMsg)
-      throws InstantiationException, JSONException {
+      throws InstantiationException, JSONException, RestServiceException {
+    router = new InMemoryRouter(verifiableProperties, clusterMap, converterFactory);
     frontendRestRequestService =
         new FrontendRestRequestService(frontendConfig, frontendMetrics, router, clusterMap, converterFactory,
-            securityServiceFactory, urlSigningService, idSigningService, null, accountService,
+            securityServiceFactory, urlSigningService, idSigningService, router.getIdConverter().getNamedBlobDb(), accountService,
             accountAndContainerInjector, datacenterName, hostname, clusterName, accountStatsStore, QUOTA_MANAGER);
     frontendRestRequestService.setupResponseHandler(responseHandler);
     frontendRestRequestService.start();
@@ -3522,7 +3523,7 @@ private void doIdConverterExceptionTest(FrontendTestIdConverterFactory converter
    * @throws JSONException
    */
   private void doSecurityServiceExceptionTest(FrontendTestSecurityServiceFactory securityFactory, String exceptionMsg)
-      throws InstantiationException, JSONException {
+      throws InstantiationException, JSONException, RestServiceException {
     for (FrontendTestSecurityServiceFactory.Mode mode : FrontendTestSecurityServiceFactory.Mode.values()) {
       securityFactory.mode = mode;
       RestMethod[] restMethods;
@@ -3536,9 +3537,11 @@ private void doSecurityServiceExceptionTest(FrontendTestSecurityServiceFactory s
       } else {
         restMethods = RestMethod.values();
       }
+      Router testRouter = new FrontendTestRouter(idConverterFactory);
       frontendRestRequestService =
-          new FrontendRestRequestService(frontendConfig, frontendMetrics, new FrontendTestRouter(), clusterMap,
-              idConverterFactory, securityFactory, urlSigningService, idSigningService, null, accountService,
+          new FrontendRestRequestService(frontendConfig, frontendMetrics, testRouter, clusterMap,
+              idConverterFactory, securityFactory, urlSigningService, idSigningService, testRouter.getIdConverter()
+              .getNamedBlobDb(), accountService,
               accountAndContainerInjector, datacenterName, hostname, clusterName, accountStatsStore, QUOTA_MANAGER);
       frontendRestRequestService.setupResponseHandler(responseHandler);
       frontendRestRequestService.start();
@@ -4233,6 +4236,11 @@ public Future<String> convert(RestRequest restRequest, String input, BlobPropert
       return completeOperation(input, blobProperties, callback);
     }
 
+    @Override
+    public NamedBlobDb getNamedBlobDb() {
+      return null;
+    }
+
     @Override
     public void close() {
       isOpen = false;
@@ -4386,6 +4394,19 @@ enum OpType {
   String deleteServiceId = null;
   String ttlUpdateServiceId = null;
   String undeleteServiceId = null;
+  IdConverter idConverter;
+
+  public FrontendTestRouter(IdConverterFactory idConverterFactory) {
+    if (idConverterFactory != null) {
+      try {
+        idConverter = idConverterFactory.getIdConverter();
+      } catch (InstantiationException e) {
+        throw new RuntimeException(e);
+      }
+    } else {
+      idConverter = new FrontendTestIdConverterFactory().getIdConverter();
+    }
+  }
 
   @Override
   public Future<GetBlobResult> getBlob(String blobId, GetBlobOptions options, Callback<GetBlobResult> callback,
@@ -4447,9 +4468,21 @@ public RouterConfig getRouterConfig() {
     return null;
   }
 
+  @Override
+  public IdConverter getIdConverter() {
+    return idConverter;
+  }
+
   @Override
   public void close() {
     isOpen = false;
+    if (idConverter != null) {
+      try {
+        idConverter.close();
+      } catch (IOException e) {
+        throw new RuntimeException(e);
+      }
+    }
   }
 
   /**

ambry-frontend/src/test/java/com/github/ambry/frontend/NamedBlobPutHandlerTest.java

@@ -143,7 +143,7 @@ public NamedBlobPutHandlerTest() throws Exception {
     Properties props = new Properties();
     CommonTestUtils.populateRequiredRouterProps(props);
     VerifiableProperties verifiableProperties = new VerifiableProperties(props);
-    router = new InMemoryRouter(verifiableProperties, CLUSTER_MAP);
+    router = new InMemoryRouter(verifiableProperties, CLUSTER_MAP, idConverterFactory);
     FrontendConfig frontendConfig = new FrontendConfig(verifiableProperties);
     metrics = new FrontendMetrics(new MetricRegistry(), frontendConfig);
     injector = new AccountAndContainerInjector(ACCOUNT_SERVICE, metrics, frontendConfig);

ambry-frontend/src/test/java/com/github/ambry/frontend/PostBlobHandlerTest.java

@@ -148,7 +148,7 @@ public PostBlobHandlerTest(boolean isReservedMetadataEnabled) {
     CommonTestUtils.populateRequiredRouterProps(props);
     props.setProperty(RouterConfig.RESERVED_METADATA_ENABLED, Boolean.toString(isReservedMetadataEnabled));
     VerifiableProperties verifiableProperties = new VerifiableProperties(props);
-    router = new InMemoryRouter(verifiableProperties, CLUSTER_MAP);
+    router = new InMemoryRouter(verifiableProperties, CLUSTER_MAP, idConverterFactory);
     FrontendConfig frontendConfig = new FrontendConfig(verifiableProperties);
     metrics = new FrontendMetrics(new MetricRegistry(), frontendConfig);
     injector = new AccountAndContainerInjector(ACCOUNT_SERVICE, metrics, frontendConfig);

ambry-frontend/src/test/java/com/github/ambry/frontend/S3DeleteHandlerTest.java

@@ -95,7 +95,6 @@ private void setup() throws Exception {
     VerifiableProperties verifiableProperties = new VerifiableProperties(properties);
     frontendConfig = new FrontendConfig(verifiableProperties);
     FrontendMetrics metrics = new FrontendMetrics(new MetricRegistry(), frontendConfig);
-    InMemoryRouter router = new InMemoryRouter(verifiableProperties, new MockClusterMap());
     AccountAndContainerInjector injector = new AccountAndContainerInjector(ACCOUNT_SERVICE, metrics, frontendConfig);
     IdSigningService idSigningService = new AmbryIdSigningService();
     AmbrySecurityServiceFactory securityServiceFactory =
@@ -107,6 +106,7 @@ private void setup() throws Exception {
     NamedBlobDb namedBlobDb = namedBlobDbFactory.getNamedBlobDb();
     AmbryIdConverterFactory ambryIdConverterFactory =
         new AmbryIdConverterFactory(verifiableProperties, new MetricRegistry(), idSigningService, namedBlobDb);
+    InMemoryRouter router = new InMemoryRouter(verifiableProperties, new MockClusterMap(), ambryIdConverterFactory);
     namedBlobPutHandler =
         new NamedBlobPutHandler(securityService, namedBlobDb, ambryIdConverterFactory.getIdConverter(),
             idSigningService, router, injector, frontendConfig, metrics, CLUSTER_NAME,