Repository: corona-warn-app/cwa-server
File: services/distribution/src/main/java/app/coronawarn/server/services/distribution/objectstore/client/S3ClientWrapper.java (Java)

Code before:

  private final String dccListPath;

  public S3ClientWrapper(S3Client s3Client) {
    this.s3Client = s3Client;
    this.dccListPath = null;
  }

  public S3ClientWrapper(final S3Client s3Client, final String dccListPath) {
    this.s3Client = s3Client;
    this.dccListPath = dccListPath;
  }

  @Override
  public boolean bucketExists(String bucketName) {
    try {
      // using S3Client.listObjectsV2 instead of S3Client.listBuckets/headBucket in order to limit required permissions
      s3Client.listObjectsV2(ListObjectsV2Request.builder().bucket(bucketName).maxKeys(1).build());
      return true;
    } catch (NoSuchBucketException e) {
      return false;
    } catch (SdkException e) {
      throw new ObjectStoreOperationFailedException("Failed to determine if bucket exists.", e);
    }
  }

  private JsonFile fromResponse(ResponseInputStream<GetObjectResponse> response) {
    return new JsonFile(response, response.response().eTag());
  }

  @Override
  @Retryable(
      value = SdkException.class,
      maxAttemptsExpression = "${services.distribution.objectstore.retry-attempts}",
      backoff = @Backoff(delayExpression = "${services.distribution.objectstore.retry-backoff}"))
  public JsonFile getSingleObjectContent(String bucket, String key) {
    GetObjectRequest request = GetObjectRequest.builder()
        .bucket(bucket)
        .key(key)
        .build();
    var object = s3Client.getObject(request);
    return fromResponse(object);
  }

  @Override
  @Retryable(
      value = SdkException.class,
      exclude = NotModifiedException.class,
      maxAttemptsExpression = "${services.distribution.objectstore.retry-attempts}",
      backoff = @Backoff(delayExpression = "${services.distribution.objectstore.retry-backoff}"))
  public JsonFile getSingleObjectContent(String bucket, String key, String ifNotETag) throws NotModifiedException {

Review comment (refactor):
@hilmarf on this level of abstraction, shouldn't this rather be exposed as `delimiter` instead of `dccListPath`? I'm sure future me will wonder why we need `dccListPath` here on this level. Btw, I would suggest to rename this to something that reflects that it's about _DCC revocation_

Code after:

  private final S3Client s3Client;

  /**
   * Default value is coming from: 'services.distribution.dcc-revocation.dcc-revocation-directory' which is currently:
   * 'dcc-rl'.
   */
  private final String dccListPath;

  /**
   * Standard constructor for reading analytics/statistics data from OBS.
   * 
   * @param s3Client delegator
   */
  public S3ClientWrapper(S3Client s3Client) {
    this.s3Client = s3Client;
    this.dccListPath = null;
  }

  /**
   * Constructor which sets a default {@link Builder#delimiter(String)}.
   * 
   * @param s3Client    delegator
   * @param dccListPath - {@link #dccListPath}.
   */
  public S3ClientWrapper(final S3Client s3Client, final String dccListPath) {
    this.s3Client = s3Client;
    this.dccListPath = dccListPath;
  }

  @Override
  public boolean bucketExists(String bucketName) {
    try {
      // using S3Client.listObjects instead of S3Client.listBuckets/headBucket in order to limit required permissions
      s3Client.listObjects(ListObjectsRequest.builder().bucket(bucketName).maxKeys(1).build());
      return true;
    } catch (NoSuchBucketException e) {
      return false;
    } catch (SdkException e) {
      throw new ObjectStoreOperationFailedException("Failed to determine if bucket exists.", e);
    }
  }

  private JsonFile fromResponse(ResponseInputStream<GetObjectResponse> response) {
    return new JsonFile(response, response.response().eTag());
  }

  @Override
  @Retryable(
      value = SdkException.class,
      maxAttemptsExpression = "${services.distribution.objectstore.retry-attempts}",