You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2022/05/17 14:12:25 UTC

[GitHub] [james-project] ottoka opened a new pull request, #1004: James 3758

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

   As discussed in the Jira ticket, this is my solution for deleting old emails form users INBOX. It can work in two ways:
   
   1. Use Expires header in the stored mails:
       POST http://ip:port/mailboxes?task=expireInboxes
   2. Use age relative to time of invocation, e.g. all mails older than 30 days:
       POST http://ip:port/mailboxes?task=expireInboxes&maxAge=30d
   
   Optionally include &usersPerSecond=10 to control execution throttling.
   
   To use variant 1, there is also a new "Expires" Mailet, which can add a default expires header, or enforce a min/max time bracket on an existing Expires header.
   
   (Also includes a small fix to MessageSearches to respect the zone offset in a date-time 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] ottoka commented on pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on PR #1004:
URL: https://github.com/apache/james-project/pull/1004#issuecomment-1131678968

   Latest push fixes the issues discussed here so far. Invocation now is 
   1. DELETE /messages?byExpiresHeader
   2. DELETE /messages?olderThan=30d
   
   Still with optional &usersPerSecond=10
   Let me know what you think. There is no documentation yet, if everything stays as is I will add it as a separate commit.


-- 
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 #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r874978669


##########
mailet/standard/src/main/java/org/apache/james/transport/mailets/Expires.java:
##########
@@ -0,0 +1,144 @@
+/****************************************************************
+ * 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.transport.mailets;
+
+import java.io.StringReader;
+import java.time.Duration;
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.time.temporal.ChronoUnit;
+import java.util.Optional;
+import java.util.function.Supplier;
+
+import javax.mail.MessagingException;
+import javax.mail.internet.MimeMessage;
+
+import org.apache.james.mime4j.dom.datetime.DateTime;
+import org.apache.james.mime4j.field.datetime.parser.DateTimeParser;
+import org.apache.james.mime4j.field.datetime.parser.ParseException;
+import org.apache.james.util.DurationParser;
+import org.apache.mailet.Mail;
+import org.apache.mailet.base.DateFormats;
+import org.apache.mailet.base.GenericMailet;
+
+
+/**
+ * <p>Adds an Expires header to the message, or enforces the period of an existing one.</p>
+ *
+ * <p>Sample configuration:</p>
+ *
+ * <pre><code>
+ * &lt;mailet match="All" class="Expires"&gt;
+ *   &lt;minAge&gt;1d&lt;/minAge&gt;
+ *   &lt;maxAge&gt;1w&lt;/maxAge&gt;
+ *   &lt;defaultAge&gt;4w&lt;/defaultAge&gt;
+ * &lt;/mailet&gt;
+ * </code></pre>
+ *
+ * @version 1.0.0, 2021-12-14
+ */
+public class Expires extends GenericMailet {
+    
+    public static final String EXPIRES = "Expires";
+    
+    Supplier<ZonedDateTime> timeSource = ZonedDateTime::now; 
+
+    private Optional<Duration> minAge = Optional.empty();
+    private Optional<Duration> maxAge = Optional.empty();
+    private Optional<Duration> defaultAge = Optional.empty();
+    
+    @Override
+    public void init() throws MessagingException {
+        minAge = parseDuration("minAge");
+        maxAge = parseDuration("maxAge");
+        defaultAge = parseDuration("defaultAge");
+
+        if (minAge.isEmpty() && maxAge.isEmpty() && defaultAge.isEmpty()) {
+            throw new MessagingException("Please configure at least one of minAge, maxAge, defaultAge");
+        }
+
+        if (isAfter(minAge, maxAge)) {
+            throw new MessagingException("minAge must be before maxAge");
+        }
+        if (isAfter(defaultAge, maxAge)) {
+            throw new MessagingException("defaultAge must be before maxAge");
+        }
+        if (isAfter(minAge, defaultAge)) {
+            throw new MessagingException("minAge must be before defaultAge");
+        }
+    }
+
+    @Override
+    public void service(Mail mail) throws MessagingException {
+        ZonedDateTime now = timeSource.get();
+        MimeMessage message = mail.getMessage();
+        Optional<ZonedDateTime> expires = parseExpiresHeader(message);
+        if (expires.isPresent()) {
+            if (minAge.isPresent() && expires.get().isBefore(now.plus(minAge.get()))) {
+                setExpiresAfter(message, now, minAge.get());
+            } else
+            if (maxAge.isPresent() && expires.get().isAfter(now.plus(maxAge.get()))) {
+                setExpiresAfter(message, now, maxAge.get());
+            }
+        } else if (defaultAge.isPresent()) {
+            setExpiresAfter(message, now, defaultAge.get());
+        }
+    }
+
+    @Override
+    public String getMailetInfo() {
+        return "Expire Mailet";
+    }
+
+    private Optional<Duration> parseDuration(String param) {
+        String duration = getInitParameter(param);
+        if (duration == null) {
+            return Optional.empty();
+        } else {
+            return Optional.of(DurationParser.parse(duration, ChronoUnit.DAYS));
+        }
+    }
+    
+    private boolean isAfter(Optional<Duration> a, Optional<Duration> b) {
+        return a.isPresent() && b.isPresent() && a.get().compareTo(b.get()) > 0;
+    }
+    
+    private Optional<ZonedDateTime> parseExpiresHeader(MimeMessage message) {
+        try {
+            String[] expires = message.getHeader(EXPIRES);
+            if (expires == null || expires.length == 0) {
+                return Optional.empty();
+            } else {
+                DateTime dt = new DateTimeParser(new StringReader(expires[0])).parseAll();
+                return Optional.of(ZonedDateTime.of(
+                    dt.getYear(), dt.getMonth(), dt.getDay(),
+                    dt.getHour(), dt.getMinute(), dt.getSecond(), 0,
+                    ZoneOffset.ofHoursMinutes(dt.getTimeZone() / 100, dt.getTimeZone() % 100)));

Review Comment:
   Calendars / dates is a complicated topic that we likely should not under-estimate. This is complex and may be subject to subtil edge cases. I would be careful. Moreover DateTime entity is untested in MIME4J and not used in relevant places.
   
   I would likely refrain from using it.
   
   A more classic approach would be to rely on `DateTimeFieldLenientImpl.PARSER` that is easy to use, well tested and moreover is polite enough to return a Date that is easy to convert into a ZoneDateTime without taking any risk.
   
   I would encourage to migrate this piece of code to use it.



##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,209 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, Optional.empty());
+
+        private final int usersPerSecond;
+
+        private final Optional<String> maxAge;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("maxAge") Optional<String> maxAge) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' needs to be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+    
+            this.maxAge = maxAge;
+            this.maxAgeDuration = maxAge.map(v -> {
+                Duration maxAgeDuration = DurationParser.parse(maxAge.get(), ChronoUnit.DAYS);
+                Preconditions.checkArgument(!maxAgeDuration.isNegative(), "'maxAge' must be positive");
+                return maxAgeDuration;
+            });
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+
+        public Optional<String> getMaxAge() {
+            return maxAge;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireInboxesService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireInboxesService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireInboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            LOGGER.info("expire with maxAge {} = {}", runningOptions.maxAge, runningOptions.maxAgeDuration);
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)
+                    )
+            );
+            return Iterators.toFlux(usersRepository.list())
+                .transform(ReactorUtils.<Username, Task.Result>throttle()
+                    .elements(runningOptions.getUsersPerSecond())
+                    .per(Duration.ofSeconds(1))
+                    .forOperation(username -> expireUserInbox(context, username, expiration)))

Review Comment:
   Coming from a POP3 world only the INBOX exists but on the outer world there are protocols allowing to access a miriad of mailboxes.
   
   I propose to instead use a multi mailbox search query to apply the deletion accross all mailboxes of the account.



##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,209 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, Optional.empty());
+
+        private final int usersPerSecond;
+
+        private final Optional<String> maxAge;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("maxAge") Optional<String> maxAge) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' needs to be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+    
+            this.maxAge = maxAge;
+            this.maxAgeDuration = maxAge.map(v -> {
+                Duration maxAgeDuration = DurationParser.parse(maxAge.get(), ChronoUnit.DAYS);
+                Preconditions.checkArgument(!maxAgeDuration.isNegative(), "'maxAge' must be positive");
+                return maxAgeDuration;
+            });
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+
+        public Optional<String> getMaxAge() {
+            return maxAge;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireInboxesService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireInboxesService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireInboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            LOGGER.info("expire with maxAge {} = {}", runningOptions.maxAge, runningOptions.maxAgeDuration);
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)

Review Comment:
   Not all search engines implementations supports date search on arbitrary headers.
   
   This would require to parse and index such headers as date. Lucene and ElasticSearch implems don't do this and treat it as text.
   
   Thus this `Expires` header stuff only works with the scanning search.
   
   This should be at least documented.



##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesTask.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.webadmin.service;
+
+import java.time.Clock;
+import java.time.Instant;
+import java.util.Date;
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.james.task.Task;
+import org.apache.james.task.TaskExecutionDetails;
+import org.apache.james.task.TaskType;
+import org.apache.james.webadmin.service.ExpireInboxesService.Context;
+import org.apache.james.webadmin.service.ExpireInboxesService.RunningOptions;
+
+import reactor.core.scheduler.Schedulers;
+
+public class ExpireInboxesTask implements Task {
+    public static final TaskType TASK_TYPE = TaskType.of("ExpireInboxesTask");
+
+    public static class AdditionalInformation implements TaskExecutionDetails.AdditionalInformation {
+
+        public static ExpireInboxesTask.AdditionalInformation from(Context context,
+                                                                   RunningOptions runningOptions) {
+            return new ExpireInboxesTask.AdditionalInformation(
+                Clock.systemUTC().instant(),
+                runningOptions,
+                context.getInboxesExpired(),
+                context.getInboxesFailed(),
+                context.getInboxesProcessed());
+        }
+
+        private final Instant timestamp;
+        private final RunningOptions runningOptions;
+        private final long inboxesExpired;
+        private final long inboxesFailed;
+        private final long inboxesProcessed;
+
+        public AdditionalInformation(Instant timestamp,
+                                     RunningOptions runningOptions,
+                                     long inboxesExpired,
+                                     long inboxesFailed,
+                                     long inboxesProcessed) {
+            this.timestamp = timestamp;
+            this.runningOptions = runningOptions;
+            this.inboxesExpired = inboxesExpired;
+            this.inboxesFailed = inboxesFailed;
+            this.inboxesProcessed = inboxesProcessed;
+        }
+
+        public RunningOptions getRunningOptions() {
+            return runningOptions;
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired;
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed;
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed;
+        }
+
+        @Override
+        public Instant timestamp() {
+            return timestamp;
+        }
+    }
+
+    private final ExpireInboxesService expireInboxesService;
+    private final Context context;
+    private final RunningOptions runningOptions;
+
+    @Inject
+    public ExpireInboxesTask(ExpireInboxesService expireInboxesService,
+                             RunningOptions runningOptions) {
+        this.expireInboxesService = expireInboxesService;
+        this.context = new Context();
+        this.runningOptions = runningOptions;
+    }
+
+    @Override
+    public Result run() {
+        return expireInboxesService.expireInboxes(context, runningOptions, new Date())
+            .subscribeOn(Schedulers.elastic())

Review Comment:
   We already have a non blocking thread to execute this very reactor pipeline so why ask an elastic thread to do the job then wait the elastic thread to finish?
   
   Removing subscribesOn before .block is a performance oriented refactoring I carried over a few weeks ago.



##########
server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/service/ExpireInboxesServiceTest.java:
##########
@@ -0,0 +1,219 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.task.Task;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyList;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;

Review Comment:
   Sorry I am uneasy with mocks.
   
   They make refactoring a nightmare. They encode everywhere the inner working of the dependencies and breaks encapsulation.
   
   I suggest to rely on InMemoryTestRessource to supply a mailbox manager ready to use.



##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesAdditionalInformationDTO.java:
##########
@@ -0,0 +1,109 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Instant;
+
+import org.apache.james.json.DTOModule;
+import org.apache.james.server.task.json.dto.AdditionalInformationDTO;
+import org.apache.james.server.task.json.dto.AdditionalInformationDTOModule;
+import org.apache.james.webadmin.service.ExpireInboxesService.RunningOptions;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+public class ExpireInboxesAdditionalInformationDTO implements AdditionalInformationDTO {
+
+    public static AdditionalInformationDTOModule<ExpireInboxesTask.AdditionalInformation, ExpireInboxesAdditionalInformationDTO> module() {
+        return DTOModule.forDomainObject(ExpireInboxesTask.AdditionalInformation.class)
+            .convertToDTO(ExpireInboxesAdditionalInformationDTO.class)
+            .toDomainObjectConverter(ExpireInboxesAdditionalInformationDTO::toDomainObject)
+            .toDTOConverter(ExpireInboxesAdditionalInformationDTO::toDto)
+            .typeName(ExpireInboxesTask.TASK_TYPE.asString())
+            .withFactory(AdditionalInformationDTOModule::new);
+    }
+
+    private static ExpireInboxesTask.AdditionalInformation toDomainObject(ExpireInboxesAdditionalInformationDTO dto) {
+        return new ExpireInboxesTask.AdditionalInformation(
+            dto.getTimestamp(),
+            dto.getRunningOptions(),
+            dto.getInboxesExpired(),
+            dto.getInboxesFailed(),
+            dto.getInboxesProcessed());
+    }
+
+    private static ExpireInboxesAdditionalInformationDTO toDto(ExpireInboxesTask.AdditionalInformation details, String type) {
+        return new ExpireInboxesAdditionalInformationDTO(
+            details.timestamp(),
+            type,
+            details.getRunningOptions(),
+            details.getInboxesExpired(),
+            details.getInboxesFailed(),
+            details.getInboxesProcessed());
+    }
+
+    private final Instant timestamp;
+    private final String type;
+    private final RunningOptions runningOptions;
+    private final long inboxesExpired;
+    private final long inboxesFailed;
+    private final long inboxesProcessed;
+
+    @JsonCreator
+    public ExpireInboxesAdditionalInformationDTO(@JsonProperty("timestamp") Instant timestamp,
+                                                 @JsonProperty("type") String type,
+                                                 @JsonProperty("runningOptions") RunningOptions runningOptions,
+                                                 @JsonProperty("inboxesExpired") long inboxesExpired,
+                                                 @JsonProperty("inboxesFailed") long inboxesFailed,
+                                                 @JsonProperty("inboxesProcessed") long inboxesProcessed) {
+        this.timestamp = timestamp;
+        this.type = type;
+        this.runningOptions = runningOptions;
+        this.inboxesExpired = inboxesExpired;
+        this.inboxesFailed = inboxesFailed;
+        this.inboxesProcessed = inboxesProcessed;
+    }
+
+    public RunningOptions getRunningOptions() {
+        return runningOptions;
+    }
+
+    public long getInboxesExpired() {
+        return inboxesExpired;
+    }
+
+    public long getInboxesFailed() {
+        return inboxesFailed;
+    }
+
+    public long getInboxesProcessed() {
+        return inboxesProcessed;
+    }

Review Comment:
   A very interesting metric would be the count of messages that we actually deleted...
   
   Maybe possibly also the count of messages we failed to delete.



##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,209 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, Optional.empty());
+
+        private final int usersPerSecond;
+
+        private final Optional<String> maxAge;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("maxAge") Optional<String> maxAge) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' needs to be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+    
+            this.maxAge = maxAge;
+            this.maxAgeDuration = maxAge.map(v -> {
+                Duration maxAgeDuration = DurationParser.parse(maxAge.get(), ChronoUnit.DAYS);
+                Preconditions.checkArgument(!maxAgeDuration.isNegative(), "'maxAge' must be positive");
+                return maxAgeDuration;
+            });
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+
+        public Optional<String> getMaxAge() {
+            return maxAge;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireInboxesService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireInboxesService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireInboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            LOGGER.info("expire with maxAge {} = {}", runningOptions.maxAge, runningOptions.maxAgeDuration);
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)
+                    )
+            );
+            return Iterators.toFlux(usersRepository.list())
+                .transform(ReactorUtils.<Username, Task.Result>throttle()
+                    .elements(runningOptions.getUsersPerSecond())
+                    .per(Duration.ofSeconds(1))
+                    .forOperation(username -> expireUserInbox(context, username, expiration)))
+                .reduce(Task.Result.COMPLETED, Task::combine);
+        } catch (UsersRepositoryException e) {
+            LOGGER.error("Error while accessing users from repository", e);
+            return Mono.just(Task.Result.PARTIAL);
+        }
+    }
+
+    private Mono<Result> expireUserInbox(Context context, Username username, SearchQuery expiration) {
+        MailboxSession session = mailboxManager.createSystemSession(username);
+        MailboxPath mailboxPath = MailboxPath.inbox(username);
+        return Mono.from(mailboxManager.getMailboxReactive(mailboxPath, session))
+            // newly created users do not have mailboxes yet, just skip them
+            .onErrorResume(MailboxNotFoundException.class, ignore -> Mono.empty())
+            .flatMap(mgr -> searchMessagesReactive(mgr, session, expiration)
+                .flatMap(list -> deleteMessagesReactive(mgr, session, list)))
+            .doOnNext(expired -> {
+                if (expired) {
+                    context.incrementExpiredCount();
+                }
+                context.incrementProcessedCount();
+            })
+            .then(Mono.just(Task.Result.COMPLETED))
+            .onErrorResume(e -> {
+                LOGGER.warn("Failed to expire user mailbox {}", username, e);
+                context.incrementFailedCount();
+                context.incrementProcessedCount();
+                return Mono.just(Task.Result.PARTIAL);
+            });
+    }
+
+    private Mono<List<MessageUid>> searchMessagesReactive(MessageManager mgr, MailboxSession session, SearchQuery expiration) {
+        try {
+            return Flux.from(mgr.search(expiration, session)).collectList();
+        } catch (MailboxException e) {
+            return Mono.error(e);
+        }
+    }
+
+    private Mono<Boolean> deleteMessagesReactive(MessageManager mgr, MailboxSession session, List<MessageUid> uids) {
+        try {
+            if (uids.isEmpty()) {
+                return Mono.just(false);
+            } else {
+                mgr.delete(uids, session);
+                return Mono.just(true);

Review Comment:
   Warning! A blocking call in a reactive pipeline!
   
   This will cause a major performance collapse.
   
   As a rule of thumbs blocking calls needs to be at the very least wrapped:
   
   ```
   Mono.fromRunnable(() -> blocking()).subscribeOn(elastic());
   ```
   
   (Which is better than nothing but still can spawn dozens of threads blocking to wait other reactor tasks to complete... So still wasteful!)
   
   Here we could very easily expose a `deleteReactive` method to not even be blocking!



##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,209 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, Optional.empty());
+
+        private final int usersPerSecond;
+
+        private final Optional<String> maxAge;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("maxAge") Optional<String> maxAge) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' needs to be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+    
+            this.maxAge = maxAge;
+            this.maxAgeDuration = maxAge.map(v -> {
+                Duration maxAgeDuration = DurationParser.parse(maxAge.get(), ChronoUnit.DAYS);
+                Preconditions.checkArgument(!maxAgeDuration.isNegative(), "'maxAge' must be positive");
+                return maxAgeDuration;
+            });
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+
+        public Optional<String> getMaxAge() {
+            return maxAge;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireInboxesService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireInboxesService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireInboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            LOGGER.info("expire with maxAge {} = {}", runningOptions.maxAge, runningOptions.maxAgeDuration);
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)
+                    )
+            );
+            return Iterators.toFlux(usersRepository.list())
+                .transform(ReactorUtils.<Username, Task.Result>throttle()
+                    .elements(runningOptions.getUsersPerSecond())
+                    .per(Duration.ofSeconds(1))
+                    .forOperation(username -> expireUserInbox(context, username, expiration)))
+                .reduce(Task.Result.COMPLETED, Task::combine);
+        } catch (UsersRepositoryException e) {
+            LOGGER.error("Error while accessing users from repository", e);
+            return Mono.just(Task.Result.PARTIAL);
+        }
+    }
+
+    private Mono<Result> expireUserInbox(Context context, Username username, SearchQuery expiration) {
+        MailboxSession session = mailboxManager.createSystemSession(username);
+        MailboxPath mailboxPath = MailboxPath.inbox(username);
+        return Mono.from(mailboxManager.getMailboxReactive(mailboxPath, session))
+            // newly created users do not have mailboxes yet, just skip them
+            .onErrorResume(MailboxNotFoundException.class, ignore -> Mono.empty())
+            .flatMap(mgr -> searchMessagesReactive(mgr, session, expiration)
+                .flatMap(list -> deleteMessagesReactive(mgr, session, list)))
+            .doOnNext(expired -> {
+                if (expired) {
+                    context.incrementExpiredCount();
+                }
+                context.incrementProcessedCount();
+            })
+            .then(Mono.just(Task.Result.COMPLETED))
+            .onErrorResume(e -> {
+                LOGGER.warn("Failed to expire user mailbox {}", username, e);
+                context.incrementFailedCount();
+                context.incrementProcessedCount();
+                return Mono.just(Task.Result.PARTIAL);
+            });
+    }
+
+    private Mono<List<MessageUid>> searchMessagesReactive(MessageManager mgr, MailboxSession session, SearchQuery expiration) {
+        try {
+            return Flux.from(mgr.search(expiration, session)).collectList();

Review Comment:
   Why do we need to collectList ?
   
   This seems like an uneeded costly overhead. Why not keep the Flux?



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875574560


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,209 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, Optional.empty());
+
+        private final int usersPerSecond;
+
+        private final Optional<String> maxAge;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("maxAge") Optional<String> maxAge) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' needs to be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+    
+            this.maxAge = maxAge;
+            this.maxAgeDuration = maxAge.map(v -> {
+                Duration maxAgeDuration = DurationParser.parse(maxAge.get(), ChronoUnit.DAYS);
+                Preconditions.checkArgument(!maxAgeDuration.isNegative(), "'maxAge' must be positive");
+                return maxAgeDuration;
+            });
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+
+        public Optional<String> getMaxAge() {
+            return maxAge;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireInboxesService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireInboxesService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireInboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            LOGGER.info("expire with maxAge {} = {}", runningOptions.maxAge, runningOptions.maxAgeDuration);
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)

