You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2021/07/22 04:33:49 UTC

[GitHub] [james-project] chibenwa opened a new pull request #546: JAMES-3544 Blob store should allow to list bucket

chibenwa opened a new pull request #546:
URL: https://github.com/apache/james-project/pull/546


   This allow bucket based expiration to be in synchronisation with the underlying data store


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a change in pull request #546: JAMES-3544 Blob store should allow to list bucket

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #546:
URL: https://github.com/apache/james-project/pull/546#discussion_r676290450



##########
File path: server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/BucketNameResolverTest.java
##########
@@ -50,10 +65,45 @@ void resolveShouldReturnValueWhenNamespace() {
             assertThat(resolver.resolve(BucketName.of("namespace")))
                 .isEqualTo(BucketName.of("namespace"));
         }
+
+        @Test
+        void unresolveShouldReturnPassedValue() {

Review comment:
       The parametrized test ensure that we can revert the resolution:
   
   ```
   Whatever (x)
   unesolve(resolve(x)) = x
   
   Mathematically: f-1(f(x)) = x
   ```
   
   This is very different from testing the behaviour of `f-1` alias `unresolve` alone.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa merged pull request #546: JAMES-3544 Blob store should allow to list bucket

Posted by GitBox <gi...@apache.org>.
chibenwa merged pull request #546:
URL: https://github.com/apache/james-project/pull/546


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] Arsnael commented on a change in pull request #546: JAMES-3544 Blob store should allow to list bucket

Posted by GitBox <gi...@apache.org>.
Arsnael commented on a change in pull request #546:
URL: https://github.com/apache/james-project/pull/546#discussion_r676276716



##########
File path: server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/BucketNameResolverTest.java
##########
@@ -50,10 +65,45 @@ void resolveShouldReturnValueWhenNamespace() {
             assertThat(resolver.resolve(BucketName.of("namespace")))
                 .isEqualTo(BucketName.of("namespace"));
         }
+
+        @Test
+        void unresolveShouldReturnPassedValue() {
+            BucketNameResolver resolver = BucketNameResolver.builder()
+                .noPrefix()
+                .namespace(BucketName.of("namespace"))
+                .build();
+
+            assertThat(resolver.unresolve(BucketName.of("bucketName")))
+                .contains(BucketName.of("bucketName"));
+        }
+
+        @Test
+        void unresolveShouldReturnValueWhenNamespace() {

Review comment:
       Isn't that test doing the same as one of the values in the parameterized test above `withShouldAddNewValuesInSet`?

##########
File path: server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/BucketNameResolverTest.java
##########
@@ -50,10 +65,45 @@ void resolveShouldReturnValueWhenNamespace() {
             assertThat(resolver.resolve(BucketName.of("namespace")))
                 .isEqualTo(BucketName.of("namespace"));
         }
+
+        @Test
+        void unresolveShouldReturnPassedValue() {

Review comment:
       Isn't that case checked above in the parameterized test `withShouldAddNewValuesInSet`?

##########
File path: server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/BucketNameResolverTest.java
##########
@@ -50,10 +65,45 @@ void resolveShouldReturnValueWhenNamespace() {
             assertThat(resolver.resolve(BucketName.of("namespace")))
                 .isEqualTo(BucketName.of("namespace"));
         }
+
+        @Test
+        void unresolveShouldReturnPassedValue() {

Review comment:
       In a more general manner, isn't this the same for tests in other nested classes?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a change in pull request #546: JAMES-3544 Blob store should allow to list bucket

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #546:
URL: https://github.com/apache/james-project/pull/546#discussion_r676290450



##########
File path: server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/BucketNameResolverTest.java
##########
@@ -50,10 +65,45 @@ void resolveShouldReturnValueWhenNamespace() {
             assertThat(resolver.resolve(BucketName.of("namespace")))
                 .isEqualTo(BucketName.of("namespace"));
         }
+
+        @Test
+        void unresolveShouldReturnPassedValue() {

Review comment:
       Ok got your point




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a change in pull request #546: JAMES-3544 Blob store should allow to list bucket

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #546:
URL: https://github.com/apache/james-project/pull/546#discussion_r676290337



##########
File path: server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/BucketNameResolverTest.java
##########
@@ -50,10 +65,45 @@ void resolveShouldReturnValueWhenNamespace() {
             assertThat(resolver.resolve(BucketName.of("namespace")))
                 .isEqualTo(BucketName.of("namespace"));
         }
+
+        @Test
+        void unresolveShouldReturnPassedValue() {

Review comment:
       > In a more general manner, isn't this the same for tests in other nested classes?
   
   Yes but un top of different bucket resolver configuration and it (might or might not) yield different results.
   
   Being very sure of the behaviour of that class is very important to me, I assume 100% having a very detailed test suite here.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[GitHub] [james-project] chibenwa commented on a change in pull request #546: JAMES-3544 Blob store should allow to list bucket

Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #546:
URL: https://github.com/apache/james-project/pull/546#discussion_r676292022



##########
File path: server/blob/blob-s3/src/test/java/org/apache/james/blob/objectstorage/aws/BucketNameResolverTest.java
##########
@@ -50,10 +65,45 @@ void resolveShouldReturnValueWhenNamespace() {
             assertThat(resolver.resolve(BucketName.of("namespace")))
                 .isEqualTo(BucketName.of("namespace"));
         }
+
+        @Test
+        void unresolveShouldReturnPassedValue() {

Review comment:
       For the mathematical background https://en.wikipedia.org/wiki/Inverse_function <3




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org