Add tests

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

@@ -64,7 +64,7 @@ default Future<String> convert(RestRequest restRequest, String input, BlobInfo b
   }
 
   /**
-   * Detect if an ID in the request can cause a conflict.
+   * Detect if an ID in the request can cause a conflict during conversion.
    * @param restRequest {@link RestRequest} representing the request.
    * @param callback the {@link Callback} to invoke once the converted ID is available. Can be null.
    * @return a {@link Future} that will eventually contain a boolean indicating whether the ID can cause a conflict.

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

@@ -65,7 +65,7 @@ public IdConverter getIdConverter() {
     return new AmbryIdConverter(idSigningService, namedBlobDb, frontendMetrics);
   }
 
-  private static class AmbryIdConverter implements IdConverter {
+  static class AmbryIdConverter implements IdConverter {
     private boolean isOpen = true;
     private final IdSigningService idSigningService;
     private final NamedBlobDb namedBlobDb;
@@ -113,7 +113,8 @@ public Future<String> convert(RestRequest restRequest, String input, BlobInfo bl
           convertedId = "/" + signIdIfRequired(restRequest, input);
         } else {
           CallbackUtils.callCallbackAfter(convertId(input, restRequest, blobInfo),
-              (id, e) -> completeConverterOperation(id, e, future, callback, frontendMetrics.idConversionDownstreamCallbackTimeInMs));
+              (id, e) -> completeConverterOperation(id, e, future, callback,
+                  frontendMetrics.idConversionDownstreamCallbackTimeInMs));
         }
       } catch (Exception e) {
         exception = e;
@@ -186,22 +187,22 @@ private CompletionStage<Boolean> detectConflict(RestRequest restRequest) throws
 
     /**
      * Completes the operation of this converter by setting the future and invoking the callback.
-     * @param results the conversion result.
+     * @param result the conversion result.
      * @param exception any exception that occurred as a part of the conversion.
      * @param completableFuture the {@link CompletableFuture} that must be set.
      * @param callback the {@link Callback} that needs to be invoked. Can be null.
      * @param downstreamCallbackTimeInMs {@link Histogram} to update the time taken for downstream callback.
      */