Review Comment:
   I was not aware of this, agreed it should be a big CAVEAT in the documentation (to be written).



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r879415190


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,209 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, Optional.empty());
+
+        private final int usersPerSecond;
+
+        private final Optional<String> maxAge;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("maxAge") Optional<String> maxAge) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' needs to be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+    
+            this.maxAge = maxAge;
+            this.maxAgeDuration = maxAge.map(v -> {
+                Duration maxAgeDuration = DurationParser.parse(maxAge.get(), ChronoUnit.DAYS);
+                Preconditions.checkArgument(!maxAgeDuration.isNegative(), "'maxAge' must be positive");
+                return maxAgeDuration;
+            });
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+
+        public Optional<String> getMaxAge() {
+            return maxAge;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireInboxesService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireInboxesService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireInboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            LOGGER.info("expire with maxAge {} = {}", runningOptions.maxAge, runningOptions.maxAgeDuration);
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)
+                    )
+            );
+            return Iterators.toFlux(usersRepository.list())
+                .transform(ReactorUtils.<Username, Task.Result>throttle()
+                    .elements(runningOptions.getUsersPerSecond())
+                    .per(Duration.ofSeconds(1))
+                    .forOperation(username -> expireUserInbox(context, username, expiration)))

Review Comment:
    I thought about this some more, and decided to make the mailbox name configurable (default INBOX). So if necessary, someone might expire an "Archived" mailbox instead, or whatever the mail clients like to use. Simply use `&mailbox=Archived`



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r880227619


