chibenwa commented on code in PR #1707:
URL: https://github.com/apache/james-project/pull/1707#discussion_r1323019284
##########
mailbox/tools/quota-recompute/src/main/java/org/apache/james/mailbox/quota/task/RecomputeCurrentQuotasTaskAdditionalInformationDTO.java:
##########
@@ -50,29 +110,22 @@ public static
AdditionalInformationDTOModule<RecomputeCurrentQuotasTask.Details,
}
private final String type;
- private final long processedQuotaRoots;
- private final ImmutableList<String> failedQuotaRoots;
+ private final List<RecomputeSingleQuotaComponentResultDTO>
recomputeSingleQuotaComponentResults;
private final Optional<RunningOptionsDTO> runningOptions;
private final Instant timestamp;
public
RecomputeCurrentQuotasTaskAdditionalInformationDTO(@JsonProperty("type") String
type,
-
@JsonProperty("processedQuotaRoots") long processedQuotaRoots,
-
@JsonProperty("failedQuotaRoots") ImmutableList<String> failedQuotaRoots,
+
@JsonProperty("recomputeSingleQuotaComponentResults")
List<RecomputeSingleQuotaComponentResultDTO>
recomputeSingleQuotaComponentResults,
Review Comment:
This breaking change will break the distributed task manager.
You NEED to enforce compatibility with the hold format and have
deserialisation tests prooving that you handle the old format.
Here itis as simple as still accepting `processedQuotaRoots` and
`failedQuotaRoots` as optionals and use it to populate a
`recomputeSingleQuotaComponentResults` for the MAILBOX entry...
##########
mailbox/tools/quota-recompute/src/test/java/org/apache/james/mailbox/quota/task/RecomputeCurrentQuotasTaskSerializationTest.java:
##########
@@ -36,23 +37,29 @@ class RecomputeCurrentQuotasTaskSerializationTest {
static final String QUOTA_ROOT_AS_STRING = "bob@localhost";
static final RecomputeCurrentQuotasService SERVICE =
mock(RecomputeCurrentQuotasService.class);
- static final RecomputeCurrentQuotasTask TASK = new
RecomputeCurrentQuotasTask(SERVICE, RunningOptions.withUsersPerSecond(17));
Review Comment:
Backward compatibility needs to be tested in this file
##########
server/data/data-jmap-cassandra/src/test/java/org/apache/james/jmap/cassandra/upload/CassandraJMAPCurrentUploadUsageCalculatorTest.java:
##########
@@ -0,0 +1,72 @@
+/****************************************************************
+ * 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.jmap.cassandra.upload;
+
+import java.time.Clock;
+
+import org.apache.james.backends.cassandra.CassandraClusterExtension;
+import org.apache.james.backends.cassandra.components.CassandraModule;
+import
org.apache.james.backends.cassandra.components.CassandraMutualizedQuotaModule;
+import
org.apache.james.backends.cassandra.components.CassandraQuotaCurrentValueDao;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.blob.api.HashBlobId;
+import org.apache.james.blob.memory.MemoryBlobStoreDAO;
+import org.apache.james.jmap.api.upload.JMAPCurrentUploadUsageCalculator;
+import
org.apache.james.jmap.api.upload.JMAPCurrentUploadUsageCalculatorContract;
+import org.apache.james.jmap.api.upload.UploadRepository;
+import org.apache.james.jmap.api.upload.UploadUsageRepository;
+import org.apache.james.server.blob.deduplication.DeDuplicationBlobStore;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+public class CassandraJMAPCurrentUploadUsageCalculatorTest implements
JMAPCurrentUploadUsageCalculatorContract {
Review Comment:
IMO testing recomputation logic on top of memory is enough.
##########
server/container/guice/protocols/webadmin-mailbox/src/main/java/org/apache/james/modules/server/RecomputeSingleComponentCurrentQuotasServicesExcludingJMAPModule.java:
##########
@@ -0,0 +1,35 @@
+/****************************************************************
+ * 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.modules.server;
+
+import
org.apache.james.mailbox.quota.task.RecomputeMailboxCurrentQuotasService;
+import
org.apache.james.mailbox.quota.task.RecomputeSingleComponentCurrentQuotasService;
+
+import com.google.inject.AbstractModule;
+import com.google.inject.multibindings.Multibinder;
+
+public class RecomputeSingleComponentCurrentQuotasServicesExcludingJMAPModule
extends AbstractModule {
Review Comment:
This too long of a class name is symptomatic: we have a problem here... 64
characters!
My bet is that it hould not exist.
My recommendation:
- Bind Mailbox recomputation service in `MailboxRoutesModule`
- Bind Jmap recomputation service in `JmapTaskSerializationModule`
- Bind Sieve recomputation service in `SieveRoutesModule`
That way no additional module is needed and the assembling would be
automatically the good one.
##########
src/site/markdown/server/manage-webadmin.md:
##########
@@ -1996,14 +1996,20 @@ go out of sync, leading to inconsistent results being
returned to the client.
An admin can specify the concurrency that should be used when running the task:
- - `usersPerSecond` rate at which users quotas should be reprocessed, per
second. Defaults to 1.
+ - `usersPerSecond` rate at which users quotas should be reprocessed, per
second. Defaults to 1.
Review Comment:
```suggestion
- `usersPerSecond` rate at which users quotas should be reprocessed, per
second. Defaults to 1.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