-    private <T> void completeConverterOperation(T results, Exception exception, CompletableFuture<T> completableFuture,
+    private <T> void completeConverterOperation(T result, Exception exception, CompletableFuture<T> completableFuture,
         Callback<T> callback, Histogram downstreamCallbackTimeInMs) {
       if (exception == null) {
-        completableFuture.complete(results);
+        completableFuture.complete(result);
       } else {
         completableFuture.completeExceptionally(exception);
       }
       if (callback != null) {
         long startTime = System.currentTimeMillis();
-        callback.onCompletion(results, exception);
+        callback.onCompletion(result, exception);
         downstreamCallbackTimeInMs.update(System.currentTimeMillis() - startTime);
       }
     }

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

@@ -208,21 +208,12 @@ private Callback<Void> securityProcessRequestCallback() {
      */
     private Callback<Void> securityPostProcessRequestCallback(BlobInfo blobInfo) {
       return buildCallback(frontendMetrics.putSecurityPostProcessRequestMetrics, securityCheckResult -> {
-        if (RestUtils.isNamedBlobStitchRequest(restRequest)) {
-          if (RestUtils.isDatasetVersionQueryEnabled(restRequest.getArgs())) {
-            accountAndContainerInjector.injectDatasetForNamedBlob(restRequest);
-            frontendMetrics.addDatasetVersionRate.mark();
-            addDatasetVersion(blobInfo.getBlobProperties(), restRequest);
-          }
-          idConverter.detectConflict(restRequest, detectConflictCallback(blobInfo));
-        } else {
-          if (RestUtils.isDatasetVersionQueryEnabled(restRequest.getArgs())) {
-            accountAndContainerInjector.injectDatasetForNamedBlob(restRequest);
-            frontendMetrics.addDatasetVersionRate.mark();
-            addDatasetVersion(blobInfo.getBlobProperties(), restRequest);
-          }
-          idConverter.detectConflict(restRequest, detectConflictCallback(blobInfo));
+        if (RestUtils.isDatasetVersionQueryEnabled(restRequest.getArgs())) {
+          accountAndContainerInjector.injectDatasetForNamedBlob(restRequest);
+          frontendMetrics.addDatasetVersionRate.mark();
+          addDatasetVersion(blobInfo.getBlobProperties(), restRequest);
         }
+        idConverter.detectConflict(restRequest, detectConflictCallback(blobInfo));
       }, uri, LOGGER, deleteDatasetCallback);
     }
 
@@ -273,8 +264,8 @@ private Callback<Long> fetchStitchRequestBodyCallback(RetainingAsyncWritableChan
       return buildCallback(frontendMetrics.putReadStitchRequestMetrics,
           bytesRead -> {
             router.stitchBlob(getPropertiesForRouterUpload(blobInfo), blobInfo.getUserMetadata(),
-                getChunksToStitch(blobInfo.getBlobProperties(), readJsonFromChannel(channel)), routerStitchBlobCallback(blobInfo),
-                QuotaUtils.buildQuotaChargeCallback(restRequest, quotaManager, true));
+                getChunksToStitch(blobInfo.getBlobProperties(), readJsonFromChannel(channel)),
+                routerStitchBlobCallback(blobInfo), QuotaUtils.buildQuotaChargeCallback(restRequest, quotaManager, true));
           },
           uri, LOGGER, deleteDatasetCallback);
     }

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

@@ -1757,7 +1757,8 @@ public void oldStyleUserMetadataTest() throws Exception {
    */
   @Test
   public void misbehavingIdConverterTest() throws InstantiationException, JSONException {
-    FrontendTestIdConverterFactory converterFactory = new FrontendTestIdConverterFactory();
+    FrontendTestIdConverterFactory converterFactory =
+        new FrontendTestIdConverterFactory(verifiableProperties, metricRegistry, namedBlobDb, idSigningService);
     String exceptionMsg = TestUtils.getRandomString(10);
     converterFactory.exceptionToThrow = new IllegalStateException(exceptionMsg);
     doIdConverterExceptionTest(converterFactory, exceptionMsg);
@@ -1770,7 +1771,8 @@ public void misbehavingIdConverterTest() throws InstantiationException, JSONExce
    */
   @Test
   public void idConverterExceptionPipelineTest() throws InstantiationException, JSONException {
-    FrontendTestIdConverterFactory converterFactory = new FrontendTestIdConverterFactory();
+    FrontendTestIdConverterFactory converterFactory =
+        new FrontendTestIdConverterFactory(verifiableProperties, metricRegistry, namedBlobDb, idSigningService);
     String exceptionMsg = TestUtils.getRandomString(10);
     converterFactory.exceptionToReturn = new IllegalStateException(exceptionMsg);
     doIdConverterExceptionTest(converterFactory, exceptionMsg);
@@ -2592,13 +2594,39 @@ static RestRequest createRestRequest(RestMethod restMethod, String uri, JSONObje
   static void setAmbryHeadersForPut(JSONObject headers, long ttlInSecs, boolean isPrivate, String serviceId,
       String contentType, String ownerId, String targetAccountName, String targetContainerName,
       String uploadNamedBlobMode) throws JSONException {
+    setAmbryHeadersForPut(headers, ttlInSecs, isPrivate, serviceId, contentType, ownerId, targetAccountName,
+        targetContainerName, uploadNamedBlobMode, true);
+  }
+
+  /**
+   * Sets headers that helps build {@link BlobProperties} on the server. See argument list for the headers that are set.
+   * Any other headers have to be set explicitly.
+   * @param headers the {@link JSONObject} where the headers should be set.
+   * @param ttlInSecs sets the {@link RestUtils.Headers#TTL} header. Set to {@link Utils#Infinite_Time} if no
+   *                  expiry.
+   * @param isPrivate sets the {@link RestUtils.Headers#PRIVATE} header. Allowed values: true, false.
+   * @param serviceId sets the {@link RestUtils.Headers#SERVICE_ID} header. Required.
+   * @param contentType sets the {@link RestUtils.Headers#AMBRY_CONTENT_TYPE} header. Required and has to be a valid MIME
+   *                    type.
+   * @param ownerId sets the {@link RestUtils.Headers#OWNER_ID} header. Optional - if not required, send null.
+   * @param targetAccountName sets the {@link RestUtils.Headers#TARGET_ACCOUNT_NAME} header. Can be {@code null}.
+   * @param targetContainerName sets the {@link RestUtils.Headers#TARGET_CONTAINER_NAME} header. Can be {@code null}.
+   * @param uploadNamedBlobMode
+   * @param namedUpsert boolean to indicate if the named blob upload is an upsert or not.
+   * @throws IllegalArgumentException if any of {@code headers}, {@code serviceId}, {@code contentType} is null or if
+   *                                  {@code contentLength} < 0 or if {@code ttlInSecs} < -1.
+   * @throws JSONException
+   */
+  static void setAmbryHeadersForPut(JSONObject headers, long ttlInSecs, boolean isPrivate, String serviceId,
+      String contentType, String ownerId, String targetAccountName, String targetContainerName,
+      String uploadNamedBlobMode, boolean namedUpsert) throws JSONException {
     if (headers != null && serviceId != null && contentType != null) {
       if (ttlInSecs != Utils.Infinite_Time) {
         headers.put(RestUtils.Headers.TTL, Long.toString(ttlInSecs));
       }
       headers.put(RestUtils.Headers.SERVICE_ID, serviceId);
       headers.put(RestUtils.Headers.AMBRY_CONTENT_TYPE, contentType);
-      headers.put(RestUtils.Headers.NAMED_UPSERT, true);
+      headers.put(RestUtils.Headers.NAMED_UPSERT, namedUpsert);
       if (targetAccountName != null) {
         headers.put(RestUtils.Headers.TARGET_ACCOUNT_NAME, targetAccountName);
       }
@@ -4125,23 +4153,47 @@ private void completeOperation(Callback<Void> callback, boolean misbehaveIfRequi
 /**
  * Implementation of {@link IdConverterFactory} that returns exceptions.
  */
-class FrontendTestIdConverterFactory implements IdConverterFactory {
+class FrontendTestIdConverterFactory extends AmbryIdConverterFactory {
+  private final NamedBlobDb namedBlobDb;
   Exception exceptionToReturn = null;
   RuntimeException exceptionToThrow = null;
   String translation = null;
   boolean returnInputIfTranslationNull = false;
-  boolean mappingExists = false;
+  boolean enableConflictDetection = false;
+  boolean passThrough = false;
   volatile String lastInput = null;
   volatile BlobInfo lastBlobInfo = null;
   volatile String lastConvertedId = null;
 
+  /**
+   * Constructor for {@link FrontendTestIdConverterFactory}
+   * @param verifiableProperties {@link VerifiableProperties} object.
+   * @param metricRegistry {@link MetricRegistry} object.
+   * @param namedBlobDb {@link NamedBlobDb} object.
+   * @param idSigningService {@link IdSigningService} object.
+   */
+  public FrontendTestIdConverterFactory(VerifiableProperties verifiableProperties, MetricRegistry metricRegistry,
+      NamedBlobDb namedBlobDb, IdSigningService idSigningService) {
+    super(verifiableProperties, metricRegistry, idSigningService, namedBlobDb);
+    this.namedBlobDb = namedBlobDb;
+  }
+
   @Override
   public IdConverter getIdConverter() {
-    return new TestIdConverter();
+      return new TestIdConverter((AmbryIdConverter) super.getIdConverter());
   }
 
   private class TestIdConverter implements IdConverter {
     private boolean isOpen = true;
+    private final AmbryIdConverter ambryIdConverter;
+
+    /**
+     * TestIdConverter constructor.
+     * @param ambryIdConverter AmbryIdConverter object to use for pass through.
+     */
+    public TestIdConverter(AmbryIdConverter ambryIdConverter) {
+      this.ambryIdConverter = ambryIdConverter;
+    }
 
     @Override
     public Future<String> convert(RestRequest restRequest, String input, Callback<String> callback) {
@@ -4153,8 +4205,16 @@ public Future<String> convert(RestRequest restRequest, String input, BlobInfo bl
       if (!isOpen) {
         throw new IllegalStateException("IdConverter closed");
       }
-      if (restRequest.getRestMethod() == RestMethod.PUT && RestUtils.getRequestPath(restRequest)
-          .matchesOperation(Operations.NAMED_BLOB)) {
+      if (passThrough) {
+        return ambryIdConverter.convert(restRequest, input, blobInfo, callback);
+      }
+      if (restRequest.getRestMethod() == RestMethod.PUT && RestUtils.getRequestPath(restRequest).matchesOperation(Operations.NAMED_BLOB)) {
+        /*try {
+          namedBlobDb.put(new NamedBlobRecord(RestUtils.getAccountFromArgs(restRequest.getArgs()).getName(),
+              RestUtils.getContainerFromArgs(restRequest.getArgs()).getName(), restRequest.getUri(), input, Utils.Infinite_Time, 0));
+        } catch (Exception ex) {
+          exceptionToThrow = new RuntimeException(ex);
+        }*/
         restRequest.setArg(RestUtils.InternalKeys.NAMED_BLOB_VERSION, -1L);
       }
       return completeOperation(input, blobInfo, callback);
@@ -4165,10 +4225,15 @@ public Future<Boolean> detectConflict(RestRequest restRequest, Callback<Boolean>
       if (!isOpen) {
         throw new IllegalStateException("IdConverter closed");
       }
-      try {
+      if (passThrough) {
+        return ambryIdConverter.detectConflict(restRequest, callback);
+      }
+      /* try {
         if (restRequest.getRestMethod() == RestMethod.PUT && RestUtils.getRequestPath(restRequest)
             .matchesOperation(Operations.NAMED_BLOB) && !RestUtils.isUpsertForNamedBlob(restRequest.getArgs())) {
-          if (mappingExists) {
+          if (enableConflictDetection) {
+            namedBlobDb.get(RestUtils.getAccountFromArgs(restRequest.getArgs()).getName(),
+                RestUtils.getContainerFromArgs(restRequest.getArgs()).getName(), restRequest.getUri());
             return completeOperation(true, callback);
           } else {
             return completeOperation(false, callback);
@@ -4178,13 +4243,16 @@ public Future<Boolean> detectConflict(RestRequest restRequest, Callback<Boolean>
         if (exceptionToReturn == null) {
           exceptionToReturn = e;
         }
-      }
-      return completeOperation(false, callback);
+      }*/
+      return completeOperation(false, null, null);
     }
 
     @Override
     public void close() {
       isOpen = false;
+      if (passThrough) {
+        ambryIdConverter.close();
+      }
     }
 
     /**

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

@@ -33,6 +33,7 @@
 import com.github.ambry.rest.RestServiceErrorCode;
 import com.github.ambry.rest.RestServiceException;
 import com.github.ambry.rest.RestUtils;
+import com.github.ambry.utils.SystemTime;
 import com.github.ambry.utils.TestUtils;
 import java.io.IOException;
 import java.util.Collections;
@@ -69,7 +70,9 @@ public class GetSignedUrlHandlerTest {
   private final FrontendTestSecurityServiceFactory securityServiceFactory = new FrontendTestSecurityServiceFactory();
   private final FrontendTestUrlSigningServiceFactory urlSigningServiceFactory =
       new FrontendTestUrlSigningServiceFactory();
-  private final FrontendTestIdConverterFactory idConverterFactory = new FrontendTestIdConverterFactory();
+  private final FrontendTestIdConverterFactory idConverterFactory = new FrontendTestIdConverterFactory(
+      new VerifiableProperties(new Properties()), new MetricRegistry(), new TestNamedBlobDb(SystemTime.getInstance(),
+      1000), new AmbryIdSigningService());
   private final GetSignedUrlHandler getSignedUrlHandler;
 
   public GetSignedUrlHandlerTest() {