You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "mbaechler (via GitHub)" <gi...@apache.org> on 2023/10/14 09:17:00 UTC
[PR] Introduce a ResolvedBucketName type to avoid temporal coupling (and e… [james-project]
mbaechler opened a new pull request, #1754:
URL: https://github.com/apache/james-project/pull/1754
…nhance readability)
--
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
Re: [PR] Introduce a ResolvedBucketName type to avoid temporal coupling (and e… [james-project]
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on PR #1754:
URL: https://github.com/apache/james-project/pull/1754#issuecomment-1763681152
```
09:19:04,821 [ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.3.0:check (check-style) on project blob-api: You have 2 Checkstyle violations. -> [Help 1]
```
--
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
Re: [PR] Introduce a ResolvedBucketName type to avoid temporal coupling (and e… [james-project]
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on PR #1754:
URL: https://github.com/apache/james-project/pull/1754#issuecomment-1794084613
@mbaechler status on this? any plan on fixing soon checkstyle issues and answering to comments? :)
--
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
Re: [PR] Introduce a ResolvedBucketName type to avoid temporal coupling (and e… [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1754:
URL: https://github.com/apache/james-project/pull/1754#discussion_r1371198555
##########
server/blob/blob-api/src/main/java/org/apache/james/blob/api/ResolvedBucketName.java:
##########
@@ -0,0 +1,66 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.api;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.Objects;
+
+public final class ResolvedBucketName {
Review Comment:
>Hi @chibenwa ,
> Do you have a plan to migrate to Java 14 (or 16), this will prevent from lot of code like this one.
`You` is out of place, as a community we should embrasse a `we`.
I would personnalyy support a switch to Java 17 or even 21.
Do you think you can pull off a mail on this topic on the mailing lists?
--
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
Re: [PR] Introduce a ResolvedBucketName type to avoid temporal coupling (and e… [james-project]
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on PR #1754:
URL: https://github.com/apache/james-project/pull/1754#issuecomment-1829000693
https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1754/4/testReport/
I think the errors in BucketNameResolverTest are related
--
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
Re: [PR] Introduce a ResolvedBucketName type to avoid temporal coupling (and e… [james-project]
Posted by "jeantil (via GitHub)" <gi...@apache.org>.
jeantil commented on code in PR #1754:
URL: https://github.com/apache/james-project/pull/1754#discussion_r1359402997
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/BucketNameResolver.java:
##########
@@ -84,33 +85,39 @@ private BucketNameResolver(Optional<BucketName> namespace, Optional<String> pref
this.prefix = prefix;
}
- BucketName resolve(BucketName bucketName) {
+ ResolvedBucketName resolve(BucketName bucketName) {
Preconditions.checkNotNull(bucketName);
if (isNameSpace(bucketName)) {
- return bucketName;
+ return ResolvedBucketName.of(bucketName.asString());
}
return prefix
- .map(bucketPrefix -> BucketName.of(bucketPrefix + bucketName.asString()))
- .orElse(bucketName);
+ .map(bucketPrefix -> ResolvedBucketName.of(bucketPrefix + bucketName.asString()))
+ .orElse(ResolvedBucketName.of(bucketName.asString()));
}
- Optional<BucketName> unresolve(BucketName bucketName) {
+ Optional<BucketName> unresolve(ResolvedBucketName bucketName) {
if (isNameSpace(bucketName)) {
- return Optional.of(bucketName);
+ return Optional.of(BucketName.of(bucketName.asString()));
}
return prefix.map(p -> {
if (bucketName.asString().startsWith(p)) {
return Optional.of(BucketName.of(bucketName.asString().substring(p.length())));
}
return Optional.<BucketName>empty();
- }).orElse(Optional.of(bucketName));
+ }).orElse(Optional.of(BucketName.of(bucketName.asString())));
}
private boolean isNameSpace(BucketName bucketName) {
return namespace
- .map(existingNamespace -> existingNamespace.equals(bucketName))
+ .map(existingNamespace -> existingNamespace.asString().equals(bucketName.asString()))
Review Comment:
**💭 thought:** the namespace term is also called defaultBucketName in other places ( it is called namespace in the configuration file, the configuration builder accepts a namespace setter but when building the actual configuration object it injects the namespace into a field named `defaultBucketName` a similarly named field is also exposed by the S3 dao but in the bucketname resolver it is again called namespace. It is unclear to me what this concept is supposed to represent and how it is different from bucketPrefix (whose name is fairly stable throughout the code)
I don't expect an action on this MR but seeing the name here reminded me of this thought.
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreDAO.java:
##########
@@ -420,7 +418,7 @@ public Publisher<BucketName> listBuckets() {
return Mono.fromFuture(client::listBuckets)
.flatMapIterable(ListBucketsResponse::buckets)
.map(Bucket::name)
- .handle((bucket, sink) -> bucketNameResolver.unresolve(BucketName.of(bucket))
+ .handle((bucket, sink) -> bucketNameResolver.unresolve(ResolvedBucketName.of(bucket))
Review Comment:
**❔ question:** same as above, I realize this was the case before this MR but is it normal that we don't go through
`ResolvedBucketName resolvedBucketName = bucketNameResolver.resolve(bucketName);` here whil we do for all other public calls ? I don't necessarily expect this to be fixed in this MR, just trying to understand if there is something that needs fixing at all :D
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreDAO.java:
##########
@@ -411,7 +409,7 @@ public Mono<Void> deleteAllBuckets() {
return Mono.fromFuture(client::listBuckets)
.publishOn(Schedulers.parallel())
.flatMapIterable(ListBucketsResponse::buckets)
- .flatMap(bucket -> deleteResolvedBucket(BucketName.of(bucket.name())), DEFAULT_CONCURRENCY)
+ .flatMap(bucket -> deleteResolvedBucket(ResolvedBucketName.of(bucket.name())), DEFAULT_CONCURRENCY)
Review Comment:
**❔ question:** I realize this was the case before this MR but is it normal that we don't go through
`ResolvedBucketName resolvedBucketName = bucketNameResolver.resolve(bucketName);` here whil we do for all other public calls ? I don't necessarily expect this to be fixed in this MR, just trying to understand if there is something that needs fixing at all :D
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreDAO.java:
##########
@@ -420,7 +418,7 @@ public Publisher<BucketName> listBuckets() {
return Mono.fromFuture(client::listBuckets)
.flatMapIterable(ListBucketsResponse::buckets)
.map(Bucket::name)
- .handle((bucket, sink) -> bucketNameResolver.unresolve(BucketName.of(bucket))
+ .handle((bucket, sink) -> bucketNameResolver.unresolve(ResolvedBucketName.of(bucket))
Review Comment:
**❔ question:** same as above, I realize this was the case before this MR but is it normal that we don't go through
`ResolvedBucketName resolvedBucketName = bucketNameResolver.resolve(bucketName);` here whil we do for all other public calls ? I don't necessarily expect this to be fixed in this MR, just trying to understand if there is something that needs fixing at all :D
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/BucketNameResolver.java:
##########
@@ -84,33 +85,39 @@ private BucketNameResolver(Optional<BucketName> namespace, Optional<String> pref
this.prefix = prefix;
}
- BucketName resolve(BucketName bucketName) {
+ ResolvedBucketName resolve(BucketName bucketName) {
Preconditions.checkNotNull(bucketName);
if (isNameSpace(bucketName)) {
- return bucketName;
+ return ResolvedBucketName.of(bucketName.asString());
}
return prefix
- .map(bucketPrefix -> BucketName.of(bucketPrefix + bucketName.asString()))
- .orElse(bucketName);
+ .map(bucketPrefix -> ResolvedBucketName.of(bucketPrefix + bucketName.asString()))
+ .orElse(ResolvedBucketName.of(bucketName.asString()));
}
- Optional<BucketName> unresolve(BucketName bucketName) {
+ Optional<BucketName> unresolve(ResolvedBucketName bucketName) {
if (isNameSpace(bucketName)) {
- return Optional.of(bucketName);
+ return Optional.of(BucketName.of(bucketName.asString()));
}
return prefix.map(p -> {
if (bucketName.asString().startsWith(p)) {
return Optional.of(BucketName.of(bucketName.asString().substring(p.length())));
}
return Optional.<BucketName>empty();
- }).orElse(Optional.of(bucketName));
+ }).orElse(Optional.of(BucketName.of(bucketName.asString())));
}
private boolean isNameSpace(BucketName bucketName) {
return namespace
- .map(existingNamespace -> existingNamespace.equals(bucketName))
+ .map(existingNamespace -> existingNamespace.asString().equals(bucketName.asString()))
Review Comment:
**❔ question:** why change the equality from `BucketName#equals` to `asString().equals` I agree that the result is identical but don't think this change makes the code clearer
I understand why it is necessary in the overload of `isNameSpace` that compares a `ResolvedBucketName` and a `BucketName`.
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/BucketNameResolver.java:
##########
@@ -84,33 +85,39 @@ private BucketNameResolver(Optional<BucketName> namespace, Optional<String> pref
this.prefix = prefix;
}
- BucketName resolve(BucketName bucketName) {
+ ResolvedBucketName resolve(BucketName bucketName) {
Preconditions.checkNotNull(bucketName);
if (isNameSpace(bucketName)) {
- return bucketName;
+ return ResolvedBucketName.of(bucketName.asString());
}
return prefix
- .map(bucketPrefix -> BucketName.of(bucketPrefix + bucketName.asString()))
- .orElse(bucketName);
+ .map(bucketPrefix -> ResolvedBucketName.of(bucketPrefix + bucketName.asString()))
+ .orElse(ResolvedBucketName.of(bucketName.asString()));
}
- Optional<BucketName> unresolve(BucketName bucketName) {
+ Optional<BucketName> unresolve(ResolvedBucketName bucketName) {
if (isNameSpace(bucketName)) {
- return Optional.of(bucketName);
+ return Optional.of(BucketName.of(bucketName.asString()));
}
return prefix.map(p -> {
if (bucketName.asString().startsWith(p)) {
return Optional.of(BucketName.of(bucketName.asString().substring(p.length())));
}
return Optional.<BucketName>empty();
- }).orElse(Optional.of(bucketName));
+ }).orElse(Optional.of(BucketName.of(bucketName.asString())));
}
private boolean isNameSpace(BucketName bucketName) {
return namespace
- .map(existingNamespace -> existingNamespace.equals(bucketName))
+ .map(existingNamespace -> existingNamespace.asString().equals(bucketName.asString()))
Review Comment:
**❔ question:** why change the equality from `BucketName#equals` to `asString().equals` I agree that the result is identical but don't think this change makes the code clearer
I understand why it is necessary in the overload of `isNameSpace` that compares a `ResolvedBucketName` and a `BucketName`.
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreDAO.java:
##########
@@ -411,7 +409,7 @@ public Mono<Void> deleteAllBuckets() {
return Mono.fromFuture(client::listBuckets)
.publishOn(Schedulers.parallel())
.flatMapIterable(ListBucketsResponse::buckets)
- .flatMap(bucket -> deleteResolvedBucket(BucketName.of(bucket.name())), DEFAULT_CONCURRENCY)
+ .flatMap(bucket -> deleteResolvedBucket(ResolvedBucketName.of(bucket.name())), DEFAULT_CONCURRENCY)
Review Comment:
**❔ question:** I realize this was the case before this MR but is it normal that we don't go through
`ResolvedBucketName resolvedBucketName = bucketNameResolver.resolve(bucketName);` here whil we do for all other public calls ? I don't necessarily expect this to be fixed in this MR, just trying to understand if there is something that needs fixing at all :D
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/BucketNameResolver.java:
##########
@@ -84,33 +85,39 @@ private BucketNameResolver(Optional<BucketName> namespace, Optional<String> pref
this.prefix = prefix;
}
- BucketName resolve(BucketName bucketName) {
+ ResolvedBucketName resolve(BucketName bucketName) {
Preconditions.checkNotNull(bucketName);
if (isNameSpace(bucketName)) {
- return bucketName;
+ return ResolvedBucketName.of(bucketName.asString());
}
return prefix
- .map(bucketPrefix -> BucketName.of(bucketPrefix + bucketName.asString()))
- .orElse(bucketName);
+ .map(bucketPrefix -> ResolvedBucketName.of(bucketPrefix + bucketName.asString()))
+ .orElse(ResolvedBucketName.of(bucketName.asString()));
}
- Optional<BucketName> unresolve(BucketName bucketName) {
+ Optional<BucketName> unresolve(ResolvedBucketName bucketName) {
if (isNameSpace(bucketName)) {
- return Optional.of(bucketName);
+ return Optional.of(BucketName.of(bucketName.asString()));
}
return prefix.map(p -> {
if (bucketName.asString().startsWith(p)) {
return Optional.of(BucketName.of(bucketName.asString().substring(p.length())));
}
return Optional.<BucketName>empty();
- }).orElse(Optional.of(bucketName));
+ }).orElse(Optional.of(BucketName.of(bucketName.asString())));
}
private boolean isNameSpace(BucketName bucketName) {
return namespace
- .map(existingNamespace -> existingNamespace.equals(bucketName))
+ .map(existingNamespace -> existingNamespace.asString().equals(bucketName.asString()))
Review Comment:
**💭 thought:** the namespace term is also called defaultBucketName in other places ( it is called namespace in the configuration file, the configuration builder accepts a namespace setter but when building the actual configuration object it injects the namespace into a field named `defaultBucketName` a similarly named field is also exposed by the S3 dao but in the bucketname resolver it is again called namespace. It is unclear to me what this concept is supposed to represent and how it is different from bucketPrefix (whose name is fairly stable throughout the code)
I don't expect an action on this MR but seeing the name here reminded me of this thought.
--
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
Re: [PR] Introduce a ResolvedBucketName type to avoid temporal coupling (and e… [james-project]
Posted by "mbaechler (via GitHub)" <gi...@apache.org>.
mbaechler commented on PR #1754:
URL: https://github.com/apache/james-project/pull/1754#issuecomment-2746353202
I probably won't finish this PR, I'm closing.
--
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
Re: [PR] Introduce a ResolvedBucketName type to avoid temporal coupling (and e… [james-project]
Posted by "mbaechler (via GitHub)" <gi...@apache.org>.
mbaechler commented on code in PR #1754:
URL: https://github.com/apache/james-project/pull/1754#discussion_r1404366324
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreDAO.java:
##########
@@ -411,7 +409,7 @@ public Mono<Void> deleteAllBuckets() {
return Mono.fromFuture(client::listBuckets)
.publishOn(Schedulers.parallel())
.flatMapIterable(ListBucketsResponse::buckets)
- .flatMap(bucket -> deleteResolvedBucket(BucketName.of(bucket.name())), DEFAULT_CONCURRENCY)
+ .flatMap(bucket -> deleteResolvedBucket(ResolvedBucketName.of(bucket.name())), DEFAULT_CONCURRENCY)
Review Comment:
We are listing buckets with S3 API then we call a delete for each bucket.
In S3, there are only buckets with "resolved names".
--
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
Re: [PR] Introduce a ResolvedBucketName type to avoid temporal coupling (and e… [james-project]
Posted by "aduprat (via GitHub)" <gi...@apache.org>.
aduprat commented on code in PR #1754:
URL: https://github.com/apache/james-project/pull/1754#discussion_r1370832940
##########
server/blob/blob-api/src/main/java/org/apache/james/blob/api/ResolvedBucketName.java:
##########
@@ -0,0 +1,66 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.api;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.Objects;
+
+public final class ResolvedBucketName {
Review Comment:
Hi @chibenwa ,
Do you have a plan to migrate to Java 14 (or 16), this will prevent from lot of code like this one.
--
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
Re: [PR] Introduce a ResolvedBucketName type to avoid temporal coupling (and e… [james-project]
Posted by "mbaechler (via GitHub)" <gi...@apache.org>.
mbaechler commented on code in PR #1754:
URL: https://github.com/apache/james-project/pull/1754#discussion_r1404367845
##########
server/blob/blob-s3/src/main/java/org/apache/james/blob/objectstorage/aws/S3BlobStoreDAO.java:
##########
@@ -420,7 +418,7 @@ public Publisher<BucketName> listBuckets() {
return Mono.fromFuture(client::listBuckets)
.flatMapIterable(ListBucketsResponse::buckets)
.map(Bucket::name)
- .handle((bucket, sink) -> bucketNameResolver.unresolve(BucketName.of(bucket))
+ .handle((bucket, sink) -> bucketNameResolver.unresolve(ResolvedBucketName.of(bucket))
Review Comment:
listBuckets returns some BucketName but list from S3 returns resolved bucket names.
--
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
Re: [PR] Introduce a ResolvedBucketName type to avoid temporal coupling (and e… [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1754:
URL: https://github.com/apache/james-project/pull/1754#discussion_r1359882574
##########
server/blob/blob-api/src/main/java/org/apache/james/blob/api/ResolvedBucketName.java:
##########
@@ -0,0 +1,66 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.api;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.Objects;
+
+public final class ResolvedBucketName {
Review Comment:
This POJO is defined in the API but only used in blob-s3.
I think introducing it is a good idea, but we should avoid poluting the API and move it into blob-s3.
--
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
Re: [PR] Introduce a ResolvedBucketName type to avoid temporal coupling (and e… [james-project]
Posted by "mbaechler (via GitHub)" <gi...@apache.org>.
mbaechler commented on PR #1754:
URL: https://github.com/apache/james-project/pull/1754#issuecomment-1804569934
> @mbaechler status on this? any plan on fixing soon checkstyle issues and answering comments? :)
I apologize for dropping PRs without a proper follow-up plan.
I will try this week.
--
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
Re: [PR] Introduce a ResolvedBucketName type to avoid temporal coupling (and e… [james-project]
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1754:
URL: https://github.com/apache/james-project/pull/1754#discussion_r1371086584
##########
server/blob/blob-api/src/main/java/org/apache/james/blob/api/ResolvedBucketName.java:
##########
@@ -0,0 +1,66 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.blob.api;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.Objects;
+
+public final class ResolvedBucketName {
Review Comment:
If we upgrade better a LTS version, so 17 (or even 21 would be better, but maybe too big of a jump?)
I would support such a migration, but I'm not sure the all community is ready to upgrade their java version :)
Could perhaps start a thread on the ML though?
--
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
Re: [PR] Introduce a ResolvedBucketName type to avoid temporal coupling (and e… [james-project]
Posted by "mbaechler (via GitHub)" <gi...@apache.org>.
mbaechler closed pull request #1754: Introduce a ResolvedBucketName type to avoid temporal coupling (and e…
URL: https://github.com/apache/james-project/pull/1754
--
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