You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "hungphan227 (via GitHub)" <gi...@apache.org> on 2023/08/08 08:01:31 UTC

[GitHub] [james-project] hungphan227 opened a new pull request, #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

hungphan227 opened a new pull request, #1670:
URL: https://github.com/apache/james-project/pull/1670

   (no comment)


-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1291178212


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, Username identifier, QuotaType quotaType, long currentValue) {

Review Comment:
   If I use QuotaRoot, I will have to use something like "private&benwa" in which I do not really understand why we need the keyword private



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1289519242


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,74 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, String identifier, QuotaType quotaType, long currentValue) {
+        return new QuotaCurrentValue(quotaComponent, identifier, quotaType, currentValue);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final String identifier;
+    private final QuotaType quotaType;
+    private final long currentValue;

Review Comment:
   Copied from cassandra official web page:
   "The counter type is used to define counter columns. A counter column is a column whose value is a 64-bit signed integer and on which 2 operations are supported: incrementing and decrementing (see the [UPDATE](https://cassandra.apache.org/doc/latest/cassandra/cql/dml.html#update-statement) statement for syntax). Note that the value of a counter cannot be set: a counter does not exist until first incremented/decremented, and that first increment/decrement is made as if the prior value was 0"



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1291138862


##########
core/src/main/java/org/apache/james/core/quota/QuotaComponent.java:
##########
@@ -0,0 +1,45 @@
+/****************************************************************
+ * 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.core.quota;
+
+import org.apache.james.core.StringWrapper;
+
+public class QuotaComponent extends StringWrapper {

Review Comment:
   No it is not enough. We should restrict the value of QuotaComponent within three values mailbox, sieve, jmapUploads



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1291040537


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, Username identifier, QuotaType quotaType, long currentValue) {

Review Comment:
   Yeah actually I am a little bit confused now. I found an example of quota root that looks strange in DefaultUserQuotaRootResolverTest, it is "private&benwa". Does anyone know what this mean?



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1295395667


##########
core/src/main/java/org/apache/james/core/quota/QuotaComponent.java:
##########
@@ -0,0 +1,70 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.commons.lang3.StringUtils;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
+
+public class QuotaComponent {
+
+    public static final QuotaComponent MAILBOX = of("mailbox");
+    public static final QuotaComponent SIEVE = of("sieve");
+    public static final QuotaComponent JMAP_UPLOADS = of("jmapUploads");

Review Comment:
   We don't need to know all the values here, and likely should not.



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1292847959


##########
core/src/main/java/org/apache/james/core/quota/QuotaComponent.java:
##########
@@ -0,0 +1,45 @@
+/****************************************************************
+ * 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.core.quota;
+
+import org.apache.james.core.StringWrapper;
+
+public class QuotaComponent extends StringWrapper {

Review Comment:
   +1 with Quan. The `StringWrapper` is a workaround for a language limitation.
   
   Complex hierarchy should be avoided and 'flat' POJOs shall be prefered.
   
   If you really want to limit the boiler plate here then you could consider adding in some scala. Or waiting a Java 17+ switch.



-- 
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] quantranhong1999 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1291115647


##########
core/src/main/java/org/apache/james/core/quota/QuotaComponent.java:
##########
@@ -0,0 +1,45 @@
+/****************************************************************
+ * 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.core.quota;
+
+import org.apache.james.core.StringWrapper;
+
+public class QuotaComponent extends StringWrapper {

Review Comment:
   Isn't
   ```
   public class QuotaComponent {
       private final String value;
       // constructor, getter, equals, hashcode, toString
   }
   ```
   Enough?
   
   rf: https://github.com/apache/james-project/pull/1666#discussion_r1282614846



##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, Username identifier, QuotaType quotaType, long currentValue) {

Review Comment:
   I think the same as Rene.
   `QuotaRoot` is a kind of ID for quota already.
   
   > Username class even provide more functions for username than QuotaRoot
   
   QuotaRoot exists for a reason, otherwise, the current quota code would use Username directly instead of QuotaRoot, no?



##########
core/src/main/java/org/apache/james/core/StringWrapper.java:
##########
@@ -0,0 +1,57 @@
+/****************************************************************
+ * 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.core;
+
+import java.util.Objects;
+
+import com.google.common.base.MoreObjects;
+
+public class StringWrapper {

Review Comment:
   From what I can see: `QuotaComponent` still can work without extending `StringWrapper`.
   
   Q: What is the point of introducing this  `StringWrapper`?



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1289762080


##########
core/src/main/java/org/apache/james/core/quota/QuotaLimit.java:
##########
@@ -20,25 +20,26 @@
 package org.apache.james.core.quota;
 
 import java.util.Objects;
+import java.util.Optional;
 
 public class QuotaLimit {
 
-    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, long limit) {
+    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, Long limit) {

Review Comment:
   org/apache/james/jmap/api/filtering/Rule.java - class Condition - line 186



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1292848037


##########
core/src/main/java/org/apache/james/core/quota/QuotaComponent.java:
##########
@@ -0,0 +1,45 @@
+/****************************************************************
+ * 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.core.quota;
+
+import org.apache.james.core.StringWrapper;
+
+public class QuotaComponent extends StringWrapper {
+
+    public static final QuotaComponent MAILBOX = of("mailbox");
+    public static final QuotaComponent SIEVE = of("sieve");
+    public static final QuotaComponent JMAP_UPLOADS = of("jmapUploads");
+
+    public static QuotaComponent of(String value) {
+        return new QuotaComponent(value);
+    }
+
+    private QuotaComponent(String value) {
+        super(value);
+    }
+
+    @Override
+    public boolean equals(Object o) {

Review Comment:
   does this pass equals verifier? No hashCode, not final...



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1294390742


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,124 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+import com.google.common.base.MoreObjects;
+
+public class QuotaCurrentValue {
+
+    public static class QuotaKey {

Review Comment:
   I am not sure what he mean exactly but it gives me the idea :))



-- 
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 merged pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael merged PR #1670:
URL: https://github.com/apache/james-project/pull/1670


-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1288108423


##########
core/src/main/java/org/apache/james/core/quota/QuotaLimit.java:
##########
@@ -0,0 +1,81 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+public class QuotaLimit {
+
+    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, long limit) {
+        return new QuotaLimit(component, scope, identifier, quotaType, limit);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final QuotaScope quotaScope;
+    private final String identifier;

Review Comment:
   identifier is in primary key so it could not be null. In case of global, we could set a fixed value to the identifier ("APP" for example)



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1289756862


##########
core/src/main/java/org/apache/james/core/quota/QuotaLimit.java:
##########
@@ -20,25 +20,26 @@
 package org.apache.james.core.quota;
 
 import java.util.Objects;
+import java.util.Optional;
 
 public class QuotaLimit {
 
-    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, long limit) {
+    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, Long limit) {

Review Comment:
   Like where?



-- 
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] vttranlina commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1290867173


##########
core/src/main/java/org/apache/james/core/quota/QuotaLimit.java:
##########
@@ -0,0 +1,82 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+import java.util.Optional;
+
+public class QuotaLimit {
+
+    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, Long limit) {
+        return new QuotaLimit(component, scope, identifier, quotaType, limit);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final QuotaScope quotaScope;
+    private final String identifier;

Review Comment:
   you should consistency type class of `identifier` here, 
   ```suggestion
       private final Username identifier;
   ```
   same with QuotaCurrentValue.java



-- 
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] quantranhong1999 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1291048650


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, Username identifier, QuotaType quotaType, long currentValue) {

Review Comment:
   IMO `#private` is namespace



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1295502582


##########
core/src/main/java/org/apache/james/core/quota/QuotaComponent.java:
##########
@@ -0,0 +1,70 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.commons.lang3.StringUtils;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
+
+public class QuotaComponent {
+
+    public static final QuotaComponent MAILBOX = of("mailbox");
+    public static final QuotaComponent SIEVE = of("sieve");
+    public static final QuotaComponent JMAP_UPLOADS = of("jmapUploads");

Review Comment:
   These constants would be used in other functions. For example, JMAP_UPLOADS would be used in jmap upload function and recomputation function. 



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1288182877


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,74 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, String identifier, QuotaType quotaType, long currentValue) {
+        return new QuotaCurrentValue(quotaComponent, identifier, quotaType, currentValue);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final String identifier;
+    private final QuotaType quotaType;
+    private final long currentValue;

Review Comment:
   In fact, this value is in counter table. I think it should never be null



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1293082122


##########
core/src/main/java/org/apache/james/core/quota/QuotaComponent.java:
##########
@@ -0,0 +1,45 @@
+/****************************************************************
+ * 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.core.quota;
+
+import org.apache.james.core.StringWrapper;
+
+public class QuotaComponent extends StringWrapper {

Review Comment:
   deleted



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1294402151


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,124 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+import com.google.common.base.MoreObjects;
+
+public class QuotaCurrentValue {
+
+    public static class QuotaKey {

Review Comment:
   I think you go the idea wrong. The fact that @quantranhong1999 put a thumb up on the comment above means he likely agrees to the comment as well
   
   Please rework on this :)



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1294384775


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,124 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+import com.google.common.base.MoreObjects;
+
+public class QuotaCurrentValue {
+
+    public static class QuotaKey {

Review Comment:
   He suggested to do one class QuotaKey, not two? 



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1288282045


##########
core/src/main/java/org/apache/james/core/quota/QuotaLimit.java:
##########
@@ -20,25 +20,26 @@
 package org.apache.james.core.quota;
 
 import java.util.Objects;
+import java.util.Optional;
 
 public class QuotaLimit {
 
-    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, long limit) {
+    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, Long limit) {

Review Comment:
   I saw this method in other places of james. I think we prefer this way to create objects :))



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1289772597


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,74 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, String identifier, QuotaType quotaType, long currentValue) {
+        return new QuotaCurrentValue(quotaComponent, identifier, quotaType, currentValue);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final String identifier;
+    private final QuotaType quotaType;
+    private final long currentValue;

Review Comment:
   If the counter does not exist, the whole row in cassandra table also does not exist. Therefore, when converting from a row to a QuotaCurrentValue object, the field currentValue of QuotaCurrentValue object always has a value



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1289819729


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, Username identifier, QuotaType quotaType, long currentValue) {

Review Comment:
   I think it does not fit. the domain part in QuotaRoot is optional while the identifier in QuotaCurrentValue always has this part



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1290871837


##########
core/src/main/java/org/apache/james/core/quota/QuotaLimit.java:
##########
@@ -0,0 +1,82 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+import java.util.Optional;
+
+public class QuotaLimit {
+
+    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, Long limit) {
+        return new QuotaLimit(component, scope, identifier, quotaType, limit);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final QuotaScope quotaScope;
+    private final String identifier;

Review Comment:
   The identifier of QuotaLimit is not necessarily username. It could be other things (domain name for example) 



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1292848120


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, Username identifier, QuotaType quotaType, long currentValue) {

Review Comment:
   Builder please?



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1292968407


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, Username identifier, QuotaType quotaType, long currentValue) {

Review Comment:
   Forget the Quota Root actually :)



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1295396341


##########
core/src/main/java/org/apache/james/core/quota/QuotaComponent.java:
##########
@@ -0,0 +1,70 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.commons.lang3.StringUtils;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
+
+public class QuotaComponent {
+
+    public static final QuotaComponent MAILBOX = of("mailbox");
+    public static final QuotaComponent SIEVE = of("sieve");
+    public static final QuotaComponent JMAP_UPLOADS = of("jmapUploads");
+
+    public static QuotaComponent of(String value) {
+        Preconditions.checkArgument(StringUtils.isNotBlank(value), "`value` is mandatory");
+        return new QuotaComponent(value);
+    }
+
+    private final String value;
+
+    private QuotaComponent(String value) {
+        this.value = value;
+    }
+
+    public String getValue() {
+        return value;
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(value);
+    }
+
+    @Override
+    public boolean equals(Object o) {

Review Comment:
   Why not final?



##########
core/src/main/java/org/apache/james/core/quota/QuotaComponent.java:
##########
@@ -0,0 +1,70 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.commons.lang3.StringUtils;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
+
+public class QuotaComponent {
+
+    public static final QuotaComponent MAILBOX = of("mailbox");
+    public static final QuotaComponent SIEVE = of("sieve");
+    public static final QuotaComponent JMAP_UPLOADS = of("jmapUploads");
+
+    public static QuotaComponent of(String value) {
+        Preconditions.checkArgument(StringUtils.isNotBlank(value), "`value` is mandatory");
+        return new QuotaComponent(value);
+    }
+
+    private final String value;
+
+    private QuotaComponent(String value) {
+        this.value = value;
+    }
+
+    public String getValue() {
+        return value;
+    }
+
+    @Override
+    public int hashCode() {

Review Comment:
   Why not final?



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1288016249


##########
core/src/main/java/org/apache/james/core/StringWrapper.java:
##########
@@ -0,0 +1,57 @@
+/****************************************************************
+ * 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.core;
+
+import java.util.Objects;
+
+import com.google.common.base.MoreObjects;
+
+public class StringWrapper {

Review Comment:
   Not sure this was necessary. Allows to reduce boiler plate, reduces readability though a bit IMO.
   
   But I'm not against it.



##########
core/src/main/java/org/apache/james/core/quota/QuotaLimit.java:
##########
@@ -0,0 +1,81 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+public class QuotaLimit {
+
+    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, long limit) {
+        return new QuotaLimit(component, scope, identifier, quotaType, limit);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final QuotaScope quotaScope;
+    private final String identifier;

Review Comment:
   Thinking what we could do here...
   
   Should be at least an Optional (global quota limit will not have an identifier for example)
   
   After the identifier will be either a Username (account limit) or a Domain (domain limit)
   
   But I guess if we want to be flexible to extensions even on the scope, String is likely fine I guess. But I would still make it Optional, for the global case



##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,74 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, String identifier, QuotaType quotaType, long currentValue) {
+        return new QuotaCurrentValue(quotaComponent, identifier, quotaType, currentValue);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final String identifier;

Review Comment:
   A quota current value would be logically the value of a user's usage... I would expect here Username instead of String



##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,74 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, String identifier, QuotaType quotaType, long currentValue) {
+        return new QuotaCurrentValue(quotaComponent, identifier, quotaType, currentValue);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final String identifier;
+    private final QuotaType quotaType;
+    private final long currentValue;

Review Comment:
   Optional<Long> ? Maybe there is no quota yet for a user that just got created  and didnt receive any message yet for example?



##########
core/src/main/java/org/apache/james/core/quota/QuotaLimit.java:
##########
@@ -0,0 +1,81 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+public class QuotaLimit {
+
+    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, long limit) {
+        return new QuotaLimit(component, scope, identifier, quotaType, limit);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final QuotaScope quotaScope;
+    private final String identifier;
+    private final QuotaType quotaType;
+    private final long limit;

Review Comment:
   Optional<Long>?
   
   We can have undefined limits



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1288280931


##########
core/src/main/java/org/apache/james/core/quota/QuotaComponent.java:
##########
@@ -23,6 +23,14 @@
 
 public class QuotaComponent extends StringWrapper {
 
+    public static final QuotaComponent MAILBOX = of("mailbox");
+    public static final QuotaComponent SIEVE = of("sieve");
+    public static final QuotaComponent JMAP_UPLOADS = of("jmapUploads");
+
+    public static QuotaComponent of(String value) {

Review Comment:
   Just a shorter way to create new QuotaComponent. It does not matter which way I use, right?



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1290834494


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, Username identifier, QuotaType quotaType, long currentValue) {

Review Comment:
   Nope you are wrong here actually :) 
   
   Look in UsersRepository code and configuration file... We have a parameter, `supportVirtualHosting`. If it's true we have domain names, if false we don't. 



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1291131646


##########
core/src/main/java/org/apache/james/core/StringWrapper.java:
##########
@@ -0,0 +1,57 @@
+/****************************************************************
+ * 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.core;
+
+import java.util.Objects;
+
+import com.google.common.base.MoreObjects;
+
+public class StringWrapper {

Review Comment:
   to reuse some code in other classes: QuotaType, QuotaScope



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1293082122


##########
core/src/main/java/org/apache/james/core/quota/QuotaComponent.java:
##########
@@ -0,0 +1,45 @@
+/****************************************************************
+ * 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.core.quota;
+
+import org.apache.james.core.StringWrapper;
+
+public class QuotaComponent extends StringWrapper {

Review Comment:
   deleted



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1290835461


##########
core/src/main/java/org/apache/james/core/quota/QuotaLimit.java:
##########
@@ -20,25 +20,26 @@
 package org.apache.james.core.quota;
 
 import java.util.Objects;
+import java.util.Optional;
 
 public class QuotaLimit {
 
-    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, long limit) {
+    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, Long limit) {

Review Comment:
   Missed your constructors were private. My bad



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1288211513


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,74 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, String identifier, QuotaType quotaType, long currentValue) {
+        return new QuotaCurrentValue(quotaComponent, identifier, quotaType, currentValue);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final String identifier;
+    private final QuotaType quotaType;
+    private final long currentValue;

Review Comment:
   Not necessary? What if it's not? As said above user just got created for example..? 



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1288212448


##########
core/src/main/java/org/apache/james/core/quota/QuotaLimit.java:
##########
@@ -0,0 +1,81 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+public class QuotaLimit {
+
+    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, long limit) {
+        return new QuotaLimit(component, scope, identifier, quotaType, limit);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final QuotaScope quotaScope;
+    private final String identifier;

Review Comment:
   Correct! I think just "global" then



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1288214890


##########
core/src/main/java/org/apache/james/core/quota/QuotaComponent.java:
##########
@@ -23,6 +23,14 @@
 
 public class QuotaComponent extends StringWrapper {
 
+    public static final QuotaComponent MAILBOX = of("mailbox");
+    public static final QuotaComponent SIEVE = of("sieve");
+    public static final QuotaComponent JMAP_UPLOADS = of("jmapUploads");
+
+    public static QuotaComponent of(String value) {

Review Comment:
   What's the point here?
   
   Could just do `public static final QuotaComponent MAILBOX = new QuotaComponent("mailbox");` right?



##########
core/src/main/java/org/apache/james/core/quota/QuotaLimit.java:
##########
@@ -20,25 +20,26 @@
 package org.apache.james.core.quota;
 
 import java.util.Objects;
+import java.util.Optional;
 
 public class QuotaLimit {
 
-    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, long limit) {
+    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, Long limit) {

Review Comment:
   Missed that the first time but is that static method useful here as well?



##########
core/src/main/java/org/apache/james/core/quota/QuotaType.java:
##########
@@ -23,6 +23,13 @@
 
 public class QuotaType extends StringWrapper {
 
+    public static final QuotaType COUNT = of("count");
+    public static final QuotaType SIZE = of("size");
+
+    public static QuotaType of(String value) {

Review Comment:
   idem



##########
core/src/main/java/org/apache/james/core/quota/QuotaScope.java:
##########
@@ -23,6 +23,14 @@
 
 public class QuotaScope extends StringWrapper {
 
+    public static final QuotaScope GLOBAL = of("global");
+    public static final QuotaScope DOMAIN = of("domain");
+    public static final QuotaScope USER = of("user");
+
+    public static QuotaScope of(String value) {

Review Comment:
   Idem



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1289758571


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,74 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, String identifier, QuotaType quotaType, long currentValue) {
+        return new QuotaCurrentValue(quotaComponent, identifier, quotaType, currentValue);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final String identifier;
+    private final QuotaType quotaType;
+    private final long currentValue;

Review Comment:
   `a counter does not exist until first incremented/decremented`
   
   Which was the point... what if the counter does not exist yet for that user? Turning in circles 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] Arsnael commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1289775658


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, Username identifier, QuotaType quotaType, long currentValue) {

Review Comment:
   Thinking about it again, maybe the identifier should be a QuotaRoot instead, when looking at current implem (and to align with IMAP)



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1288025380


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,74 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, String identifier, QuotaType quotaType, long currentValue) {
+        return new QuotaCurrentValue(quotaComponent, identifier, quotaType, currentValue);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final String identifier;
+    private final QuotaType quotaType;
+    private final long currentValue;

Review Comment:
   `Optional<Long>` ? Maybe there is no quota yet for a user that just got created  and didnt receive any message yet for example?



##########
core/src/main/java/org/apache/james/core/quota/QuotaLimit.java:
##########
@@ -0,0 +1,81 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+public class QuotaLimit {
+
+    public static QuotaLimit of(QuotaComponent component, QuotaScope scope, String identifier, QuotaType quotaType, long limit) {
+        return new QuotaLimit(component, scope, identifier, quotaType, limit);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final QuotaScope quotaScope;
+    private final String identifier;
+    private final QuotaType quotaType;
+    private final long limit;

Review Comment:
   `Optional<Long>`?
   
   We can have undefined limits



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1288182877


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,74 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, String identifier, QuotaType quotaType, long currentValue) {
+        return new QuotaCurrentValue(quotaComponent, identifier, quotaType, currentValue);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final String identifier;
+    private final QuotaType quotaType;
+    private final long currentValue;

Review Comment:
   In fact, this value in in counter table. I think it should never be null



-- 
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] vttranlina commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1290870509


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, Username identifier, QuotaType quotaType, long currentValue) {

Review Comment:
   Not sure, but I just quickly check `com.linagora.tmail.team.TMailQuotaRootResolver`, Quotaroot is not only Username, It depends on which impl of `UserQuotaRootResolver` 
   Example quotaRoot of TeamMailbox
   
   ```
   def quotaRoot: QuotaRoot = QuotaRoot.quotaRoot(s"$TEAM_MAILBOX_NAMESPACE&${mailboxName.value.value}@${domain.asString()}", Some(domain).toJava)
   ```



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1290835962


##########
core/src/main/java/org/apache/james/core/quota/QuotaComponent.java:
##########
@@ -23,6 +23,14 @@
 
 public class QuotaComponent extends StringWrapper {
 
+    public static final QuotaComponent MAILBOX = of("mailbox");
+    public static final QuotaComponent SIEVE = of("sieve");
+    public static final QuotaComponent JMAP_UPLOADS = of("jmapUploads");
+
+    public static QuotaComponent of(String value) {

Review Comment:
   Make the constructor private then?



##########
core/src/main/java/org/apache/james/core/quota/QuotaType.java:
##########
@@ -23,6 +23,13 @@
 
 public class QuotaType extends StringWrapper {
 
+    public static final QuotaType COUNT = of("count");
+    public static final QuotaType SIZE = of("size");
+
+    public static QuotaType of(String value) {

Review Comment:
   Make the constructor private then?



##########
core/src/main/java/org/apache/james/core/quota/QuotaScope.java:
##########
@@ -23,6 +23,14 @@
 
 public class QuotaScope extends StringWrapper {
 
+    public static final QuotaScope GLOBAL = of("global");
+    public static final QuotaScope DOMAIN = of("domain");
+    public static final QuotaScope USER = of("user");
+
+    public static QuotaScope of(String value) {

Review Comment:
   Make the constructor private then?



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1294278686


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,124 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+import com.google.common.base.MoreObjects;
+
+public class QuotaCurrentValue {
+
+    public static class QuotaKey {

Review Comment:
   After reading Benoit's comment, I feel it is better to group these field in a subclass as a key. Not sure why you are confused.



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1293084488


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, Username identifier, QuotaType quotaType, long currentValue) {

Review Comment:
   changed to quota key and currentvalue -> too few fields -> builder not needed



-- 
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] quantranhong1999 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1291170821


##########
core/src/main/java/org/apache/james/core/quota/QuotaComponent.java:
##########
@@ -0,0 +1,45 @@
+/****************************************************************
+ * 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.core.quota;
+
+import org.apache.james.core.StringWrapper;
+
+public class QuotaComponent extends StringWrapper {

Review Comment:
   The way how we blueprint the `QuotaComponent` class is not related to `restrict the value of QuotaComponent`, no?
   
   How is that related to `StringWrapper` usage BTW?



-- 
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] hungphan227 commented on pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on PR #1670:
URL: https://github.com/apache/james-project/pull/1670#issuecomment-1680109557

   > I do not get why these POJOs are in `/core` and not in `backend-cassandra`
   
   I see that old quota POJOs are in `/core` so I put new POJOs in there as well


-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1289756065


##########
core/src/main/java/org/apache/james/core/quota/QuotaComponent.java:
##########
@@ -23,6 +23,14 @@
 
 public class QuotaComponent extends StringWrapper {
 
+    public static final QuotaComponent MAILBOX = of("mailbox");
+    public static final QuotaComponent SIEVE = of("sieve");
+    public static final QuotaComponent JMAP_UPLOADS = of("jmapUploads");
+
+    public static QuotaComponent of(String value) {

Review Comment:
   I find adding a whole static method just to pass from `new QuotaComponent` to `of` is adding more verbosity for... not much any kind of gain?



-- 
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] quantranhong1999 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1291134486


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, Username identifier, QuotaType quotaType, long currentValue) {

Review Comment:
   > what is namespace? :))
   
   https://www.rfc-editor.org/rfc/rfc2342.html
   
   QUOTAROOT IMAP commands spec: https://www.rfc-editor.org/rfc/rfc2087
   
   QUOTAROOT IMAP example: https://github.com/apache/james-project/blob/master/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/Quota.test
   
   It looks like QuotaRoot is actually the id of quota, and is specially bound to the IMAP concept. As Rene said, I think that we should still respect the QuotaRoot 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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1291114118


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, Username identifier, QuotaType quotaType, long currentValue) {

Review Comment:
   what is namespace? :))



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1290854868


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, Username identifier, QuotaType quotaType, long currentValue) {

Review Comment:
   I see that we still could use Username class that have the same logic as QuotaRoot in terms of username. Username class even provide more functions for username than QuotaRoot



-- 
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 diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1294143589


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,124 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+import com.google.common.base.MoreObjects;
+
+public class QuotaCurrentValue {
+
+    public static class QuotaKey {

Review Comment:
   Those 2 QuotaKey classes that look similar but a bit different is looking wrong and confusing to me. 
   
   Or we:
   - find a way to unify that into one POJO (and separated, not a subclass of an other POJO) => then maybe using QuotaScope as identifier and finding a good structure would make sense then
   - just sticking to builders here (need to add them), and have the QuotaKey for the DAO with current values only (next PR), if sticking to Username as identifier.
   
   I start to think though now that refactoring QuotaScope to fit the identifier and scope needs might be the better way 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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1288105380


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,74 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, String identifier, QuotaType quotaType, long currentValue) {
+        return new QuotaCurrentValue(quotaComponent, identifier, quotaType, currentValue);
+    }
+
+    private final QuotaComponent quotaComponent;
+    private final String identifier;
+    private final QuotaType quotaType;
+    private final long currentValue;

Review Comment:
   I think initially it should be zero



-- 
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] quantranhong1999 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1291179061


##########
core/src/main/java/org/apache/james/core/StringWrapper.java:
##########
@@ -0,0 +1,57 @@
+/****************************************************************
+ * 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.core;
+
+import java.util.Objects;
+
+import com.google.common.base.MoreObjects;
+
+public class StringWrapper {

Review Comment:
   If you mean "reuse some code" is reuse the String attribute then I am not convinced.
   
   Why just not declare that string attribute directly in `QuotaType`, `QuotaScope`, and `QuotaComponent`?
   
   IMO the types of classes `QuotaType`, `QuotaScope`, and `QuotaComponent` have nothing in common (imagine in the real world) that needs inheritance. String is just an implementation detail of those class, extending all of them from `StringWrapper` just add more complexity and potentially misunderstand binding IMO.



-- 
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] hungphan227 commented on a diff in pull request #1670: JAMES-4824 Refactor quota for Cassandra: create POJOs

Posted by "hungphan227 (via GitHub)" <gi...@apache.org>.
hungphan227 commented on code in PR #1670:
URL: https://github.com/apache/james-project/pull/1670#discussion_r1291178212


##########
core/src/main/java/org/apache/james/core/quota/QuotaCurrentValue.java:
##########
@@ -0,0 +1,76 @@
+/****************************************************************
+ * 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.core.quota;
+
+import java.util.Objects;
+
+import org.apache.james.core.Username;
+
+public class QuotaCurrentValue {
+
+    public static QuotaCurrentValue of(QuotaComponent quotaComponent, Username identifier, QuotaType quotaType, long currentValue) {

Review Comment:
   If I use QuotaRoot, I will have to use something like "private&benwa" in which I do not really understand why we need the keywork private



-- 
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