Merge pull request #2503 from adobe/code-optimizations

afranken · web-flow · commit d73c1058fdf8 · 2025-07-06T21:13:46.000+02:00
Fail PUT object with match on non-existent keys &amp; code optimizations
diff --git a/.gitignore b/.gitignore
@@ -7,3 +7,5 @@ target/
 .settings/
 *.springBeans
 .mvn/wrapper/maven-wrapper.jar
+.vscode
+.cursor
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,7 +8,8 @@ Whenever a 3rd party library is updated, S3Mock will update it's MINOR version.
 * [PLANNED - 5.x - RELEASE TBD](#planned---5x---release-tbd)
   * [Planned changes](#planned-changes)
 * [CURRENT - 4.x - THIS VERSION IS UNDER ACTIVE DEVELOPMENT](#current---4x---this-version-is-under-active-development)
-  * [4.6.0 - PLANNED](#460---planned)
+  * [4.7.0 - PLANNED](#470---planned)
+  * [4.6.0](#460)
   * [4.5.1](#451)
   * [4.5.0](#450)
   * [4.4.0](#440)
@@ -146,7 +147,7 @@ Version 4.x is JDK17 LTS bytecode compatible, with Docker and JUnit / direct Jav
 
 **The current major version 4 will receive new features, dependency updates and bug fixes on a continuous basis.**
 
-## 4.6.0 - PLANNED
+## 4.7.0 - PLANNED
 Version 4.x is JDK17 LTS bytecode compatible, with Docker and JUnit / direct Java integration.
 
 * Features and fixes
@@ -158,6 +159,28 @@ Version 4.x is JDK17 LTS bytecode compatible, with Docker and JUnit / direct Jav
 * Version updates (build dependencies)
   * TBD
 
+## 4.6.0
+Version 4.x is JDK17 LTS bytecode compatible, with Docker and JUnit / direct Java integration.
+
+* Features and fixes
+  * Fail PUT object with match on non-existent keys (fixes #2502)
+* Refactorings
+  * Remove unused imports
+  * Fix Kotlin 2.2 usage
+  * Ignore .vscode and .cursor configurations
+  * Minor refactorings for clarity.
+  * Use fixed list of StorageClass values in tests. New values added by AWS sometimes break tests. We want to make sure to test a few different storage classes, no need to test every one.
+* Version updates (deliverable dependencies)
+  * Bump aws-v2.version from 2.31.67 to 2.31.77
+  * Bump testcontainers.version from 1.21.2 to 1.21.3
+* Version updates (build dependencies)
+  * Bump aws.sdk.kotlin:s3-jvm from 1.4.109 to 1.4.119
+  * Bump org.apache.maven.plugins:maven-gpg-plugin from 3.2.7 to 3.2.8
+  * Bump org.apache.maven.plugins:maven-enforcer-plugin from 3.5.0 to 3.6.0
+  * Bump com.puppycrawl.tools:checkstyle from 10.26.0 to 10.26.1
+  * Bump github/codeql-action from 3.29.1 to 3.29.2
+  * Bump step-security/harden-runner from 2.12.1 to 2.12.2
+
 ## 4.5.1
 Version 4.x is JDK17 LTS bytecode compatible, with Docker and JUnit / direct Java integration.
 
diff --git a/Makefile b/Makefile
@@ -0,0 +1,10 @@
+.PHONY: build verify install
+.DEFAULT_GOAL := build
+
+build: verify
+
+verify:
+	./mvnw -B -V -Dstyle.color=always clean verify
+
+install:
+	./mvnw -B -V -Dstyle.color=always clean install
diff --git a/build-config/pom.xml b/build-config/pom.xml
@@ -21,7 +21,7 @@
   <parent>
     <groupId>com.adobe.testing</groupId>
     <artifactId>s3mock-parent</artifactId>
-    <version>4.5.2-SNAPSHOT</version>
+    <version>4.6.0-SNAPSHOT</version>
   </parent>
 
   <artifactId>s3mock-build-config</artifactId>
diff --git a/docker/pom.xml b/docker/pom.xml
@@ -22,7 +22,7 @@
   <parent>
     <groupId>com.adobe.testing</groupId>
     <artifactId>s3mock-parent</artifactId>
-    <version>4.5.2-SNAPSHOT</version>
+    <version>4.6.0-SNAPSHOT</version>
   </parent>
 
   <artifactId>s3mock-docker</artifactId>
diff --git a/integration-tests/pom.xml b/integration-tests/pom.xml
@@ -22,7 +22,7 @@
   <parent>
     <groupId>com.adobe.testing</groupId>
     <artifactId>s3mock-parent</artifactId>
-    <version>4.5.2-SNAPSHOT</version>
+    <version>4.6.0-SNAPSHOT</version>
   </parent>
 
   <artifactId>s3mock-integration-tests</artifactId>
diff --git a/integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/GetPutDeleteObjectIT.kt b/integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/GetPutDeleteObjectIT.kt
@@ -16,7 +16,6 @@
 
 package com.adobe.testing.s3mock.its
 
-import com.adobe.testing.s3mock.its.S3TestBase.Companion.UPLOAD_FILE
 import com.adobe.testing.s3mock.util.DigestUtil
 import org.assertj.core.api.Assertions.assertThat
 import org.assertj.core.api.Assertions.assertThatThrownBy
@@ -912,6 +911,21 @@ internal class GetPutDeleteObjectIT : S3TestBase() {
       .hasMessageContaining("Service: S3, Status Code: 412")
   }
 
+  @Test
+  @S3VerifiedSuccess(year = 2025)
+  fun `PUT object succeeds with if-none-match on non-existing object`(testInfo: TestInfo) {
+    val nonMatchingEtag = WILDCARD
+    val bucketName = givenBucket(testInfo)
+
+    s3Client.putObject(
+      {
+        it.bucket(bucketName)
+        it.key(UPLOAD_FILE_NAME)
+        it.ifNoneMatch(nonMatchingEtag)
+      }, RequestBody.fromFile(UPLOAD_FILE)
+    )
+  }
+
   @Test
   @S3VerifiedSuccess(year = 2025)
   fun `PUT object fails with if-match=false`(testInfo: TestInfo) {
@@ -930,6 +944,24 @@ internal class GetPutDeleteObjectIT : S3TestBase() {
       .hasMessageContaining("Service: S3, Status Code: 412")
   }
 
+  @Test
+  @S3VerifiedSuccess(year = 2025)
+  fun `PUT object fails with if-match on non-existing object`(testInfo: TestInfo) {
+    val nonMatchingEtag = "\"$randomName\""
+    val bucketName = givenBucket(testInfo)
+
+    assertThatThrownBy {
+      s3Client.putObject(
+        {
+          it.bucket(bucketName)
+          it.key(UPLOAD_FILE_NAME)
+          it.ifMatch(nonMatchingEtag)
+        }, RequestBody.fromFile(UPLOAD_FILE)
+      )
+    }.isInstanceOf(S3Exception::class.java)
+      .hasMessageContaining("Service: S3, Status Code: 404")
+  }
+
   @Test
   @S3VerifiedFailure(year = 2025,
     reason = "Supported only on directory buckets. S3 returns: A header you provided implies functionality that is not implemented.")
diff --git a/integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/S3TestBase.kt b/integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/S3TestBase.kt
@@ -627,16 +627,14 @@ internal abstract class S3TestBase {
 
     @JvmStatic
     protected fun storageClasses(): Stream<StorageClass> {
-      return StorageClass
-        .entries
-        .filter { it != StorageClass.UNKNOWN_TO_SDK_VERSION }
-        .filter { it != StorageClass.SNOW }
-        .filter { it != StorageClass.EXPRESS_ONEZONE }
-        .filter { it != StorageClass.GLACIER }
-        .filter { it != StorageClass.DEEP_ARCHIVE }
-        .filter { it != StorageClass.OUTPOSTS }
-        .map { it }
-        .stream()
+      return listOf(
+        StorageClass.STANDARD,
+        StorageClass.REDUCED_REDUNDANCY,
+        StorageClass.STANDARD_IA,
+        StorageClass.ONEZONE_IA,
+        StorageClass.INTELLIGENT_TIERING,
+        StorageClass.GLACIER,
+      ).stream()
     }
 
     @JvmStatic
diff --git a/integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/S3VerifiedFailure.kt b/integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/S3VerifiedFailure.kt
@@ -1,5 +1,5 @@
 /*
- *  Copyright 2017-2023 Adobe.
+ *  Copyright 2017-2025 Adobe.
  *
  *  Licensed under the Apache License, Version 2.0 (the "License");
  *  you may not use this file except in compliance with the License.
@@ -13,22 +13,15 @@
  *  See the License for the specific language governing permissions and
  *  limitations under the License.
  */
+package com.adobe.testing.s3mock.its
 
-package com.adobe.testing.s3mock.its;
-
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
-import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.extension.ExtendWith
 
 /**
  * A test has been verified as failing against S3 APIs.
  * Some tests can't ever run successfully against the S3 API.
  * The reason will tell us why.
  */
-@Retention(RetentionPolicy.RUNTIME)
-@ExtendWith(RealS3BackendUsedCondition.class)
-public @interface S3VerifiedFailure {
-  String reason();
-
-  int year();
-}
+@Retention(AnnotationRetention.RUNTIME)
+@ExtendWith(RealS3BackendUsedCondition::class)
+annotation class S3VerifiedFailure(val reason: String, val year: Int)
diff --git a/integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/S3VerifiedSuccess.kt b/integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/S3VerifiedSuccess.kt
@@ -1,5 +1,5 @@
 /*
- *  Copyright 2017-2023 Adobe.
+ *  Copyright 2017-2025 Adobe.
  *
  *  Licensed under the Apache License, Version 2.0 (the "License");
  *  you may not use this file except in compliance with the License.
@@ -13,20 +13,13 @@
  *  See the License for the specific language governing permissions and
  *  limitations under the License.
  */
+package com.adobe.testing.s3mock.its
 
-package com.adobe.testing.s3mock.its;
-
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
-import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.extension.ExtendWith
 
 /**
  * A test has been verified successfully against S3 APIs.
  */
-@Retention(RetentionPolicy.RUNTIME)
-@ExtendWith(RealS3BackendUsedCondition.class)
-public @interface S3VerifiedSuccess {
-
-  int year();
-
-}
+@Retention(AnnotationRetention.RUNTIME)
+@ExtendWith(RealS3BackendUsedCondition::class)
+annotation class S3VerifiedSuccess(val year: Int)
diff --git a/integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/S3VerifiedTodo.kt b/integration-tests/src/test/kotlin/com/adobe/testing/s3mock/its/S3VerifiedTodo.kt
@@ -1,5 +1,5 @@
 /*
- *  Copyright 2017-2023 Adobe.
+ *  Copyright 2017-2025 Adobe.
  *
  *  Licensed under the Apache License, Version 2.0 (the "License");
  *  you may not use this file except in compliance with the License.
@@ -13,12 +13,9 @@
  *  See the License for the specific language governing permissions and
  *  limitations under the License.
  */
-
-package com.adobe.testing.s3mock.its;
+package com.adobe.testing.s3mock.its
 
 /**
  * A test is new and needs verification against S3 APIs.
  */
-public @interface S3VerifiedTodo {
-
-}
+annotation class S3VerifiedTodo
diff --git a/pom.xml b/pom.xml
@@ -21,7 +21,7 @@
 
   <groupId>com.adobe.testing</groupId>
   <artifactId>s3mock-parent</artifactId>
-  <version>4.5.2-SNAPSHOT</version>
+  <version>4.6.0-SNAPSHOT</version>
   <packaging>pom</packaging>
 
   <name>S3Mock - Parent</name>
@@ -80,8 +80,8 @@
     <java.version>17</java.version>
     <kotlin.version>2.2.0</kotlin.version>
     <kotlin.compiler.jvmTarget>${java.version}</kotlin.compiler.jvmTarget>
-    <kotlin.compiler.apiVersion>2.1</kotlin.compiler.apiVersion>
-    <kotlin.compiler.languageVersion>2.1</kotlin.compiler.languageVersion>
+    <kotlin.compiler.apiVersion>2.2</kotlin.compiler.apiVersion>
+    <kotlin.compiler.languageVersion>2.2</kotlin.compiler.languageVersion>
 
     <aws-v2.version>2.31.67</aws-v2.version>
 
diff --git a/server/pom.xml b/server/pom.xml
@@ -22,7 +22,7 @@
   <parent>
     <groupId>com.adobe.testing</groupId>
     <artifactId>s3mock-parent</artifactId>
-    <version>4.5.2-SNAPSHOT</version>
+    <version>4.6.0-SNAPSHOT</version>
   </parent>
 
   <artifactId>s3mock</artifactId>
diff --git a/server/src/main/java/com/adobe/testing/s3mock/S3MockProperties.java b/server/src/main/java/com/adobe/testing/s3mock/S3MockProperties.java
@@ -16,7 +16,6 @@
 
 package com.adobe.testing.s3mock;
 
-import com.adobe.testing.s3mock.dto.Region;
 import org.springframework.boot.context.properties.ConfigurationProperties;
 import org.springframework.boot.context.properties.bind.DefaultValue;
 
diff --git a/server/src/main/java/com/adobe/testing/s3mock/service/ObjectService.java b/server/src/main/java/com/adobe/testing/s3mock/service/ObjectService.java
@@ -346,17 +346,24 @@ public void verifyObjectMatching(
       @Nullable List<Instant> ifUnmodifiedSince,
       @Nullable S3ObjectMetadata s3ObjectMetadata) {
     if (s3ObjectMetadata == null) {
-      // object does not exist, so we can skip the rest of the checks.
+      // object does not exist,
+      if (match != null && !match.isEmpty()) {
+        // client expects an existing object to match a value, but it could not be found.
+        throw NO_SUCH_KEY;
+      }
+      // no client expectations, skip the rest of the checks.
       return;
     }
 
     var etag = s3ObjectMetadata.etag();
     var lastModified = Instant.ofEpochMilli(s3ObjectMetadata.lastModified());
 
     var setModifiedSince = ifModifiedSince != null && !ifModifiedSince.isEmpty();
-    if (setModifiedSince && ifModifiedSince.get(0).isAfter(lastModified)) {
-      LOG.debug("Object {} not modified since {}", s3ObjectMetadata.key(), ifModifiedSince.get(0));
-      throw NOT_MODIFIED;
+    if (setModifiedSince) {
+      if (ifModifiedSince.get(0).isAfter(lastModified)) {
+        LOG.debug("Object {} not modified since {}", s3ObjectMetadata.key(), ifModifiedSince.get(0));
+        throw NOT_MODIFIED;
+      }
     }
 
     var setMatch = match != null && !match.isEmpty();
@@ -372,18 +379,20 @@ public void verifyObjectMatching(
     }
 
     var setUnmodifiedSince = ifUnmodifiedSince != null && !ifUnmodifiedSince.isEmpty();
-    if (setUnmodifiedSince && ifUnmodifiedSince.get(0).isBefore(lastModified)) {
-      LOG.debug("Object {} modified since {}", s3ObjectMetadata.key(), ifUnmodifiedSince.get(0));
-      throw PRECONDITION_FAILED;
+    if (setUnmodifiedSince) {
+      if (ifUnmodifiedSince.get(0).isBefore(lastModified)) {
+        LOG.debug("Object {} modified since {}", s3ObjectMetadata.key(), ifUnmodifiedSince.get(0));
+        throw PRECONDITION_FAILED;
+      }
     }
 
     var setNoneMatch = noneMatch != null && !noneMatch.isEmpty();
-    if (setNoneMatch
-        && (noneMatch.contains(WILDCARD_ETAG) || noneMatch.contains(WILDCARD) || noneMatch.contains(etag))
-    ) {
-      //request cares only that the object etag does not match.
-      LOG.debug("Object {} does not exist", s3ObjectMetadata.key());
-      throw NOT_MODIFIED;
+    if (setNoneMatch) {
+      if (noneMatch.contains(WILDCARD_ETAG) || noneMatch.contains(WILDCARD) || noneMatch.contains(etag)) {
+        // request cares only that the object etag does not match.
+        LOG.debug("Object {} has an ETag {} that matches one of the 'noneMatch' values", s3ObjectMetadata.key(), etag);
+        throw NOT_MODIFIED;
+      }
     }
   }
 
diff --git a/server/src/main/java/com/adobe/testing/s3mock/store/StoreConfiguration.java b/server/src/main/java/com/adobe/testing/s3mock/store/StoreConfiguration.java
@@ -97,7 +97,7 @@ BucketStore bucketStore(
           bucketStore.createBucket(name,
               false,
               ObjectOwnership.BUCKET_OWNER_ENFORCED,
-              region.id(),
+              mockRegion.id(),
               null,
               null
           );
diff --git a/server/src/main/java/com/adobe/testing/s3mock/util/DigestUtil.java b/server/src/main/java/com/adobe/testing/s3mock/util/DigestUtil.java
@@ -38,7 +38,6 @@
 import org.apache.commons.codec.binary.Hex;
 import org.apache.commons.codec.digest.DigestUtils;
 import org.apache.commons.lang3.ArrayUtils;
-import org.jspecify.annotations.NonNull;
 import org.jspecify.annotations.Nullable;
 import software.amazon.awssdk.checksums.SdkChecksum;
 import software.amazon.awssdk.checksums.spi.ChecksumAlgorithm;
diff --git a/testsupport/common/pom.xml b/testsupport/common/pom.xml
@@ -22,7 +22,7 @@
   <parent>
     <groupId>com.adobe.testing</groupId>
     <artifactId>s3mock-testsupport-reactor</artifactId>
-    <version>4.5.2-SNAPSHOT</version>
+    <version>4.6.0-SNAPSHOT</version>
   </parent>
 
   <artifactId>s3mock-testsupport-common</artifactId>
diff --git a/testsupport/common/src/main/java/com/adobe/testing/s3mock/testsupport/common/S3MockStarter.java b/testsupport/common/src/main/java/com/adobe/testing/s3mock/testsupport/common/S3MockStarter.java
@@ -39,7 +39,6 @@
 import javax.net.ssl.X509ExtendedTrustManager;
 import org.apache.http.conn.ssl.NoopHostnameVerifier;
 import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
-import org.jspecify.annotations.NonNull;
 import org.jspecify.annotations.Nullable;
 import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
 import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
diff --git a/testsupport/junit4/pom.xml b/testsupport/junit4/pom.xml
diff --git a/testsupport/junit5/pom.xml b/testsupport/junit5/pom.xml
diff --git a/testsupport/pom.xml b/testsupport/pom.xml
diff --git a/testsupport/testcontainers/pom.xml b/testsupport/testcontainers/pom.xml
diff --git a/testsupport/testng/pom.xml b/testsupport/testng/pom.xml