##########
mailet/standard/src/main/java/org/apache/james/transport/mailets/Expires.java:
##########
@@ -0,0 +1,144 @@
+/****************************************************************
+ * 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.transport.mailets;
+
+import java.io.StringReader;
+import java.time.Duration;
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.time.temporal.ChronoUnit;
+import java.util.Optional;
+import java.util.function.Supplier;
+
+import javax.mail.MessagingException;
+import javax.mail.internet.MimeMessage;
+
+import org.apache.james.mime4j.dom.datetime.DateTime;
+import org.apache.james.mime4j.field.datetime.parser.DateTimeParser;
+import org.apache.james.mime4j.field.datetime.parser.ParseException;
+import org.apache.james.util.DurationParser;
+import org.apache.mailet.Mail;
+import org.apache.mailet.base.DateFormats;
+import org.apache.mailet.base.GenericMailet;
+
+
+/**
+ * <p>Adds an Expires header to the message, or enforces the period of an existing one.</p>
+ *
+ * <p>Sample configuration:</p>
+ *
+ * <pre><code>
+ * &lt;mailet match="All" class="Expires"&gt;
+ *   &lt;minAge&gt;1d&lt;/minAge&gt;
+ *   &lt;maxAge&gt;1w&lt;/maxAge&gt;
+ *   &lt;defaultAge&gt;4w&lt;/defaultAge&gt;
+ * &lt;/mailet&gt;
+ * </code></pre>
+ *
+ * @version 1.0.0, 2021-12-14
+ */
+public class Expires extends GenericMailet {
+    
+    public static final String EXPIRES = "Expires";
+    
+    Supplier<ZonedDateTime> timeSource = ZonedDateTime::now; 
+
+    private Optional<Duration> minAge = Optional.empty();
+    private Optional<Duration> maxAge = Optional.empty();
+    private Optional<Duration> defaultAge = Optional.empty();
+    
+    @Override
+    public void init() throws MessagingException {
+        minAge = parseDuration("minAge");
+        maxAge = parseDuration("maxAge");
+        defaultAge = parseDuration("defaultAge");
+
+        if (minAge.isEmpty() && maxAge.isEmpty() && defaultAge.isEmpty()) {
+            throw new MessagingException("Please configure at least one of minAge, maxAge, defaultAge");
+        }
+
+        if (isAfter(minAge, maxAge)) {
+            throw new MessagingException("minAge must be before maxAge");
+        }
+        if (isAfter(defaultAge, maxAge)) {
+            throw new MessagingException("defaultAge must be before maxAge");
+        }
+        if (isAfter(minAge, defaultAge)) {
+            throw new MessagingException("minAge must be before defaultAge");
+        }
+    }
+
+    @Override
+    public void service(Mail mail) throws MessagingException {
+        ZonedDateTime now = timeSource.get();
+        MimeMessage message = mail.getMessage();
+        Optional<ZonedDateTime> expires = parseExpiresHeader(message);
+        if (expires.isPresent()) {
+            if (minAge.isPresent() && expires.get().isBefore(now.plus(minAge.get()))) {
+                setExpiresAfter(message, now, minAge.get());
+            } else
+            if (maxAge.isPresent() && expires.get().isAfter(now.plus(maxAge.get()))) {
+                setExpiresAfter(message, now, maxAge.get());
+            }
+        } else if (defaultAge.isPresent()) {
+            setExpiresAfter(message, now, defaultAge.get());
+        }
+    }
+
+    @Override
+    public String getMailetInfo() {
+        return "Expire Mailet";
+    }
+
+    private Optional<Duration> parseDuration(String param) {
+        String duration = getInitParameter(param);
+        if (duration == null) {
+            return Optional.empty();
+        } else {
+            return Optional.of(DurationParser.parse(duration, ChronoUnit.DAYS));
+        }

Review Comment:
   Indeed, and quite obvious, now that you point it out. :D



-- 
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 #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875783236


##########
server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/service/ExpireInboxesServiceTest.java:
##########
@@ -0,0 +1,219 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.task.Task;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyList;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;

Review Comment:
   You could split the tests: those that needs mock (and for wich we will need to pay maintenance cost on the long run) and the others that are OK with in memory implementation.
   
   The testing mantra being my hate of mock: test what is being done, not how it is being done.



-- 
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 #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875574808


##########
mailet/standard/src/main/java/org/apache/james/transport/mailets/Expires.java:
##########
@@ -0,0 +1,144 @@
+/****************************************************************
+ * 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.transport.mailets;
+
+import java.io.StringReader;
+import java.time.Duration;
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.time.temporal.ChronoUnit;
+import java.util.Optional;
+import java.util.function.Supplier;
+
+import javax.mail.MessagingException;
+import javax.mail.internet.MimeMessage;
+
+import org.apache.james.mime4j.dom.datetime.DateTime;
+import org.apache.james.mime4j.field.datetime.parser.DateTimeParser;
+import org.apache.james.mime4j.field.datetime.parser.ParseException;
+import org.apache.james.util.DurationParser;
+import org.apache.mailet.Mail;
+import org.apache.mailet.base.DateFormats;
+import org.apache.mailet.base.GenericMailet;
+
+
+/**
+ * <p>Adds an Expires header to the message, or enforces the period of an existing one.</p>
+ *
+ * <p>Sample configuration:</p>
+ *
+ * <pre><code>
+ * &lt;mailet match="All" class="Expires"&gt;
+ *   &lt;minAge&gt;1d&lt;/minAge&gt;
+ *   &lt;maxAge&gt;1w&lt;/maxAge&gt;
+ *   &lt;defaultAge&gt;4w&lt;/defaultAge&gt;
+ * &lt;/mailet&gt;
+ * </code></pre>
+ *
+ * @version 1.0.0, 2021-12-14
+ */
+public class Expires extends GenericMailet {
+    
+    public static final String EXPIRES = "Expires";
+    
+    Supplier<ZonedDateTime> timeSource = ZonedDateTime::now; 
+
+    private Optional<Duration> minAge = Optional.empty();
+    private Optional<Duration> maxAge = Optional.empty();
+    private Optional<Duration> defaultAge = Optional.empty();
+    
+    @Override
+    public void init() throws MessagingException {
+        minAge = parseDuration("minAge");
+        maxAge = parseDuration("maxAge");
+        defaultAge = parseDuration("defaultAge");
+
+        if (minAge.isEmpty() && maxAge.isEmpty() && defaultAge.isEmpty()) {
+            throw new MessagingException("Please configure at least one of minAge, maxAge, defaultAge");
+        }
+
+        if (isAfter(minAge, maxAge)) {
+            throw new MessagingException("minAge must be before maxAge");
+        }
+        if (isAfter(defaultAge, maxAge)) {
+            throw new MessagingException("defaultAge must be before maxAge");
+        }
+        if (isAfter(minAge, defaultAge)) {
+            throw new MessagingException("minAge must be before defaultAge");
+        }
+    }
+
+    @Override
+    public void service(Mail mail) throws MessagingException {
+        ZonedDateTime now = timeSource.get();
+        MimeMessage message = mail.getMessage();
+        Optional<ZonedDateTime> expires = parseExpiresHeader(message);
+        if (expires.isPresent()) {
+            if (minAge.isPresent() && expires.get().isBefore(now.plus(minAge.get()))) {
+                setExpiresAfter(message, now, minAge.get());
+            } else
+            if (maxAge.isPresent() && expires.get().isAfter(now.plus(maxAge.get()))) {
+                setExpiresAfter(message, now, maxAge.get());
+            }
+        } else if (defaultAge.isPresent()) {
+            setExpiresAfter(message, now, defaultAge.get());
+        }
+    }
+
+    @Override
+    public String getMailetInfo() {
+        return "Expire Mailet";
+    }
+
+    private Optional<Duration> parseDuration(String param) {
+        String duration = getInitParameter(param);
+        if (duration == null) {
+            return Optional.empty();
+        } else {
+            return Optional.of(DurationParser.parse(duration, ChronoUnit.DAYS));
+        }
+    }
+    
+    private boolean isAfter(Optional<Duration> a, Optional<Duration> b) {
+        return a.isPresent() && b.isPresent() && a.get().compareTo(b.get()) > 0;
+    }
+    
+    private Optional<ZonedDateTime> parseExpiresHeader(MimeMessage message) {
+        try {
+            String[] expires = message.getHeader(EXPIRES);
+            if (expires == null || expires.length == 0) {
+                return Optional.empty();
+            } else {
+                DateTime dt = new DateTimeParser(new StringReader(expires[0])).parseAll();
+                return Optional.of(ZonedDateTime.of(
+                    dt.getYear(), dt.getMonth(), dt.getDay(),
+                    dt.getHour(), dt.getMinute(), dt.getSecond(), 0,
+                    ZoneOffset.ofHoursMinutes(dt.getTimeZone() / 100, dt.getTimeZone() % 100)));

Review Comment:
   No I did open a PR in MIME4J since then to bring it under test. We should be good with it IMO
   https://github.com/apache/james-mime4j/pull/70
   
   Maybe the offset part of the story could be made less hacky though...
   



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

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

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


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


[GitHub] [james-project] chibenwa commented on pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1004:
URL: https://github.com/apache/james-project/pull/1004#issuecomment-1129016609

   As a first feedback the API could be maybe improved...
   
   As a common practice we tend in the project to lean toward `DELETE` verb when deleting stuff, even when such deletion is done as a task. Maybe we could stick to this convention here too.
   
   The entity that we delete are messages, not mailboxes. I would say that the fact that we iterate on mailboxes to actually delete messages is an implementation detail that maybe should not leak to the API.
   
   Finally I am unhappy with the `task` name. I could hardly find a meaningfull task name. `cleanup` maybe. Or the concept of `retention` or `purge`. But maybe not even needing to name it would be better.
   
   I think `DELETE /messages?olderThan=30days` / `DELETE /messages?usingExpiresHeader` might be cleaner.
   
   `BlobRoutes` shows a related exemple of how to set up such a DELETE route without requiring `task` parameter.
   
   For a deeper look, I will need to put together a review of 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] ottoka commented on pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on PR #1004:
URL: https://github.com/apache/james-project/pull/1004#issuecomment-1129646543

   Thanks for the feedback. I don't mind changing the call to DELETE based on your suggestion, will take a look at the example.
   The failed test is likely related to the MessageSearches patch, but maybe it is supposed to ignore the zone offset? I will check.


-- 
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 #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875658831


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,209 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, Optional.empty());
+
+        private final int usersPerSecond;
+
+        private final Optional<String> maxAge;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("maxAge") Optional<String> maxAge) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' needs to be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+    
+            this.maxAge = maxAge;
+            this.maxAgeDuration = maxAge.map(v -> {
+                Duration maxAgeDuration = DurationParser.parse(maxAge.get(), ChronoUnit.DAYS);
+                Preconditions.checkArgument(!maxAgeDuration.isNegative(), "'maxAge' must be positive");
+                return maxAgeDuration;
+            });
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+
+        public Optional<String> getMaxAge() {
+            return maxAge;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireInboxesService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireInboxesService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireInboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            LOGGER.info("expire with maxAge {} = {}", runningOptions.maxAge, runningOptions.maxAgeDuration);
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)
+                    )
+            );
+            return Iterators.toFlux(usersRepository.list())
+                .transform(ReactorUtils.<Username, Task.Result>throttle()
+                    .elements(runningOptions.getUsersPerSecond())
+                    .per(Duration.ofSeconds(1))
+                    .forOperation(username -> expireUserInbox(context, username, expiration)))

Review Comment:
   The use case you describe makes sens.
   
   If other use case arise we could easily add options at that time if needed. That's fine.



-- 
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 #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875781690


##########
mailet/standard/src/main/java/org/apache/james/transport/mailets/Expires.java:
##########
@@ -0,0 +1,144 @@
+/****************************************************************
+ * 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.transport.mailets;
+
+import java.io.StringReader;
+import java.time.Duration;
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.time.temporal.ChronoUnit;
+import java.util.Optional;
+import java.util.function.Supplier;
+
+import javax.mail.MessagingException;
+import javax.mail.internet.MimeMessage;
+
+import org.apache.james.mime4j.dom.datetime.DateTime;
+import org.apache.james.mime4j.field.datetime.parser.DateTimeParser;
+import org.apache.james.mime4j.field.datetime.parser.ParseException;
+import org.apache.james.util.DurationParser;
+import org.apache.mailet.Mail;
+import org.apache.mailet.base.DateFormats;
+import org.apache.mailet.base.GenericMailet;
+
+
+/**
+ * <p>Adds an Expires header to the message, or enforces the period of an existing one.</p>
+ *
+ * <p>Sample configuration:</p>
+ *
+ * <pre><code>
+ * &lt;mailet match="All" class="Expires"&gt;
+ *   &lt;minAge&gt;1d&lt;/minAge&gt;
+ *   &lt;maxAge&gt;1w&lt;/maxAge&gt;
+ *   &lt;defaultAge&gt;4w&lt;/defaultAge&gt;
+ * &lt;/mailet&gt;
+ * </code></pre>
+ *
+ * @version 1.0.0, 2021-12-14
+ */
+public class Expires extends GenericMailet {
+    
+    public static final String EXPIRES = "Expires";
+    
+    Supplier<ZonedDateTime> timeSource = ZonedDateTime::now; 
+
+    private Optional<Duration> minAge = Optional.empty();
+    private Optional<Duration> maxAge = Optional.empty();
+    private Optional<Duration> defaultAge = Optional.empty();
+    
+    @Override
+    public void init() throws MessagingException {
+        minAge = parseDuration("minAge");
+        maxAge = parseDuration("maxAge");
+        defaultAge = parseDuration("defaultAge");
+
+        if (minAge.isEmpty() && maxAge.isEmpty() && defaultAge.isEmpty()) {
+            throw new MessagingException("Please configure at least one of minAge, maxAge, defaultAge");
+        }
+
+        if (isAfter(minAge, maxAge)) {
+            throw new MessagingException("minAge must be before maxAge");
+        }
+        if (isAfter(defaultAge, maxAge)) {
+            throw new MessagingException("defaultAge must be before maxAge");
+        }
+        if (isAfter(minAge, defaultAge)) {
+            throw new MessagingException("minAge must be before defaultAge");
+        }
+    }
+
+    @Override
+    public void service(Mail mail) throws MessagingException {
+        ZonedDateTime now = timeSource.get();
+        MimeMessage message = mail.getMessage();
+        Optional<ZonedDateTime> expires = parseExpiresHeader(message);
+        if (expires.isPresent()) {
+            if (minAge.isPresent() && expires.get().isBefore(now.plus(minAge.get()))) {
+                setExpiresAfter(message, now, minAge.get());
+            } else
+            if (maxAge.isPresent() && expires.get().isAfter(now.plus(maxAge.get()))) {
+                setExpiresAfter(message, now, maxAge.get());
+            }
+        } else if (defaultAge.isPresent()) {
+            setExpiresAfter(message, now, defaultAge.get());
+        }
+    }
+
+    @Override
+    public String getMailetInfo() {
+        return "Expire Mailet";
+    }
+
+    private Optional<Duration> parseDuration(String param) {
+        String duration = getInitParameter(param);
+        if (duration == null) {
+            return Optional.empty();
+        } else {
+            return Optional.of(DurationParser.parse(duration, ChronoUnit.DAYS));
+        }
+    }
+    
+    private boolean isAfter(Optional<Duration> a, Optional<Duration> b) {
+        return a.isPresent() && b.isPresent() && a.get().compareTo(b.get()) > 0;
+    }
+    
+    private Optional<ZonedDateTime> parseExpiresHeader(MimeMessage message) {
+        try {
+            String[] expires = message.getHeader(EXPIRES);
+            if (expires == null || expires.length == 0) {
+                return Optional.empty();
+            } else {
+                DateTime dt = new DateTimeParser(new StringReader(expires[0])).parseAll();
+                return Optional.of(ZonedDateTime.of(
+                    dt.getYear(), dt.getMonth(), dt.getDay(),
+                    dt.getHour(), dt.getMinute(), dt.getSecond(), 0,
+                    ZoneOffset.ofHoursMinutes(dt.getTimeZone() / 100, dt.getTimeZone() % 100)));

Review Comment:
   Fair



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875605597


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesTask.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.webadmin.service;
+
+import java.time.Clock;
+import java.time.Instant;
+import java.util.Date;
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.james.task.Task;
+import org.apache.james.task.TaskExecutionDetails;
+import org.apache.james.task.TaskType;
+import org.apache.james.webadmin.service.ExpireInboxesService.Context;
+import org.apache.james.webadmin.service.ExpireInboxesService.RunningOptions;
+
+import reactor.core.scheduler.Schedulers;
+
+public class ExpireInboxesTask implements Task {
+    public static final TaskType TASK_TYPE = TaskType.of("ExpireInboxesTask");
+
+    public static class AdditionalInformation implements TaskExecutionDetails.AdditionalInformation {
+
+        public static ExpireInboxesTask.AdditionalInformation from(Context context,
+                                                                   RunningOptions runningOptions) {
+            return new ExpireInboxesTask.AdditionalInformation(
+                Clock.systemUTC().instant(),
+                runningOptions,
+                context.getInboxesExpired(),
+                context.getInboxesFailed(),
+                context.getInboxesProcessed());
+        }
+
+        private final Instant timestamp;
+        private final RunningOptions runningOptions;
+        private final long inboxesExpired;
+        private final long inboxesFailed;
+        private final long inboxesProcessed;
+
+        public AdditionalInformation(Instant timestamp,
+                                     RunningOptions runningOptions,
+                                     long inboxesExpired,
+                                     long inboxesFailed,
+                                     long inboxesProcessed) {
+            this.timestamp = timestamp;
+            this.runningOptions = runningOptions;
+            this.inboxesExpired = inboxesExpired;
+            this.inboxesFailed = inboxesFailed;
+            this.inboxesProcessed = inboxesProcessed;
+        }
+
+        public RunningOptions getRunningOptions() {
+            return runningOptions;
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired;
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed;
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed;
+        }
+
+        @Override
+        public Instant timestamp() {
+            return timestamp;
+        }
+    }
+
+    private final ExpireInboxesService expireInboxesService;
+    private final Context context;
+    private final RunningOptions runningOptions;
+
+    @Inject
+    public ExpireInboxesTask(ExpireInboxesService expireInboxesService,
+                             RunningOptions runningOptions) {
+        this.expireInboxesService = expireInboxesService;
+        this.context = new Context();
+        this.runningOptions = runningOptions;
+    }
+
+    @Override
+    public Result run() {
+        return expireInboxesService.expireInboxes(context, runningOptions, new Date())
+            .subscribeOn(Schedulers.elastic())

Review Comment:
   Ok, that is an easy fix.



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875607900


##########
server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/service/ExpireInboxesServiceTest.java:
##########
@@ -0,0 +1,219 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.task.Task;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyList;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;

Review Comment:
   I did not encounter InMemoryTestRessource before, I will change the test accordingly.



-- 
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 #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
vttranlina commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r880134596


##########
mailet/standard/src/main/java/org/apache/james/transport/mailets/Expires.java:
##########
@@ -0,0 +1,144 @@
+/****************************************************************
+ * 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.transport.mailets;
+
+import java.io.StringReader;
+import java.time.Duration;
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.time.temporal.ChronoUnit;
+import java.util.Optional;
+import java.util.function.Supplier;
+
+import javax.mail.MessagingException;
+import javax.mail.internet.MimeMessage;
+
+import org.apache.james.mime4j.dom.datetime.DateTime;
+import org.apache.james.mime4j.field.datetime.parser.DateTimeParser;
+import org.apache.james.mime4j.field.datetime.parser.ParseException;
+import org.apache.james.util.DurationParser;
+import org.apache.mailet.Mail;
+import org.apache.mailet.base.DateFormats;
+import org.apache.mailet.base.GenericMailet;
+
+
+/**
+ * <p>Adds an Expires header to the message, or enforces the period of an existing one.</p>
+ *
+ * <p>Sample configuration:</p>
+ *
+ * <pre><code>
+ * &lt;mailet match="All" class="Expires"&gt;
+ *   &lt;minAge&gt;1d&lt;/minAge&gt;
+ *   &lt;maxAge&gt;1w&lt;/maxAge&gt;
+ *   &lt;defaultAge&gt;4w&lt;/defaultAge&gt;
+ * &lt;/mailet&gt;
+ * </code></pre>
+ *
+ * @version 1.0.0, 2021-12-14
+ */
+public class Expires extends GenericMailet {
+    
+    public static final String EXPIRES = "Expires";
+    
+    Supplier<ZonedDateTime> timeSource = ZonedDateTime::now; 
+
+    private Optional<Duration> minAge = Optional.empty();
+    private Optional<Duration> maxAge = Optional.empty();
+    private Optional<Duration> defaultAge = Optional.empty();
+    
+    @Override
+    public void init() throws MessagingException {
+        minAge = parseDuration("minAge");
+        maxAge = parseDuration("maxAge");
+        defaultAge = parseDuration("defaultAge");
+
+        if (minAge.isEmpty() && maxAge.isEmpty() && defaultAge.isEmpty()) {
+            throw new MessagingException("Please configure at least one of minAge, maxAge, defaultAge");
+        }
+
+        if (isAfter(minAge, maxAge)) {
+            throw new MessagingException("minAge must be before maxAge");
+        }
+        if (isAfter(defaultAge, maxAge)) {
+            throw new MessagingException("defaultAge must be before maxAge");
+        }
+        if (isAfter(minAge, defaultAge)) {
+            throw new MessagingException("minAge must be before defaultAge");
+        }
+    }
+
+    @Override
+    public void service(Mail mail) throws MessagingException {
+        ZonedDateTime now = timeSource.get();
+        MimeMessage message = mail.getMessage();
+        Optional<ZonedDateTime> expires = parseExpiresHeader(message);
+        if (expires.isPresent()) {
+            if (minAge.isPresent() && expires.get().isBefore(now.plus(minAge.get()))) {
+                setExpiresAfter(message, now, minAge.get());
+            } else
+            if (maxAge.isPresent() && expires.get().isAfter(now.plus(maxAge.get()))) {
+                setExpiresAfter(message, now, maxAge.get());
+            }
+        } else if (defaultAge.isPresent()) {
+            setExpiresAfter(message, now, defaultAge.get());
+        }
+    }
+
+    @Override
+    public String getMailetInfo() {
+        return "Expire Mailet";
+    }
+
+    private Optional<Duration> parseDuration(String param) {
+        String duration = getInitParameter(param);
+        if (duration == null) {
+            return Optional.empty();
+        } else {
+            return Optional.of(DurationParser.parse(duration, ChronoUnit.DAYS));
+        }

Review Comment:
   ```suggestion
   return Optional.ofNullable(getInitParameter(param))
               .map(duration -> DurationParser.parse(duration, ChronoUnit.DAYS));
   ```
   more beauty :D 
   



##########
mailet/standard/src/main/java/org/apache/james/transport/mailets/Expires.java:
##########
@@ -0,0 +1,144 @@
+/****************************************************************
+ * 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.transport.mailets;
+
+import java.io.StringReader;
+import java.time.Duration;
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.time.temporal.ChronoUnit;
+import java.util.Optional;
+import java.util.function.Supplier;
+
+import javax.mail.MessagingException;
+import javax.mail.internet.MimeMessage;
+
+import org.apache.james.mime4j.dom.datetime.DateTime;
+import org.apache.james.mime4j.field.datetime.parser.DateTimeParser;
+import org.apache.james.mime4j.field.datetime.parser.ParseException;
+import org.apache.james.util.DurationParser;
+import org.apache.mailet.Mail;
+import org.apache.mailet.base.DateFormats;
+import org.apache.mailet.base.GenericMailet;
+
+
+/**
+ * <p>Adds an Expires header to the message, or enforces the period of an existing one.</p>
+ *
+ * <p>Sample configuration:</p>
+ *
+ * <pre><code>
+ * &lt;mailet match="All" class="Expires"&gt;
+ *   &lt;minAge&gt;1d&lt;/minAge&gt;
+ *   &lt;maxAge&gt;1w&lt;/maxAge&gt;
+ *   &lt;defaultAge&gt;4w&lt;/defaultAge&gt;
+ * &lt;/mailet&gt;
+ * </code></pre>
+ *
+ * @version 1.0.0, 2021-12-14
+ */
+public class Expires extends GenericMailet {
+    
+    public static final String EXPIRES = "Expires";
+    
+    Supplier<ZonedDateTime> timeSource = ZonedDateTime::now; 

Review Comment:
   Should we use `clock`  here ? `() -> ZonedDateTime.now(clock)`
   // We already have the Clock guice bean



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r880201000


##########
server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc:
##########
@@ -1206,6 +1206,51 @@ This task could be run safely online and can be scheduled on a recurring
 basis outside of peak traffic by an admin to ensure Cassandra message
 consistency.
 
+=== Deleting old messages of all users
+
+*Note:*
+Consider enabling the xref:configure/vault.adoc[Deleted Messages Vault]
+if you use this feature.
+
+Old messages tend to pile up in user INBOXes. An admin might want to delete
+these on behalf of the users, e.g. all messages older than 30 days:
+....
+curl -XDELETE http://ip:port/messages?olderThan=30d
+....
+
+link:#_endpoints_returning_a_task[More details about endpoints returning a task].
+
+The `olderThan` parameter should be expressed in the following format: `Nunit`.
+`N` should be strictly positive. `unit` could be either in the short form
+(`d`, `w`, `y` etc.), or in the long form (`days`, `weeks`, `months`, `years`).
+The default unit is `days`.
+
+Response codes:
+
+* 201: Success. Corresponding task id is returned.
+* 400: Error in the request. Details can be found in the reported error.
+
+To delete old mails from a different mailbox than INBOX, e.g. a mailbox
+named "Archived" :
+....
+curl -XDELETE http://ip:port/messages?mailbox=Archived&olderThan=30d
+....
+
+Since this is a somewhat expensive operation, the task is throttled to one user
+per second. You may speed it up via `usersPerSecond=10` for example. But keep
+in mind that a high rate might overwhelm your database or blob store.
+
+*Cassandra only:*

Review Comment:
   Right, that was our special setup. Makes sense this way.



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r879110112


##########
server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/MessageRoutesExpireTest.java:
##########
@@ -0,0 +1,284 @@
+/****************************************************************
+ * 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.webadmin.routes;
+
+import com.google.common.collect.ImmutableSet;
+import io.restassured.RestAssured;
+import org.apache.james.core.Username;
+import org.apache.james.domainlist.api.DomainList;
+import org.apache.james.json.DTOConverter;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.inmemory.InMemoryMailboxManager;
+import org.apache.james.mailbox.inmemory.InMemoryMessageId;
+import org.apache.james.mailbox.inmemory.manager.InMemoryIntegrationResources;
+import org.apache.james.mailbox.model.ComposedMessageId;
+import org.apache.james.mailbox.model.Mailbox;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.store.mail.model.MailboxMessage;
+import org.apache.james.mailbox.store.search.ListeningMessageSearchIndex;
+import org.apache.james.task.Hostname;
+import org.apache.james.task.MemoryTaskManager;
+import org.apache.james.user.memory.MemoryUsersRepository;
+import org.apache.james.webadmin.WebAdminServer;
+import org.apache.james.webadmin.WebAdminUtils;
+import org.apache.james.webadmin.service.ExpireInboxesAdditionalInformationDTO;
+import org.apache.james.webadmin.service.ExpireInboxesService;
+import org.apache.james.webadmin.service.ExpireInboxesService.RunningOptions;
+import org.apache.james.webadmin.utils.ErrorResponder;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.apache.mailbox.tools.indexer.MessageIdReIndexerImpl;
+import org.apache.mailbox.tools.indexer.MessageIdReIndexingTask;
+import org.apache.mailbox.tools.indexer.MessageIdReindexingTaskAdditionalInformationDTO;
+import org.apache.mailbox.tools.indexer.ReIndexerPerformer;
+import org.eclipse.jetty.http.HttpStatus;
+import org.hamcrest.Matchers;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mockito;
+import reactor.core.publisher.Mono;
+
+import java.util.Date;
+import java.util.Optional;
+
+import static io.restassured.RestAssured.given;
+import static io.restassured.RestAssured.when;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+
+class MessageRoutesExpireTest {
+    private static final DomainList NO_DOMAIN_LIST = null;
+    private static final Username USERNAME = Username.of("benwa");
+    private static final MailboxPath INBOX = MailboxPath.inbox(USERNAME);
+
+    private WebAdminServer webAdminServer;
+    private InMemoryMailboxManager mailboxManager;
+    private MemoryTaskManager taskManager;
+    private MemoryUsersRepository usersRepository;
+
+    @BeforeEach
+    void beforeEach() {
+        taskManager = new MemoryTaskManager(new Hostname("foo"));
+        mailboxManager = InMemoryIntegrationResources.defaultResources().getMailboxManager();
+        usersRepository = MemoryUsersRepository.withoutVirtualHosting(NO_DOMAIN_LIST);
+        JsonTransformer jsonTransformer = new JsonTransformer();
+
+        webAdminServer = WebAdminUtils.createWebAdminServer(
+                new TasksRoutes(taskManager, jsonTransformer,
+                    DTOConverter.of(
+                        ExpireInboxesAdditionalInformationDTO.module())),
+                new MessagesRoutes(taskManager,
+                    new InMemoryMessageId.Factory(),
+                    null,
+                    new ExpireInboxesService(usersRepository, mailboxManager),
+                    jsonTransformer,
+                    ImmutableSet.of()))
+            .start();
+
+        RestAssured.requestSpecification = WebAdminUtils.buildRequestSpecification(webAdminServer).build();
+        RestAssured.enableLoggingOfRequestAndResponseIfValidationFails();
+    }
+
+    @AfterEach
+    void tearDown() {
+        webAdminServer.destroy();
+        taskManager.stop();
+    }
+
+    @Nested
+    class ExpireInboxes {
+        @Nested
+        class Validation {
+            @Test
+            void expireInboxesShouldFailWithNoOption() {
+                when()
+                    .delete("/messages")
+                .then()
+                    .statusCode(HttpStatus.BAD_REQUEST_400)
+                    .body("statusCode", is(400))
+                    .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("Must specify either 'olderThan' or 'byExpiresHeader' parameter"));
+            }
+
+            @Test
+            void expireInboxesShouldFailWithBothOptions() {
+                when()
+                    .delete("/messages?byExpiresHeader&olderThan=30d")
+                .then()
+                    .statusCode(HttpStatus.BAD_REQUEST_400)
+                    .body("statusCode", is(400))
+                    .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("Must specify either 'olderThan' or 'byExpiresHeader' parameter"));
+            }
+
+            @Test
+            void expireInboxesShouldFailWithBadOlderThan() {
+                when()
+                    .delete("/messages?olderThan=bad")
+                    .then()
+                    .statusCode(HttpStatus.BAD_REQUEST_400)
+                    .body("statusCode", is(400))
+                    .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("'usersPerSecond' must be numeric"));
+            }
+
+            @Test
+            void expireInboxesShouldFailWithNegativeOlderThan() {
+                when()
+                    .delete("/messages?olderThan=-30d")
+                    .then()
+                    .statusCode(HttpStatus.BAD_REQUEST_400)
+                    .body("statusCode", is(400))
+                    .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("Duration amount should be positive"));
+            }
+
+            @Test
+            void expireInboxesShouldFailWithBadUsersPerSeconds() {
+                when()
+                    .delete("/messages?byExpiresHeader&usersPerSecond=bad")
+                    .then()
+                    .statusCode(HttpStatus.BAD_REQUEST_400)
+                    .body("statusCode", is(400))
+                    .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("'usersPerSecond' must be numeric"));
+            }
+
+            @Test
+            void expireInboxesShouldFailWithNegativeUsersPerSeconds() {
+                when()
+                    .delete("/messages?byExpiresHeader&usersPerSecond=-10")
+                    .then()
+                    .statusCode(HttpStatus.BAD_REQUEST_400)
+                    .body("statusCode", is(400))
+                    .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("'usersPerSecond' must be strictly positive"));
+            }
+
+        }
+
+        @Nested
+        class TaskDetails {
+            @Test
+            void expireInboxesShouldNotFailWhenNoMailsFound() {
+                String taskId = when()
+                    .delete("/messages?byExpiresHeader")
+                    .jsonPath()
+                    .get("taskId");
+
+                given()
+                    .basePath(TasksRoutes.BASE)
+                .when()
+                    .get(taskId + "/await")
+                .then()
+                    .body("type", Matchers.is("ExpireInboxesTask"))
+                    .body("status", is("completed"))
+                    .body("taskId", is(notNullValue()))
+                    .body("additionalInformation.type", is("ExpireInboxesTask"))
+                    .body("additionalInformation.runningOptions.usersPerSecond", is(RunningOptions.DEFAULT.getUsersPerSecond()))
+                    .body("additionalInformation.runningOptions.byExpiresHeader", is(true))
+                    .body("additionalInformation.runningOptions.olderThan", is(nullValue()))
+                    .body("additionalInformation.inboxesExpired", is(0))
+                    .body("additionalInformation.inboxesFailed", is(0))
+                    .body("additionalInformation.inboxesProcessed", is(0))
+                    .body("additionalInformation.messagesDeleted", is(0));
+            }
+
+            @Test
+            void expireInboxesShouldReturnTaskDetailsWhenMail() throws Exception {
+                usersRepository.addUser(USERNAME, "secret");
+                MailboxSession systemSession = mailboxManager.createSystemSession(USERNAME);
+                mailboxManager.createMailbox(INBOX, systemSession).get();
+                mailboxManager.getMailbox(INBOX, systemSession).appendMessage(
+                    MessageManager.AppendCommand.builder()
+                        .withInternalDate(new Date(System.currentTimeMillis() - 5000))
+                        .build("header: value\r\n\r\nbody"),
+                    systemSession).getId();
+
+                String taskId = when()
+                    .delete("/messages?olderThan=1s")
+                    .jsonPath()
+                    .get("taskId");
+
+                given()
+                    .basePath(TasksRoutes.BASE)
+                .when()
+                    .get(taskId + "/await")
+                .then()
+                    .body("type", Matchers.is("ExpireInboxesTask"))
+                    .body("status", is("completed"))
+                    .body("taskId", is(notNullValue()))
+                    .body("additionalInformation.type", is("ExpireInboxesTask"))
+                    .body("additionalInformation.runningOptions.usersPerSecond", is(RunningOptions.DEFAULT.getUsersPerSecond()))
+                    .body("additionalInformation.runningOptions.byExpiresHeader", is(false))
+                    .body("additionalInformation.runningOptions.olderThan", is("1s"))
+                    .body("additionalInformation.inboxesExpired", is(1))
+                    .body("additionalInformation.inboxesFailed", is(0))
+                    .body("additionalInformation.inboxesProcessed", is(1))
+                    .body("additionalInformation.messagesDeleted", is(1));
+            }

Review Comment:
   Er, right. This is kind of obvious, how could I have missed it oO



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r880155136


##########
server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc:
##########
@@ -1206,6 +1206,51 @@ This task could be run safely online and can be scheduled on a recurring
 basis outside of peak traffic by an admin to ensure Cassandra message
 consistency.
 
+=== Deleting old messages of all users
+
+*Note:*
+Consider enabling the xref:configure/vault.adoc[Deleted Messages Vault]
+if you use this feature.
+
+Old messages tend to pile up in user INBOXes. An admin might want to delete
+these on behalf of the users, e.g. all messages older than 30 days:
+....
+curl -XDELETE http://ip:port/messages?olderThan=30d
+....
+
+link:#_endpoints_returning_a_task[More details about endpoints returning a task].
+
+The `olderThan` parameter should be expressed in the following format: `Nunit`.
+`N` should be strictly positive. `unit` could be either in the short form
+(`d`, `w`, `y` etc.), or in the long form (`days`, `weeks`, `months`, `years`).
+The default unit is `days`.
+
+Response codes:
+
+* 201: Success. Corresponding task id is returned.
+* 400: Error in the request. Details can be found in the reported error.
+
+To delete old mails from a different mailbox than INBOX, e.g. a mailbox
+named "Archived" :
+....
+curl -XDELETE http://ip:port/messages?mailbox=Archived&olderThan=30d
+....
+
+Since this is a somewhat expensive operation, the task is throttled to one user
+per second. You may speed it up via `usersPerSecond=10` for example. But keep
+in mind that a high rate might overwhelm your database or blob store.
+
+*Cassandra only:*

Review Comment:
   This is correct from a technical point of view, but will the average user understand its implications? Even I am not sure which James variants use "scanning search", I only know Cassandra works because I tested it. Could you tell me which other variants will work too?
   Maybe "Scanning search only (e.g. Cassandra)" ?



-- 
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 #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r877688224


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,238 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.github.fge.lambdas.Throwing;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+import reactor.core.scheduler.Schedulers;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, true, Optional.empty());
+
+        public static RunningOptions fromParams(
+                Optional<String> byExpiresHeader, Optional<String> olderThan, Optional<String> usersPerSecond) {
+            try {
+                if ((byExpiresHeader.isEmpty() && olderThan.isEmpty()) || (byExpiresHeader.isPresent() && olderThan.isPresent())) {

Review Comment:
   `Booleans.countTrue(byExpiresHeader.isEmpty(), olderThan.isEmpty()) == 1 `
   
   
   A XOR is also doing the job.
   
   This will make this boolean condition more readable.



-- 
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 #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
Arsnael commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r877826226


##########
server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/routes/MessageRoutesExpireTest.java:
##########
@@ -0,0 +1,284 @@
+/****************************************************************
+ * 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.webadmin.routes;
+
+import com.google.common.collect.ImmutableSet;
+import io.restassured.RestAssured;
+import org.apache.james.core.Username;
+import org.apache.james.domainlist.api.DomainList;
+import org.apache.james.json.DTOConverter;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.inmemory.InMemoryMailboxManager;
+import org.apache.james.mailbox.inmemory.InMemoryMessageId;
+import org.apache.james.mailbox.inmemory.manager.InMemoryIntegrationResources;
+import org.apache.james.mailbox.model.ComposedMessageId;
+import org.apache.james.mailbox.model.Mailbox;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.store.mail.model.MailboxMessage;
+import org.apache.james.mailbox.store.search.ListeningMessageSearchIndex;
+import org.apache.james.task.Hostname;
+import org.apache.james.task.MemoryTaskManager;
+import org.apache.james.user.memory.MemoryUsersRepository;
+import org.apache.james.webadmin.WebAdminServer;
+import org.apache.james.webadmin.WebAdminUtils;
+import org.apache.james.webadmin.service.ExpireInboxesAdditionalInformationDTO;
+import org.apache.james.webadmin.service.ExpireInboxesService;
+import org.apache.james.webadmin.service.ExpireInboxesService.RunningOptions;
+import org.apache.james.webadmin.utils.ErrorResponder;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.apache.mailbox.tools.indexer.MessageIdReIndexerImpl;
+import org.apache.mailbox.tools.indexer.MessageIdReIndexingTask;
+import org.apache.mailbox.tools.indexer.MessageIdReindexingTaskAdditionalInformationDTO;
+import org.apache.mailbox.tools.indexer.ReIndexerPerformer;
+import org.eclipse.jetty.http.HttpStatus;
+import org.hamcrest.Matchers;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mockito;
+import reactor.core.publisher.Mono;
+
+import java.util.Date;
+import java.util.Optional;
+
+import static io.restassured.RestAssured.given;
+import static io.restassured.RestAssured.when;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+
+class MessageRoutesExpireTest {
+    private static final DomainList NO_DOMAIN_LIST = null;
+    private static final Username USERNAME = Username.of("benwa");
+    private static final MailboxPath INBOX = MailboxPath.inbox(USERNAME);
+
+    private WebAdminServer webAdminServer;
+    private InMemoryMailboxManager mailboxManager;
+    private MemoryTaskManager taskManager;
+    private MemoryUsersRepository usersRepository;
+
+    @BeforeEach
+    void beforeEach() {
+        taskManager = new MemoryTaskManager(new Hostname("foo"));
+        mailboxManager = InMemoryIntegrationResources.defaultResources().getMailboxManager();
+        usersRepository = MemoryUsersRepository.withoutVirtualHosting(NO_DOMAIN_LIST);
+        JsonTransformer jsonTransformer = new JsonTransformer();
+
+        webAdminServer = WebAdminUtils.createWebAdminServer(
+                new TasksRoutes(taskManager, jsonTransformer,
+                    DTOConverter.of(
+                        ExpireInboxesAdditionalInformationDTO.module())),
+                new MessagesRoutes(taskManager,
+                    new InMemoryMessageId.Factory(),
+                    null,
+                    new ExpireInboxesService(usersRepository, mailboxManager),
+                    jsonTransformer,
+                    ImmutableSet.of()))
+            .start();
+
+        RestAssured.requestSpecification = WebAdminUtils.buildRequestSpecification(webAdminServer).build();
+        RestAssured.enableLoggingOfRequestAndResponseIfValidationFails();
+    }
+
+    @AfterEach
+    void tearDown() {
+        webAdminServer.destroy();
+        taskManager.stop();
+    }
+
+    @Nested
+    class ExpireInboxes {
+        @Nested
+        class Validation {
+            @Test
+            void expireInboxesShouldFailWithNoOption() {
+                when()
+                    .delete("/messages")
+                .then()
+                    .statusCode(HttpStatus.BAD_REQUEST_400)
+                    .body("statusCode", is(400))
+                    .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("Must specify either 'olderThan' or 'byExpiresHeader' parameter"));
+            }
+
+            @Test
+            void expireInboxesShouldFailWithBothOptions() {
+                when()
+                    .delete("/messages?byExpiresHeader&olderThan=30d")
+                .then()
+                    .statusCode(HttpStatus.BAD_REQUEST_400)
+                    .body("statusCode", is(400))
+                    .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("Must specify either 'olderThan' or 'byExpiresHeader' parameter"));
+            }
+
+            @Test
+            void expireInboxesShouldFailWithBadOlderThan() {
+                when()
+                    .delete("/messages?olderThan=bad")
+                    .then()
+                    .statusCode(HttpStatus.BAD_REQUEST_400)
+                    .body("statusCode", is(400))
+                    .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("'usersPerSecond' must be numeric"));
+            }
+
+            @Test
+            void expireInboxesShouldFailWithNegativeOlderThan() {
+                when()
+                    .delete("/messages?olderThan=-30d")
+                    .then()
+                    .statusCode(HttpStatus.BAD_REQUEST_400)
+                    .body("statusCode", is(400))
+                    .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("Duration amount should be positive"));
+            }
+
+            @Test
+            void expireInboxesShouldFailWithBadUsersPerSeconds() {
+                when()
+                    .delete("/messages?byExpiresHeader&usersPerSecond=bad")
+                    .then()
+                    .statusCode(HttpStatus.BAD_REQUEST_400)
+                    .body("statusCode", is(400))
+                    .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("'usersPerSecond' must be numeric"));
+            }
+
+            @Test
+            void expireInboxesShouldFailWithNegativeUsersPerSeconds() {
+                when()
+                    .delete("/messages?byExpiresHeader&usersPerSecond=-10")
+                    .then()
+                    .statusCode(HttpStatus.BAD_REQUEST_400)
+                    .body("statusCode", is(400))
+                    .body("type", is(ErrorResponder.ErrorType.INVALID_ARGUMENT.getType()))
+                    .body("message", is("Invalid arguments supplied in the user request"))
+                    .body("details", is("'usersPerSecond' must be strictly positive"));
+            }
+
+        }
+
+        @Nested
+        class TaskDetails {
+            @Test
+            void expireInboxesShouldNotFailWhenNoMailsFound() {
+                String taskId = when()
+                    .delete("/messages?byExpiresHeader")
+                    .jsonPath()
+                    .get("taskId");
+
+                given()
+                    .basePath(TasksRoutes.BASE)
+                .when()
+                    .get(taskId + "/await")
+                .then()
+                    .body("type", Matchers.is("ExpireInboxesTask"))
+                    .body("status", is("completed"))
+                    .body("taskId", is(notNullValue()))
+                    .body("additionalInformation.type", is("ExpireInboxesTask"))
+                    .body("additionalInformation.runningOptions.usersPerSecond", is(RunningOptions.DEFAULT.getUsersPerSecond()))
+                    .body("additionalInformation.runningOptions.byExpiresHeader", is(true))
+                    .body("additionalInformation.runningOptions.olderThan", is(nullValue()))
+                    .body("additionalInformation.inboxesExpired", is(0))
+                    .body("additionalInformation.inboxesFailed", is(0))
+                    .body("additionalInformation.inboxesProcessed", is(0))
+                    .body("additionalInformation.messagesDeleted", is(0));
+            }
+
+            @Test
+            void expireInboxesShouldReturnTaskDetailsWhenMail() throws Exception {
+                usersRepository.addUser(USERNAME, "secret");
+                MailboxSession systemSession = mailboxManager.createSystemSession(USERNAME);
+                mailboxManager.createMailbox(INBOX, systemSession).get();
+                mailboxManager.getMailbox(INBOX, systemSession).appendMessage(
+                    MessageManager.AppendCommand.builder()
+                        .withInternalDate(new Date(System.currentTimeMillis() - 5000))
+                        .build("header: value\r\n\r\nbody"),
+                    systemSession).getId();
+
+                String taskId = when()
+                    .delete("/messages?olderThan=1s")
+                    .jsonPath()
+                    .get("taskId");
+
+                given()
+                    .basePath(TasksRoutes.BASE)
+                .when()
+                    .get(taskId + "/await")
+                .then()
+                    .body("type", Matchers.is("ExpireInboxesTask"))
+                    .body("status", is("completed"))
+                    .body("taskId", is(notNullValue()))
+                    .body("additionalInformation.type", is("ExpireInboxesTask"))
+                    .body("additionalInformation.runningOptions.usersPerSecond", is(RunningOptions.DEFAULT.getUsersPerSecond()))
+                    .body("additionalInformation.runningOptions.byExpiresHeader", is(false))
+                    .body("additionalInformation.runningOptions.olderThan", is("1s"))
+                    .body("additionalInformation.inboxesExpired", is(1))
+                    .body("additionalInformation.inboxesFailed", is(0))
+                    .body("additionalInformation.inboxesProcessed", is(1))
+                    .body("additionalInformation.messagesDeleted", is(1));
+            }

Review Comment:
   Maybe adding a test to show it does not delete messages not older than the value specified in the task?



-- 
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] ottoka commented on pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on PR #1004:
URL: https://github.com/apache/james-project/pull/1004#issuecomment-1135679133

   Latest push includes suggested fixes and rebased everything on top of current master.


-- 
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] ottoka commented on pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on PR #1004:
URL: https://github.com/apache/james-project/pull/1004#issuecomment-1130037200

   Checked SearchUtilsTest, turns out it actually was ignoring the zone offset: Mon, 26 Mar 2007 00:00:00 +0300 is NOT on 26 Mar 2007 in GMT, but three hours earlier and thus 25 Mar 2007. I changed it to Mon, 26 Mar 2007 **03**:00:00 +0300 which is 00:00:00 in GMT, seems to be what the test intended. As you said, Date/Time operations are always nasty.


-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875605597


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesTask.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.webadmin.service;
+
+import java.time.Clock;
+import java.time.Instant;
+import java.util.Date;
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.james.task.Task;
+import org.apache.james.task.TaskExecutionDetails;
+import org.apache.james.task.TaskType;
+import org.apache.james.webadmin.service.ExpireInboxesService.Context;
+import org.apache.james.webadmin.service.ExpireInboxesService.RunningOptions;
+
+import reactor.core.scheduler.Schedulers;
+
+public class ExpireInboxesTask implements Task {
+    public static final TaskType TASK_TYPE = TaskType.of("ExpireInboxesTask");
+
+    public static class AdditionalInformation implements TaskExecutionDetails.AdditionalInformation {
+
+        public static ExpireInboxesTask.AdditionalInformation from(Context context,
+                                                                   RunningOptions runningOptions) {
+            return new ExpireInboxesTask.AdditionalInformation(
+                Clock.systemUTC().instant(),
+                runningOptions,
+                context.getInboxesExpired(),
+                context.getInboxesFailed(),
+                context.getInboxesProcessed());
+        }
+
+        private final Instant timestamp;
+        private final RunningOptions runningOptions;
+        private final long inboxesExpired;
+        private final long inboxesFailed;
+        private final long inboxesProcessed;
+
+        public AdditionalInformation(Instant timestamp,
+                                     RunningOptions runningOptions,
+                                     long inboxesExpired,
+                                     long inboxesFailed,
+                                     long inboxesProcessed) {
+            this.timestamp = timestamp;
+            this.runningOptions = runningOptions;
+            this.inboxesExpired = inboxesExpired;
+            this.inboxesFailed = inboxesFailed;
+            this.inboxesProcessed = inboxesProcessed;
+        }
+
+        public RunningOptions getRunningOptions() {
+            return runningOptions;
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired;
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed;
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed;
+        }
+
+        @Override
+        public Instant timestamp() {
+            return timestamp;
+        }
+    }
+
+    private final ExpireInboxesService expireInboxesService;
+    private final Context context;
+    private final RunningOptions runningOptions;
+
+    @Inject
+    public ExpireInboxesTask(ExpireInboxesService expireInboxesService,
+                             RunningOptions runningOptions) {
+        this.expireInboxesService = expireInboxesService;
+        this.context = new Context();
+        this.runningOptions = runningOptions;
+    }
+
+    @Override
+    public Result run() {
+        return expireInboxesService.expireInboxes(context, runningOptions, new Date())
+            .subscribeOn(Schedulers.elastic())

Review Comment:
   Ok, seems I missed that, should be an easy fix.



-- 
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] ottoka commented on pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on PR #1004:
URL: https://github.com/apache/james-project/pull/1004#issuecomment-1134634272

   Latest push adds the missing test, allows using a different mailbox than INBOX, and adds Webadmin and Mailet documentation.


-- 
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 #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r880159040


##########
server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc:
##########
@@ -1206,6 +1206,51 @@ This task could be run safely online and can be scheduled on a recurring
 basis outside of peak traffic by an admin to ensure Cassandra message
 consistency.
 
+=== Deleting old messages of all users
+
+*Note:*
+Consider enabling the xref:configure/vault.adoc[Deleted Messages Vault]
+if you use this feature.
+
+Old messages tend to pile up in user INBOXes. An admin might want to delete
+these on behalf of the users, e.g. all messages older than 30 days:
+....
+curl -XDELETE http://ip:port/messages?olderThan=30d
+....
+
+link:#_endpoints_returning_a_task[More details about endpoints returning a task].
+
+The `olderThan` parameter should be expressed in the following format: `Nunit`.
+`N` should be strictly positive. `unit` could be either in the short form
+(`d`, `w`, `y` etc.), or in the long form (`days`, `weeks`, `months`, `years`).
+The default unit is `days`.
+
+Response codes:
+
+* 201: Success. Corresponding task id is returned.
+* 400: Error in the request. Details can be found in the reported error.
+
+To delete old mails from a different mailbox than INBOX, e.g. a mailbox
+named "Archived" :
+....
+curl -XDELETE http://ip:port/messages?mailbox=Archived&olderThan=30d
+....
+
+Since this is a somewhat expensive operation, the task is throttled to one user
+per second. You may speed it up via `usersPerSecond=10` for example. But keep
+in mind that a high rate might overwhelm your database or blob store.
+
+*Cassandra only:*

Review Comment:
   Then let me suggest:
   
   ```suggestion
   *Scanning search only:* (unsupported for Lucene and ElasticSearch search implementations).
   ```
   
   You can very well use cassandra in conjunction with ElasticSearch. That's the norm more than the exception. 



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r880151791


##########
mailet/standard/src/main/java/org/apache/james/transport/mailets/Expires.java:
##########
@@ -0,0 +1,144 @@
+/****************************************************************
+ * 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.transport.mailets;
+
+import java.io.StringReader;
+import java.time.Duration;
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.time.temporal.ChronoUnit;
+import java.util.Optional;
+import java.util.function.Supplier;
+
+import javax.mail.MessagingException;
+import javax.mail.internet.MimeMessage;
+
+import org.apache.james.mime4j.dom.datetime.DateTime;
+import org.apache.james.mime4j.field.datetime.parser.DateTimeParser;
+import org.apache.james.mime4j.field.datetime.parser.ParseException;
+import org.apache.james.util.DurationParser;
+import org.apache.mailet.Mail;
+import org.apache.mailet.base.DateFormats;
+import org.apache.mailet.base.GenericMailet;
+
+
+/**
+ * <p>Adds an Expires header to the message, or enforces the period of an existing one.</p>
+ *
+ * <p>Sample configuration:</p>
+ *
+ * <pre><code>
+ * &lt;mailet match="All" class="Expires"&gt;
+ *   &lt;minAge&gt;1d&lt;/minAge&gt;
+ *   &lt;maxAge&gt;1w&lt;/maxAge&gt;
+ *   &lt;defaultAge&gt;4w&lt;/defaultAge&gt;
+ * &lt;/mailet&gt;
+ * </code></pre>
+ *
+ * @version 1.0.0, 2021-12-14

Review Comment:
   Makes sense, I will put the example curl call 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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875603196


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,209 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, Optional.empty());
+
+        private final int usersPerSecond;
+
+        private final Optional<String> maxAge;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("maxAge") Optional<String> maxAge) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' needs to be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+    
+            this.maxAge = maxAge;
+            this.maxAgeDuration = maxAge.map(v -> {
+                Duration maxAgeDuration = DurationParser.parse(maxAge.get(), ChronoUnit.DAYS);
+                Preconditions.checkArgument(!maxAgeDuration.isNegative(), "'maxAge' must be positive");
+                return maxAgeDuration;
+            });
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+
+        public Optional<String> getMaxAge() {
+            return maxAge;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireInboxesService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireInboxesService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireInboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            LOGGER.info("expire with maxAge {} = {}", runningOptions.maxAge, runningOptions.maxAgeDuration);
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)
+                    )
+            );
+            return Iterators.toFlux(usersRepository.list())
+                .transform(ReactorUtils.<Username, Task.Result>throttle()
+                    .elements(runningOptions.getUsersPerSecond())
+                    .per(Duration.ofSeconds(1))
+                    .forOperation(username -> expireUserInbox(context, username, expiration)))
+                .reduce(Task.Result.COMPLETED, Task::combine);
+        } catch (UsersRepositoryException e) {
+            LOGGER.error("Error while accessing users from repository", e);
+            return Mono.just(Task.Result.PARTIAL);
+        }
+    }
+
+    private Mono<Result> expireUserInbox(Context context, Username username, SearchQuery expiration) {
+        MailboxSession session = mailboxManager.createSystemSession(username);
+        MailboxPath mailboxPath = MailboxPath.inbox(username);
+        return Mono.from(mailboxManager.getMailboxReactive(mailboxPath, session))
+            // newly created users do not have mailboxes yet, just skip them
+            .onErrorResume(MailboxNotFoundException.class, ignore -> Mono.empty())
+            .flatMap(mgr -> searchMessagesReactive(mgr, session, expiration)
+                .flatMap(list -> deleteMessagesReactive(mgr, session, list)))
+            .doOnNext(expired -> {
+                if (expired) {
+                    context.incrementExpiredCount();
+                }
+                context.incrementProcessedCount();
+            })
+            .then(Mono.just(Task.Result.COMPLETED))
+            .onErrorResume(e -> {
+                LOGGER.warn("Failed to expire user mailbox {}", username, e);
+                context.incrementFailedCount();
+                context.incrementProcessedCount();
+                return Mono.just(Task.Result.PARTIAL);
+            });
+    }
+
+    private Mono<List<MessageUid>> searchMessagesReactive(MessageManager mgr, MailboxSession session, SearchQuery expiration) {
+        try {
+            return Flux.from(mgr.search(expiration, session)).collectList();
+        } catch (MailboxException e) {
+            return Mono.error(e);
+        }
+    }
+
+    private Mono<Boolean> deleteMessagesReactive(MessageManager mgr, MailboxSession session, List<MessageUid> uids) {
+        try {
+            if (uids.isEmpty()) {
+                return Mono.just(false);
+            } else {
+                mgr.delete(uids, session);
+                return Mono.just(true);

Review Comment:
   I do not feel comfortable in messing with the MessageManager, so I will go with the wrap approach. Feel free to change this later at your convenience :)



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875586230


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,209 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, Optional.empty());
+
+        private final int usersPerSecond;
+
+        private final Optional<String> maxAge;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("maxAge") Optional<String> maxAge) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' needs to be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+    
+            this.maxAge = maxAge;
+            this.maxAgeDuration = maxAge.map(v -> {
+                Duration maxAgeDuration = DurationParser.parse(maxAge.get(), ChronoUnit.DAYS);
+                Preconditions.checkArgument(!maxAgeDuration.isNegative(), "'maxAge' must be positive");
+                return maxAgeDuration;
+            });
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+
+        public Optional<String> getMaxAge() {
+            return maxAge;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireInboxesService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireInboxesService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireInboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            LOGGER.info("expire with maxAge {} = {}", runningOptions.maxAge, runningOptions.maxAgeDuration);
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)
+                    )
+            );
+            return Iterators.toFlux(usersRepository.list())
+                .transform(ReactorUtils.<Username, Task.Result>throttle()
+                    .elements(runningOptions.getUsersPerSecond())
+                    .per(Duration.ofSeconds(1))
+                    .forOperation(username -> expireUserInbox(context, username, expiration)))

Review Comment:
   This implementation is based on an assumption:
   1. People only sort mails into folders if they consider them "important" and want to keep them,
   2. People let anything else pile up in the INBOX and can't be bothered with cleaning it up.
   
   Thus, I want to do the INBOX cleanup for them, either with a hard cutoff after a month or two (maxAge) or giving them more control (Expires).
   
   What you describe seems to be a different use case, which I do not quite understand yet. How would you distinguish between "important" mails to keep forever, and general post pile that should be cleaned regularly? Is there no more "important" in this scenario? Or would it be a site policy via a configuration parameter, telling which mailboxes (or name match patterns) to clean up? Please elaborate.



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875596843


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,209 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, Optional.empty());
+
+        private final int usersPerSecond;
+
+        private final Optional<String> maxAge;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("maxAge") Optional<String> maxAge) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' needs to be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+    
+            this.maxAge = maxAge;
+            this.maxAgeDuration = maxAge.map(v -> {
+                Duration maxAgeDuration = DurationParser.parse(maxAge.get(), ChronoUnit.DAYS);
+                Preconditions.checkArgument(!maxAgeDuration.isNegative(), "'maxAge' must be positive");
+                return maxAgeDuration;
+            });
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+
+        public Optional<String> getMaxAge() {
+            return maxAge;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireInboxesService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireInboxesService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireInboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            LOGGER.info("expire with maxAge {} = {}", runningOptions.maxAge, runningOptions.maxAgeDuration);
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)
+                    )
+            );
+            return Iterators.toFlux(usersRepository.list())
+                .transform(ReactorUtils.<Username, Task.Result>throttle()
+                    .elements(runningOptions.getUsersPerSecond())
+                    .per(Duration.ofSeconds(1))
+                    .forOperation(username -> expireUserInbox(context, username, expiration)))
+                .reduce(Task.Result.COMPLETED, Task::combine);
+        } catch (UsersRepositoryException e) {
+            LOGGER.error("Error while accessing users from repository", e);
+            return Mono.just(Task.Result.PARTIAL);
+        }
+    }
+
+    private Mono<Result> expireUserInbox(Context context, Username username, SearchQuery expiration) {
+        MailboxSession session = mailboxManager.createSystemSession(username);
+        MailboxPath mailboxPath = MailboxPath.inbox(username);
+        return Mono.from(mailboxManager.getMailboxReactive(mailboxPath, session))
+            // newly created users do not have mailboxes yet, just skip them
+            .onErrorResume(MailboxNotFoundException.class, ignore -> Mono.empty())
+            .flatMap(mgr -> searchMessagesReactive(mgr, session, expiration)
+                .flatMap(list -> deleteMessagesReactive(mgr, session, list)))
+            .doOnNext(expired -> {
+                if (expired) {
+                    context.incrementExpiredCount();
+                }
+                context.incrementProcessedCount();
+            })
+            .then(Mono.just(Task.Result.COMPLETED))
+            .onErrorResume(e -> {
+                LOGGER.warn("Failed to expire user mailbox {}", username, e);
+                context.incrementFailedCount();
+                context.incrementProcessedCount();
+                return Mono.just(Task.Result.PARTIAL);
+            });
+    }
+
+    private Mono<List<MessageUid>> searchMessagesReactive(MessageManager mgr, MailboxSession session, SearchQuery expiration) {
+        try {
+            return Flux.from(mgr.search(expiration, session)).collectList();

Review Comment:
   I did it this way because MessageManager.delete() wants a list. I assume this lets it do a batch delete in the underlying database, instead of stressing it with multiple individual deletes (think cassandra cluster). Also, this will be an order of tens of mails, not thousands. From this perspective, it seems ultimately cheaper to collect the list in memory first.
   Let me know if I am wrong, in that case it is an easy fix :)



-- 
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 #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r879988100


##########
mailet/standard/src/main/java/org/apache/james/transport/mailets/Expires.java:
##########
@@ -0,0 +1,144 @@
+/****************************************************************
+ * 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.transport.mailets;
+
+import java.io.StringReader;
+import java.time.Duration;
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.time.temporal.ChronoUnit;
+import java.util.Optional;
+import java.util.function.Supplier;
+
+import javax.mail.MessagingException;
+import javax.mail.internet.MimeMessage;
+
+import org.apache.james.mime4j.dom.datetime.DateTime;
+import org.apache.james.mime4j.field.datetime.parser.DateTimeParser;
+import org.apache.james.mime4j.field.datetime.parser.ParseException;
+import org.apache.james.util.DurationParser;
+import org.apache.mailet.Mail;
+import org.apache.mailet.base.DateFormats;
+import org.apache.mailet.base.GenericMailet;
+
+
+/**
+ * <p>Adds an Expires header to the message, or enforces the period of an existing one.</p>
+ *
+ * <p>Sample configuration:</p>
+ *
+ * <pre><code>
+ * &lt;mailet match="All" class="Expires"&gt;
+ *   &lt;minAge&gt;1d&lt;/minAge&gt;
+ *   &lt;maxAge&gt;1w&lt;/maxAge&gt;
+ *   &lt;defaultAge&gt;4w&lt;/defaultAge&gt;
+ * &lt;/mailet&gt;
+ * </code></pre>
+ *
+ * @version 1.0.0, 2021-12-14

Review Comment:
   IMO we can remove the @version orut it to the upcoming 3.8.0 release.
   
   Also we should explain that to remove the expired mails one need to call the associated webadmin endpoints.



##########
server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc:
##########
@@ -1206,6 +1206,51 @@ This task could be run safely online and can be scheduled on a recurring
 basis outside of peak traffic by an admin to ensure Cassandra message
 consistency.
 
+=== Deleting old messages of all users
+
+*Note:*
+Consider enabling the xref:configure/vault.adoc[Deleted Messages Vault]
+if you use this feature.
+
+Old messages tend to pile up in user INBOXes. An admin might want to delete
+these on behalf of the users, e.g. all messages older than 30 days:
+....
+curl -XDELETE http://ip:port/messages?olderThan=30d
+....
+
+link:#_endpoints_returning_a_task[More details about endpoints returning a task].
+
+The `olderThan` parameter should be expressed in the following format: `Nunit`.
+`N` should be strictly positive. `unit` could be either in the short form
+(`d`, `w`, `y` etc.), or in the long form (`days`, `weeks`, `months`, `years`).
+The default unit is `days`.
+
+Response codes:
+
+* 201: Success. Corresponding task id is returned.
+* 400: Error in the request. Details can be found in the reported error.
+
+To delete old mails from a different mailbox than INBOX, e.g. a mailbox
+named "Archived" :
+....
+curl -XDELETE http://ip:port/messages?mailbox=Archived&olderThan=30d
+....
+
+Since this is a somewhat expensive operation, the task is throttled to one user
+per second. You may speed it up via `usersPerSecond=10` for example. But keep
+in mind that a high rate might overwhelm your database or blob store.
+
+*Cassandra only:*

Review Comment:
   ```suggestion
   *Scanning search only:*
   ```



##########
server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc:
##########
@@ -1206,6 +1206,51 @@ This task could be run safely online and can be scheduled on a recurring
 basis outside of peak traffic by an admin to ensure Cassandra message
 consistency.
 
+=== Deleting old messages of all users

Review Comment:
   Generaly we also document tasks details here in webadmin documentation



##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireMailboxService.java:
##########
@@ -0,0 +1,249 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxConstants;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.github.fge.lambdas.Throwing;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+import reactor.core.scheduler.Schedulers;
+
+
+public class ExpireMailboxService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, MailboxConstants.INBOX, true, Optional.empty());
+
+        public static RunningOptions fromParams(
+                Optional<String> byExpiresHeader, Optional<String> olderThan,
+                Optional<String> usersPerSecond, Optional<String> mailbox) {
+            try {
+                if (byExpiresHeader.isPresent() == olderThan.isPresent()) {
+                    throw new IllegalArgumentException("Must specify either 'olderThan' or 'byExpiresHeader' parameter");
+                }
+                return new RunningOptions(
+                    usersPerSecond.map(Integer::parseInt).orElse(DEFAULT.getUsersPerSecond()),
+                    mailbox.orElse(DEFAULT.getMailbox()), byExpiresHeader.isPresent(), olderThan);
+            } catch (NumberFormatException ex) {
+                throw new IllegalArgumentException("'usersPerSecond' must be numeric");
+            }
+        }
+        
+        private final int usersPerSecond;
+        
+        private final String mailbox;
+        
+        private final boolean byExpiresHeader;
+
+        private final Optional<String> olderThan;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("mailbox") String mailbox,
+                              @JsonProperty("byExpiresHeader") boolean byExpiresHeader,
+                              @JsonProperty("olderThan") Optional<String> olderThan) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' must be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+            this.mailbox = mailbox;
+            this.byExpiresHeader = byExpiresHeader;
+            this.olderThan = olderThan;
+            this.maxAgeDuration = olderThan.map(v -> DurationParser.parse(olderThan.get(), ChronoUnit.DAYS));
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+        
+        public String getMailbox() { 
+            return mailbox;
+        }
+
+        public boolean getByExpiresHeader() {
+            return byExpiresHeader;
+        }
+
+        public Optional<String> getOlderThan() {
+            return olderThan;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+        private final AtomicLong messagesDeleted;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+            this.messagesDeleted = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public long getMessagesDeleted() {
+            return messagesDeleted.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+        
+        public void incrementMessagesDeleted(long count) { 
+            messagesDeleted.addAndGet(count);
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireMailboxService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireMailboxService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireMailboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)
+                    )
+            );
+            return Iterators.toFlux(usersRepository.list())
+                .transform(ReactorUtils.<Username, Task.Result>throttle()
+                    .elements(runningOptions.getUsersPerSecond())
+                    .per(Duration.ofSeconds(1))
+                    .forOperation(username -> 
+                        expireUserMailbox(context, username, runningOptions.getMailbox(), expiration)))
+                .reduce(Task.Result.COMPLETED, Task::combine);
+        } catch (UsersRepositoryException e) {
+            LOGGER.error("Error while accessing users from repository", e);
+            return Mono.just(Task.Result.PARTIAL);
+        }
+    }
+
+    private Mono<Result> expireUserMailbox(Context context, Username username, String mailbox, SearchQuery expiration) {
+        MailboxSession session = mailboxManager.createSystemSession(username);
+        MailboxPath mailboxPath = MailboxPath.forUser(username, mailbox);
+        return Mono.from(mailboxManager.getMailboxReactive(mailboxPath, session))
+            // newly created users do not have mailboxes yet, just skip them
+            .onErrorResume(MailboxNotFoundException.class, ignore -> Mono.empty())
+            .flatMap(mgr -> searchMessagesReactive(mgr, session, expiration)
+                .flatMap(list -> deleteMessagesReactive(mgr, session, list)))
+            .doOnNext(expired -> {
+                if (expired > 0) {
+                    context.incrementExpiredCount();
+                    context.incrementMessagesDeleted(expired);
+                }
+                context.incrementProcessedCount();
+            })
+            .then(Mono.just(Task.Result.COMPLETED))
+            .onErrorResume(e -> {
+                LOGGER.warn("Failed to expire user mailbox {}", username, e);
+                context.incrementFailedCount();
+                context.incrementProcessedCount();
+                return Mono.just(Task.Result.PARTIAL);
+            });
+    }
+
+    private Mono<List<MessageUid>> searchMessagesReactive(MessageManager mgr, MailboxSession session, SearchQuery expiration) {
+        try {
+            return Flux.from(mgr.search(expiration, session)).collectList();
+        } catch (MailboxException e) {
+            return Mono.error(e);
+        }
+    }
+
+    private Mono<Integer> deleteMessagesReactive(MessageManager mgr, MailboxSession session, List<MessageUid> uids) {
+        if (uids.isEmpty()) {
+            return Mono.just(0);
+        } else {
+            return Mono.fromSupplier(

Review Comment:
   Using fromCallable should allow getting read of the Throwing...sneakyThrow boiler plate



##########
server/apps/distributed-app/docs/modules/ROOT/partials/Expires.adoc:
##########
@@ -0,0 +1,28 @@
+=== Expires
+
+Sanitizes or adds an expiration date to a message, in the form of an `Expires`
+header (RFC 4021). By itself this header is informational only, but may be
+useful in combination with the Web Admin operation to 
+xref:operate/webadmin.adoc#_administrating_messages[delete old messages].
+
+The mailet can force an existing expiration date to be within the bounds
+given by `minAge`, `maxAge`, or both. `minAge` specifies the minimum time
+the date must lie in the future, while `maxAge` specifies a maximum.
+
+If a message has no expiration date, the mailet can add one according to
+the optional `defaultAge` parameter.
+
+All parameter values should be expressed in the following format: `Nunit`.
+`N` should be positive. `unit` could be either in the short form
+(`h`, `d`, `w`, `y` etc.), or in the long form (`hours`, days`, `weeks`,
+`months`, `years`). The default unit is `days`.
+
+Sample configuration:
+
+....
+<mailet match="All" class="Expires">
+    <minAge>12h</minAge>
+    <defaultAge>7d</defaultAge>
+    <maxAge>8w</maxAge>
+</mailet>
+....

Review Comment:
   Point to the route here too?



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r880291311


##########
mailet/standard/src/main/java/org/apache/james/transport/mailets/Expires.java:
##########
@@ -0,0 +1,144 @@
+/****************************************************************
+ * 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.transport.mailets;
+
+import java.io.StringReader;
+import java.time.Duration;
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.time.temporal.ChronoUnit;
+import java.util.Optional;
+import java.util.function.Supplier;
+
+import javax.mail.MessagingException;
+import javax.mail.internet.MimeMessage;
+
+import org.apache.james.mime4j.dom.datetime.DateTime;
+import org.apache.james.mime4j.field.datetime.parser.DateTimeParser;
+import org.apache.james.mime4j.field.datetime.parser.ParseException;
+import org.apache.james.util.DurationParser;
+import org.apache.mailet.Mail;
+import org.apache.mailet.base.DateFormats;
+import org.apache.mailet.base.GenericMailet;
+
+
+/**
+ * <p>Adds an Expires header to the message, or enforces the period of an existing one.</p>
+ *
+ * <p>Sample configuration:</p>
+ *
+ * <pre><code>
+ * &lt;mailet match="All" class="Expires"&gt;
+ *   &lt;minAge&gt;1d&lt;/minAge&gt;
+ *   &lt;maxAge&gt;1w&lt;/maxAge&gt;
+ *   &lt;defaultAge&gt;4w&lt;/defaultAge&gt;
+ * &lt;/mailet&gt;
+ * </code></pre>
+ *
+ * @version 1.0.0, 2021-12-14
+ */
+public class Expires extends GenericMailet {
+    
+    public static final String EXPIRES = "Expires";
+    
+    Supplier<ZonedDateTime> timeSource = ZonedDateTime::now; 

Review Comment:
   I did not know you could `@Inject ` dependencies into a mailet, the existing ones seem to be mostly self-sufficient. This is of course a much nicer solution than the default field exposed for testing only.
   Strangely enough, the injected Clock seems to run in UTC +0000 instead of the local timezone. Is it supposed to be that way? I would prefer the local timezone in the Expires date for readability, but I can live with the way it is.



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r879105881


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,238 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.github.fge.lambdas.Throwing;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+import reactor.core.scheduler.Schedulers;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, true, Optional.empty());
+
+        public static RunningOptions fromParams(
+                Optional<String> byExpiresHeader, Optional<String> olderThan, Optional<String> usersPerSecond) {
+            try {
+                if ((byExpiresHeader.isEmpty() && olderThan.isEmpty()) || (byExpiresHeader.isPresent() && olderThan.isPresent())) {

Review Comment:
   Right. I will go with `byExpiresHeader.isPresent() == olderThan.isPresent()`



-- 
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 #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875657517


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,209 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, Optional.empty());
+
+        private final int usersPerSecond;
+
+        private final Optional<String> maxAge;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("maxAge") Optional<String> maxAge) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' needs to be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+    
+            this.maxAge = maxAge;
+            this.maxAgeDuration = maxAge.map(v -> {
+                Duration maxAgeDuration = DurationParser.parse(maxAge.get(), ChronoUnit.DAYS);
+                Preconditions.checkArgument(!maxAgeDuration.isNegative(), "'maxAge' must be positive");
+                return maxAgeDuration;
+            });
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+
+        public Optional<String> getMaxAge() {
+            return maxAge;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireInboxesService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireInboxesService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireInboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            LOGGER.info("expire with maxAge {} = {}", runningOptions.maxAge, runningOptions.maxAgeDuration);
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)
+                    )
+            );
+            return Iterators.toFlux(usersRepository.list())
+                .transform(ReactorUtils.<Username, Task.Result>throttle()
+                    .elements(runningOptions.getUsersPerSecond())
+                    .per(Duration.ofSeconds(1))
+                    .forOperation(username -> expireUserInbox(context, username, expiration)))
+                .reduce(Task.Result.COMPLETED, Task::combine);
+        } catch (UsersRepositoryException e) {
+            LOGGER.error("Error while accessing users from repository", e);
+            return Mono.just(Task.Result.PARTIAL);
+        }
+    }
+
+    private Mono<Result> expireUserInbox(Context context, Username username, SearchQuery expiration) {
+        MailboxSession session = mailboxManager.createSystemSession(username);
+        MailboxPath mailboxPath = MailboxPath.inbox(username);
+        return Mono.from(mailboxManager.getMailboxReactive(mailboxPath, session))
+            // newly created users do not have mailboxes yet, just skip them
+            .onErrorResume(MailboxNotFoundException.class, ignore -> Mono.empty())
+            .flatMap(mgr -> searchMessagesReactive(mgr, session, expiration)
+                .flatMap(list -> deleteMessagesReactive(mgr, session, list)))
+            .doOnNext(expired -> {
+                if (expired) {
+                    context.incrementExpiredCount();
+                }
+                context.incrementProcessedCount();
+            })
+            .then(Mono.just(Task.Result.COMPLETED))
+            .onErrorResume(e -> {
+                LOGGER.warn("Failed to expire user mailbox {}", username, e);
+                context.incrementFailedCount();
+                context.incrementProcessedCount();
+                return Mono.just(Task.Result.PARTIAL);
+            });
+    }
+
+    private Mono<List<MessageUid>> searchMessagesReactive(MessageManager mgr, MailboxSession session, SearchQuery expiration) {
+        try {
+            return Flux.from(mgr.search(expiration, session)).collectList();

Review Comment:
   Fair. 



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875751938


##########
mailet/standard/src/main/java/org/apache/james/transport/mailets/Expires.java:
##########
@@ -0,0 +1,144 @@
+/****************************************************************
+ * 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.transport.mailets;
+
+import java.io.StringReader;
+import java.time.Duration;
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.time.temporal.ChronoUnit;
+import java.util.Optional;
+import java.util.function.Supplier;
+
+import javax.mail.MessagingException;
+import javax.mail.internet.MimeMessage;
+
+import org.apache.james.mime4j.dom.datetime.DateTime;
+import org.apache.james.mime4j.field.datetime.parser.DateTimeParser;
+import org.apache.james.mime4j.field.datetime.parser.ParseException;
+import org.apache.james.util.DurationParser;
+import org.apache.mailet.Mail;
+import org.apache.mailet.base.DateFormats;
+import org.apache.mailet.base.GenericMailet;
+
+
+/**
+ * <p>Adds an Expires header to the message, or enforces the period of an existing one.</p>
+ *
+ * <p>Sample configuration:</p>
+ *
+ * <pre><code>
+ * &lt;mailet match="All" class="Expires"&gt;
+ *   &lt;minAge&gt;1d&lt;/minAge&gt;
+ *   &lt;maxAge&gt;1w&lt;/maxAge&gt;
+ *   &lt;defaultAge&gt;4w&lt;/defaultAge&gt;
+ * &lt;/mailet&gt;
+ * </code></pre>
+ *
+ * @version 1.0.0, 2021-12-14
+ */
+public class Expires extends GenericMailet {
+    
+    public static final String EXPIRES = "Expires";
+    
+    Supplier<ZonedDateTime> timeSource = ZonedDateTime::now; 
+
+    private Optional<Duration> minAge = Optional.empty();
+    private Optional<Duration> maxAge = Optional.empty();
+    private Optional<Duration> defaultAge = Optional.empty();
+    
+    @Override
+    public void init() throws MessagingException {
+        minAge = parseDuration("minAge");
+        maxAge = parseDuration("maxAge");
+        defaultAge = parseDuration("defaultAge");
+
+        if (minAge.isEmpty() && maxAge.isEmpty() && defaultAge.isEmpty()) {
+            throw new MessagingException("Please configure at least one of minAge, maxAge, defaultAge");
+        }
+
+        if (isAfter(minAge, maxAge)) {
+            throw new MessagingException("minAge must be before maxAge");
+        }
+        if (isAfter(defaultAge, maxAge)) {
+            throw new MessagingException("defaultAge must be before maxAge");
+        }
+        if (isAfter(minAge, defaultAge)) {
+            throw new MessagingException("minAge must be before defaultAge");
+        }
+    }
+
+    @Override
+    public void service(Mail mail) throws MessagingException {
+        ZonedDateTime now = timeSource.get();
+        MimeMessage message = mail.getMessage();
+        Optional<ZonedDateTime> expires = parseExpiresHeader(message);
+        if (expires.isPresent()) {
+            if (minAge.isPresent() && expires.get().isBefore(now.plus(minAge.get()))) {
+                setExpiresAfter(message, now, minAge.get());
+            } else
+            if (maxAge.isPresent() && expires.get().isAfter(now.plus(maxAge.get()))) {
+                setExpiresAfter(message, now, maxAge.get());
+            }
+        } else if (defaultAge.isPresent()) {
+            setExpiresAfter(message, now, defaultAge.get());
+        }
+    }
+
+    @Override
+    public String getMailetInfo() {
+        return "Expire Mailet";
+    }
+
+    private Optional<Duration> parseDuration(String param) {
+        String duration = getInitParameter(param);
+        if (duration == null) {
+            return Optional.empty();
+        } else {
+            return Optional.of(DurationParser.parse(duration, ChronoUnit.DAYS));
+        }
+    }
+    
+    private boolean isAfter(Optional<Duration> a, Optional<Duration> b) {
+        return a.isPresent() && b.isPresent() && a.get().compareTo(b.get()) > 0;
+    }
+    
+    private Optional<ZonedDateTime> parseExpiresHeader(MimeMessage message) {
+        try {
+            String[] expires = message.getHeader(EXPIRES);
+            if (expires == null || expires.length == 0) {
+                return Optional.empty();
+            } else {
+                DateTime dt = new DateTimeParser(new StringReader(expires[0])).parseAll();
+                return Optional.of(ZonedDateTime.of(
+                    dt.getYear(), dt.getMonth(), dt.getDay(),
+                    dt.getHour(), dt.getMinute(), dt.getSecond(), 0,
+                    ZoneOffset.ofHoursMinutes(dt.getTimeZone() / 100, dt.getTimeZone() % 100)));

Review Comment:
   Well, that is how the DateTimeParser handles the offset, just treats it as an integer value without further checks. At least ZoneOffset.ofHoursMinutes(hh, mm) will throw for values too far off the chart, like -9999. It will accept anything is hhmm range though, like -1234. I don't see how to make this less hacky than it is, except patching it at the source in DateTimeParser.



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875924982


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesAdditionalInformationDTO.java:
##########
@@ -0,0 +1,109 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Instant;
+
+import org.apache.james.json.DTOModule;
+import org.apache.james.server.task.json.dto.AdditionalInformationDTO;
+import org.apache.james.server.task.json.dto.AdditionalInformationDTOModule;
+import org.apache.james.webadmin.service.ExpireInboxesService.RunningOptions;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+public class ExpireInboxesAdditionalInformationDTO implements AdditionalInformationDTO {
+
+    public static AdditionalInformationDTOModule<ExpireInboxesTask.AdditionalInformation, ExpireInboxesAdditionalInformationDTO> module() {
+        return DTOModule.forDomainObject(ExpireInboxesTask.AdditionalInformation.class)
+            .convertToDTO(ExpireInboxesAdditionalInformationDTO.class)
+            .toDomainObjectConverter(ExpireInboxesAdditionalInformationDTO::toDomainObject)
+            .toDTOConverter(ExpireInboxesAdditionalInformationDTO::toDto)
+            .typeName(ExpireInboxesTask.TASK_TYPE.asString())
+            .withFactory(AdditionalInformationDTOModule::new);
+    }
+
+    private static ExpireInboxesTask.AdditionalInformation toDomainObject(ExpireInboxesAdditionalInformationDTO dto) {
+        return new ExpireInboxesTask.AdditionalInformation(
+            dto.getTimestamp(),
+            dto.getRunningOptions(),
+            dto.getInboxesExpired(),
+            dto.getInboxesFailed(),
+            dto.getInboxesProcessed());
+    }
+
+    private static ExpireInboxesAdditionalInformationDTO toDto(ExpireInboxesTask.AdditionalInformation details, String type) {
+        return new ExpireInboxesAdditionalInformationDTO(
+            details.timestamp(),
+            type,
+            details.getRunningOptions(),
+            details.getInboxesExpired(),
+            details.getInboxesFailed(),
+            details.getInboxesProcessed());
+    }
+
+    private final Instant timestamp;
+    private final String type;
+    private final RunningOptions runningOptions;
+    private final long inboxesExpired;
+    private final long inboxesFailed;
+    private final long inboxesProcessed;
+
+    @JsonCreator
+    public ExpireInboxesAdditionalInformationDTO(@JsonProperty("timestamp") Instant timestamp,
+                                                 @JsonProperty("type") String type,
+                                                 @JsonProperty("runningOptions") RunningOptions runningOptions,
+                                                 @JsonProperty("inboxesExpired") long inboxesExpired,
+                                                 @JsonProperty("inboxesFailed") long inboxesFailed,
+                                                 @JsonProperty("inboxesProcessed") long inboxesProcessed) {
+        this.timestamp = timestamp;
+        this.type = type;
+        this.runningOptions = runningOptions;
+        this.inboxesExpired = inboxesExpired;
+        this.inboxesFailed = inboxesFailed;
+        this.inboxesProcessed = inboxesProcessed;
+    }
+
+    public RunningOptions getRunningOptions() {
+        return runningOptions;
+    }
+
+    public long getInboxesExpired() {
+        return inboxesExpired;
+    }
+
+    public long getInboxesFailed() {
+        return inboxesFailed;
+    }
+
+    public long getInboxesProcessed() {
+        return inboxesProcessed;
+    }

Review Comment:
   I added a metric for total messages deleted by the task. But I am not sure if the batch deletion is guaranteed to be atomic/transactional, i.e. if it fails on one message in the batch, could it still have deleted the messages listed before, or even all other messages? In that case, measuring failed deletions precisely is not really possible.



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875572072


##########
mailet/standard/src/main/java/org/apache/james/transport/mailets/Expires.java:
##########
@@ -0,0 +1,144 @@
+/****************************************************************
+ * 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.transport.mailets;
+
+import java.io.StringReader;
+import java.time.Duration;
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.time.temporal.ChronoUnit;
+import java.util.Optional;
+import java.util.function.Supplier;
+
+import javax.mail.MessagingException;
+import javax.mail.internet.MimeMessage;
+
+import org.apache.james.mime4j.dom.datetime.DateTime;
+import org.apache.james.mime4j.field.datetime.parser.DateTimeParser;
+import org.apache.james.mime4j.field.datetime.parser.ParseException;
+import org.apache.james.util.DurationParser;
+import org.apache.mailet.Mail;
+import org.apache.mailet.base.DateFormats;
+import org.apache.mailet.base.GenericMailet;
+
+
+/**
+ * <p>Adds an Expires header to the message, or enforces the period of an existing one.</p>
+ *
+ * <p>Sample configuration:</p>
+ *
+ * <pre><code>
+ * &lt;mailet match="All" class="Expires"&gt;
+ *   &lt;minAge&gt;1d&lt;/minAge&gt;
+ *   &lt;maxAge&gt;1w&lt;/maxAge&gt;
+ *   &lt;defaultAge&gt;4w&lt;/defaultAge&gt;
+ * &lt;/mailet&gt;
+ * </code></pre>
+ *
+ * @version 1.0.0, 2021-12-14
+ */
+public class Expires extends GenericMailet {
+    
+    public static final String EXPIRES = "Expires";
+    
+    Supplier<ZonedDateTime> timeSource = ZonedDateTime::now; 
+
+    private Optional<Duration> minAge = Optional.empty();
+    private Optional<Duration> maxAge = Optional.empty();
+    private Optional<Duration> defaultAge = Optional.empty();
+    
+    @Override
+    public void init() throws MessagingException {
+        minAge = parseDuration("minAge");
+        maxAge = parseDuration("maxAge");
+        defaultAge = parseDuration("defaultAge");
+
+        if (minAge.isEmpty() && maxAge.isEmpty() && defaultAge.isEmpty()) {
+            throw new MessagingException("Please configure at least one of minAge, maxAge, defaultAge");
+        }
+
+        if (isAfter(minAge, maxAge)) {
+            throw new MessagingException("minAge must be before maxAge");
+        }
+        if (isAfter(defaultAge, maxAge)) {
+            throw new MessagingException("defaultAge must be before maxAge");
+        }
+        if (isAfter(minAge, defaultAge)) {
+            throw new MessagingException("minAge must be before defaultAge");
+        }
+    }
+
+    @Override
+    public void service(Mail mail) throws MessagingException {
+        ZonedDateTime now = timeSource.get();
+        MimeMessage message = mail.getMessage();
+        Optional<ZonedDateTime> expires = parseExpiresHeader(message);
+        if (expires.isPresent()) {
+            if (minAge.isPresent() && expires.get().isBefore(now.plus(minAge.get()))) {
+                setExpiresAfter(message, now, minAge.get());
+            } else
+            if (maxAge.isPresent() && expires.get().isAfter(now.plus(maxAge.get()))) {
+                setExpiresAfter(message, now, maxAge.get());
+            }
+        } else if (defaultAge.isPresent()) {
+            setExpiresAfter(message, now, defaultAge.get());
+        }
+    }
+
+    @Override
+    public String getMailetInfo() {
+        return "Expire Mailet";
+    }
+
+    private Optional<Duration> parseDuration(String param) {
+        String duration = getInitParameter(param);
+        if (duration == null) {
+            return Optional.empty();
+        } else {
+            return Optional.of(DurationParser.parse(duration, ChronoUnit.DAYS));
+        }
+    }
+    
+    private boolean isAfter(Optional<Duration> a, Optional<Duration> b) {
+        return a.isPresent() && b.isPresent() && a.get().compareTo(b.get()) > 0;
+    }
+    
+    private Optional<ZonedDateTime> parseExpiresHeader(MimeMessage message) {
+        try {
+            String[] expires = message.getHeader(EXPIRES);
+            if (expires == null || expires.length == 0) {
+                return Optional.empty();
+            } else {
+                DateTime dt = new DateTimeParser(new StringReader(expires[0])).parseAll();
+                return Optional.of(ZonedDateTime.of(
+                    dt.getYear(), dt.getMonth(), dt.getDay(),
+                    dt.getHour(), dt.getMinute(), dt.getSecond(), 0,
+                    ZoneOffset.ofHoursMinutes(dt.getTimeZone() / 100, dt.getTimeZone() % 100)));

Review Comment:
   I assumed this was the right way of doing it, since MessageSearches uses DateTimeParser as well. In its defense, it nicely ignores any extra zone info some mail programs like to add, like (CEST) or [Berlin]. Plus it actually preserves the zone offset, giving us a proper DateTime. 
   Of course, I can change it over to another parser. DateTimeFieldLenientImpl.PARSER seems to be somewhat strict for this purpose, would require additional input string sanitation before use. What about javax.mail.internet.MailDateFormat ?



-- 
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] ottoka merged pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka merged PR #1004:
URL: https://github.com/apache/james-project/pull/1004


-- 
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 pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1004:
URL: https://github.com/apache/james-project/pull/1004#issuecomment-1129061837

   Failed tests looks related...
   
   ```
   [org.apache.james.mailbox.store.search.SearchUtilsTest.testShouldMatchHeaderDateAfterWithOffset](https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1004/1/testReport/junit/org.apache.james.mailbox.store.search/SearchUtilsTest/testShouldMatchHeaderDateAfterWithOffset/)
       [org.apache.james.mailbox.store.search.SearchUtilsTest.testShouldMatchHeaderDateBeforeWithOffset](https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1004/1/testReport/junit/org.apache.james.mailbox.store.search/SearchUtilsTest/testShouldMatchHeaderDateBeforeWithOffset/)
       [org.apache.james.mailbox.store.search.SearchUtilsTest.testMatchHeaderDateOnWithOffset](https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1004/1/testReport/junit/org.apache.james.mailbox.store.search/SearchUtilsTest/testMatchHeaderDateOnWithOffset/)
    ```
    
    Could you have a look?


-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875753770


##########
server/protocols/webadmin/webadmin-mailbox/src/test/java/org/apache/james/webadmin/service/ExpireInboxesServiceTest.java:
##########
@@ -0,0 +1,219 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.task.Task;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyList;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;

Review Comment:
   Ok, it is actually InMemoryIntegrationResources. May be difficult to make it fail for certain test cases. I'll think of something.



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875586230


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,209 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, Optional.empty());
+
+        private final int usersPerSecond;
+
+        private final Optional<String> maxAge;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("maxAge") Optional<String> maxAge) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' needs to be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+    
+            this.maxAge = maxAge;
+            this.maxAgeDuration = maxAge.map(v -> {
+                Duration maxAgeDuration = DurationParser.parse(maxAge.get(), ChronoUnit.DAYS);
+                Preconditions.checkArgument(!maxAgeDuration.isNegative(), "'maxAge' must be positive");
+                return maxAgeDuration;
+            });
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+
+        public Optional<String> getMaxAge() {
+            return maxAge;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireInboxesService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireInboxesService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireInboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            LOGGER.info("expire with maxAge {} = {}", runningOptions.maxAge, runningOptions.maxAgeDuration);
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)
+                    )
+            );
+            return Iterators.toFlux(usersRepository.list())
+                .transform(ReactorUtils.<Username, Task.Result>throttle()
+                    .elements(runningOptions.getUsersPerSecond())
+                    .per(Duration.ofSeconds(1))
+                    .forOperation(username -> expireUserInbox(context, username, expiration)))

Review Comment:
   This implementation is based on an assumption:
   1. People only sort mails into folders if they consider them "important" and want to keep them,
   2. People let anything else pile up in the INBOX and can't be bothered with cleaning it up.
   Thus, I want to do the INBOX cleanup for them, either with a hard cutoff after a month or two (maxAge) or giving them more control (Expires).
   
   What you describe seems to be a different use case, which I do not quite understand yet. How would you distinguish between "important" mails to keep forever, and general post pile that should be cleaned regularly? Is there no more "important" in this scenario? Or would it be a site policy via a configuration parameter, telling which mailboxes (or name match patterns) to clean up? Please elaborate.



-- 
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 #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r875660221


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,209 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, Optional.empty());
+
+        private final int usersPerSecond;
+
+        private final Optional<String> maxAge;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("maxAge") Optional<String> maxAge) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' needs to be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+    
+            this.maxAge = maxAge;
+            this.maxAgeDuration = maxAge.map(v -> {
+                Duration maxAgeDuration = DurationParser.parse(maxAge.get(), ChronoUnit.DAYS);
+                Preconditions.checkArgument(!maxAgeDuration.isNegative(), "'maxAge' must be positive");
+                return maxAgeDuration;
+            });
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+
+        public Optional<String> getMaxAge() {
+            return maxAge;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireInboxesService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireInboxesService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireInboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            LOGGER.info("expire with maxAge {} = {}", runningOptions.maxAge, runningOptions.maxAgeDuration);
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)
+                    )
+            );
+            return Iterators.toFlux(usersRepository.list())
+                .transform(ReactorUtils.<Username, Task.Result>throttle()
+                    .elements(runningOptions.getUsersPerSecond())
+                    .per(Duration.ofSeconds(1))
+                    .forOperation(username -> expireUserInbox(context, username, expiration)))
+                .reduce(Task.Result.COMPLETED, Task::combine);
+        } catch (UsersRepositoryException e) {
+            LOGGER.error("Error while accessing users from repository", e);
+            return Mono.just(Task.Result.PARTIAL);
+        }
+    }
+
+    private Mono<Result> expireUserInbox(Context context, Username username, SearchQuery expiration) {
+        MailboxSession session = mailboxManager.createSystemSession(username);
+        MailboxPath mailboxPath = MailboxPath.inbox(username);
+        return Mono.from(mailboxManager.getMailboxReactive(mailboxPath, session))
+            // newly created users do not have mailboxes yet, just skip them
+            .onErrorResume(MailboxNotFoundException.class, ignore -> Mono.empty())
+            .flatMap(mgr -> searchMessagesReactive(mgr, session, expiration)
+                .flatMap(list -> deleteMessagesReactive(mgr, session, list)))
+            .doOnNext(expired -> {
+                if (expired) {
+                    context.incrementExpiredCount();
+                }
+                context.incrementProcessedCount();
+            })
+            .then(Mono.just(Task.Result.COMPLETED))
+            .onErrorResume(e -> {
+                LOGGER.warn("Failed to expire user mailbox {}", username, e);
+                context.incrementFailedCount();
+                context.incrementProcessedCount();
+                return Mono.just(Task.Result.PARTIAL);
+            });
+    }
+
+    private Mono<List<MessageUid>> searchMessagesReactive(MessageManager mgr, MailboxSession session, SearchQuery expiration) {
+        try {
+            return Flux.from(mgr.search(expiration, session)).collectList();
+        } catch (MailboxException e) {
+            return Mono.error(e);
+        }
+    }
+
+    private Mono<Boolean> deleteMessagesReactive(MessageManager mgr, MailboxSession session, List<MessageUid> uids) {
+        try {
+            if (uids.isEmpty()) {
+                return Mono.just(false);
+            } else {
+                mgr.delete(uids, session);
+                return Mono.just(true);

Review Comment:
   Ok but at the very least wrap the blocking call for now ;-)
   
   I'd be happy to get one more reactify-everything-commit on my kill list.



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r879412604


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireInboxesService.java:
##########
@@ -0,0 +1,209 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+
+public class ExpireInboxesService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, Optional.empty());
+
+        private final int usersPerSecond;
+
+        private final Optional<String> maxAge;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("maxAge") Optional<String> maxAge) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' needs to be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+    
+            this.maxAge = maxAge;
+            this.maxAgeDuration = maxAge.map(v -> {
+                Duration maxAgeDuration = DurationParser.parse(maxAge.get(), ChronoUnit.DAYS);
+                Preconditions.checkArgument(!maxAgeDuration.isNegative(), "'maxAge' must be positive");
+                return maxAgeDuration;
+            });
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+
+        public Optional<String> getMaxAge() {
+            return maxAge;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireInboxesService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireInboxesService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireInboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            LOGGER.info("expire with maxAge {} = {}", runningOptions.maxAge, runningOptions.maxAgeDuration);
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)

Review Comment:
   I documented the byExpiresHeader variant as "Cassandra only". InMemory works too of course, but it is not relevant for deployments. I do not know if any of the other James server variants would work, probably not if they use ES.



-- 
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] ottoka commented on a diff in pull request #1004: James 3758 Endpoint to delete old emails from INBOXes

Posted by GitBox <gi...@apache.org>.
ottoka commented on code in PR #1004:
URL: https://github.com/apache/james-project/pull/1004#discussion_r880225009


##########
server/protocols/webadmin/webadmin-mailbox/src/main/java/org/apache/james/webadmin/service/ExpireMailboxService.java:
##########
@@ -0,0 +1,249 @@
+/****************************************************************
+ * 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.webadmin.service;
+
+import java.time.Duration;
+import java.time.temporal.ChronoUnit;
+import java.util.Date;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicLong;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.MessageUid;
+import org.apache.james.mailbox.exception.MailboxException;
+import org.apache.james.mailbox.exception.MailboxNotFoundException;
+import org.apache.james.mailbox.model.MailboxConstants;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.SearchQuery;
+import org.apache.james.mailbox.model.SearchQuery.DateResolution;
+import org.apache.james.task.Task;
+import org.apache.james.task.Task.Result;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.DurationParser;
+import org.apache.james.util.ReactorUtils;
+import org.apache.james.util.streams.Iterators;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.github.fge.lambdas.Throwing;
+import com.google.common.base.Preconditions;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+import reactor.core.scheduler.Schedulers;
+
+
+public class ExpireMailboxService {
+
+    public static class RunningOptions {
+        public static final RunningOptions DEFAULT = new RunningOptions(1, MailboxConstants.INBOX, true, Optional.empty());
+
+        public static RunningOptions fromParams(
+                Optional<String> byExpiresHeader, Optional<String> olderThan,
+                Optional<String> usersPerSecond, Optional<String> mailbox) {
+            try {
+                if (byExpiresHeader.isPresent() == olderThan.isPresent()) {
+                    throw new IllegalArgumentException("Must specify either 'olderThan' or 'byExpiresHeader' parameter");
+                }
+                return new RunningOptions(
+                    usersPerSecond.map(Integer::parseInt).orElse(DEFAULT.getUsersPerSecond()),
+                    mailbox.orElse(DEFAULT.getMailbox()), byExpiresHeader.isPresent(), olderThan);
+            } catch (NumberFormatException ex) {
+                throw new IllegalArgumentException("'usersPerSecond' must be numeric");
+            }
+        }
+        
+        private final int usersPerSecond;
+        
+        private final String mailbox;
+        
+        private final boolean byExpiresHeader;
+
+        private final Optional<String> olderThan;
+
+        @JsonIgnore
+        private final Optional<Duration> maxAgeDuration;
+
+        @JsonCreator
+        public RunningOptions(@JsonProperty("usersPerSecond") int usersPerSecond,
+                              @JsonProperty("mailbox") String mailbox,
+                              @JsonProperty("byExpiresHeader") boolean byExpiresHeader,
+                              @JsonProperty("olderThan") Optional<String> olderThan) {
+            Preconditions.checkArgument(usersPerSecond > 0, "'usersPerSecond' must be strictly positive");
+            this.usersPerSecond = usersPerSecond;
+            this.mailbox = mailbox;
+            this.byExpiresHeader = byExpiresHeader;
+            this.olderThan = olderThan;
+            this.maxAgeDuration = olderThan.map(v -> DurationParser.parse(olderThan.get(), ChronoUnit.DAYS));
+        }
+
+        public int getUsersPerSecond() {
+            return usersPerSecond;
+        }
+        
+        public String getMailbox() { 
+            return mailbox;
+        }
+
+        public boolean getByExpiresHeader() {
+            return byExpiresHeader;
+        }
+
+        public Optional<String> getOlderThan() {
+            return olderThan;
+        }
+    }
+
+    public static class Context {
+        private final AtomicLong inboxesExpired;
+        private final AtomicLong inboxesFailed;
+        private final AtomicLong inboxesProcessed;
+        private final AtomicLong messagesDeleted;
+
+        public Context() {
+            this.inboxesExpired = new AtomicLong(0L);
+            this.inboxesFailed = new AtomicLong(0L);
+            this.inboxesProcessed = new AtomicLong(0L);
+            this.messagesDeleted = new AtomicLong(0L);
+        }
+
+        public long getInboxesExpired() {
+            return inboxesExpired.get();
+        }
+
+        public long getInboxesFailed() {
+            return inboxesFailed.get();
+        }
+
+        public long getInboxesProcessed() {
+            return inboxesProcessed.get();
+        }
+
+        public long getMessagesDeleted() {
+            return messagesDeleted.get();
+        }
+
+        public void incrementExpiredCount() {
+            inboxesExpired.incrementAndGet();
+        }
+
+        public void incrementFailedCount() {
+            inboxesFailed.incrementAndGet();
+        }
+
+        public void incrementProcessedCount() {
+            inboxesProcessed.incrementAndGet();
+        }
+        
+        public void incrementMessagesDeleted(long count) { 
+            messagesDeleted.addAndGet(count);
+        }
+    }
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExpireMailboxService.class);
+
+    private final UsersRepository usersRepository;
+    private final MailboxManager mailboxManager;
+
+    @Inject
+    public ExpireMailboxService(UsersRepository usersRepository, MailboxManager mailboxManager) {
+        this.usersRepository = usersRepository;
+        this.mailboxManager = mailboxManager;
+    }
+
+    public Mono<Result> expireMailboxes(Context context, RunningOptions runningOptions, Date now) {
+        try {
+            SearchQuery expiration = SearchQuery.of(
+                runningOptions.maxAgeDuration.map(maxAge -> {
+                        Date limit = Date.from(now.toInstant().minus(maxAge));
+                        return SearchQuery.internalDateBefore(limit, DateResolution.Second);
+                    })
+                    .orElse(
+                        SearchQuery.headerDateBefore("Expires", now, DateResolution.Second)
+                    )
+            );
+            return Iterators.toFlux(usersRepository.list())
+                .transform(ReactorUtils.<Username, Task.Result>throttle()
+                    .elements(runningOptions.getUsersPerSecond())
+                    .per(Duration.ofSeconds(1))
+                    .forOperation(username -> 
+                        expireUserMailbox(context, username, runningOptions.getMailbox(), expiration)))
+                .reduce(Task.Result.COMPLETED, Task::combine);
+        } catch (UsersRepositoryException e) {
+            LOGGER.error("Error while accessing users from repository", e);
+            return Mono.just(Task.Result.PARTIAL);
+        }
+    }
+
+    private Mono<Result> expireUserMailbox(Context context, Username username, String mailbox, SearchQuery expiration) {
+        MailboxSession session = mailboxManager.createSystemSession(username);
+        MailboxPath mailboxPath = MailboxPath.forUser(username, mailbox);
+        return Mono.from(mailboxManager.getMailboxReactive(mailboxPath, session))
+            // newly created users do not have mailboxes yet, just skip them
+            .onErrorResume(MailboxNotFoundException.class, ignore -> Mono.empty())
+            .flatMap(mgr -> searchMessagesReactive(mgr, session, expiration)
+                .flatMap(list -> deleteMessagesReactive(mgr, session, list)))
+            .doOnNext(expired -> {
+                if (expired > 0) {
+                    context.incrementExpiredCount();
+                    context.incrementMessagesDeleted(expired);
+                }
+                context.incrementProcessedCount();
+            })
+            .then(Mono.just(Task.Result.COMPLETED))
+            .onErrorResume(e -> {
+                LOGGER.warn("Failed to expire user mailbox {}", username, e);
+                context.incrementFailedCount();
+                context.incrementProcessedCount();
+                return Mono.just(Task.Result.PARTIAL);
+            });
+    }
+
+    private Mono<List<MessageUid>> searchMessagesReactive(MessageManager mgr, MailboxSession session, SearchQuery expiration) {
+        try {
+            return Flux.from(mgr.search(expiration, session)).collectList();
+        } catch (MailboxException e) {
+            return Mono.error(e);
+        }
+    }
+
+    private Mono<Integer> deleteMessagesReactive(MessageManager mgr, MailboxSession session, List<MessageUid> uids) {
+        if (uids.isEmpty()) {
+            return Mono.just(0);
+        } else {
+            return Mono.fromSupplier(

Review Comment:
   Right, since a callable may throw, this should handle the exception. Still more to learn about Reaktor, sigh.



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