chibenwa commented on code in PR #1501:
URL: https://github.com/apache/james-project/pull/1501#discussion_r1166181451


##########
mailet/api/src/main/java/org/apache/mailet/Mail.java:
##########
@@ -451,4 +451,4 @@ default void setDsnParameters(DsnParameters dsnParameters) {
             .asAttributes()
             .forEach(this::setAttribute);
     }
-}
+}

Review Comment:
   idem unrelated



##########
server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/futurerelease/FutureReleaseEHLOHook.java:
##########
@@ -0,0 +1,43 @@
+/****************************************************************
+ * 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.smtpserver.futurerelease;
+
+import java.util.Set;
+
+import org.apache.james.protocols.smtp.SMTPSession;
+import org.apache.james.protocols.smtp.hook.HeloHook;
+import org.apache.james.protocols.smtp.hook.HookResult;
+
+import com.google.common.collect.ImmutableSet;
+
+public class FutureReleaseEHLOHook implements HeloHook {
+    private static final long MAX_FUTURE_RELEASE_INTERVAL = 86400;
+    private static final String MAX_FUTURE_RELEASE_DATE_TIME = 
"2023-04-14T10:00:00Z";

Review Comment:
   Hard coding the date do not make sens!
   
   Get it from a `Clock`



##########
server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/futurerelease/FutureReleaseMessageHook.java:
##########
@@ -0,0 +1,57 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.smtpserver.futurerelease;
+
+import static 
org.apache.james.smtpserver.futurerelease.FutureReleaseMailParameterHook.FUTURERELEASE_HOLDFOR;
+
+import java.util.Optional;
+
+import org.apache.james.protocols.api.ProtocolSession;
+import org.apache.james.protocols.smtp.SMTPSession;
+import org.apache.james.protocols.smtp.hook.HookResult;
+import org.apache.james.smtpserver.JamesMessageHook;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.AttributeName;
+import org.apache.mailet.AttributeValue;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class FutureReleaseMessageHook implements JamesMessageHook {

Review Comment:
   > Completly delete this class if SendMailHandler works with SMTPSession 
attachments directly
   
   ?



##########
protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/MailCmdHandler.java:
##########
@@ -138,7 +138,6 @@ protected Response doFilterChecks(SMTPSession session, 
String command,
      */
     private Response doMAILFilter(SMTPSession session, String argument) {
         String sender = null;
-

Review Comment:
   unrelated



##########
mailet/api/src/main/java/org/apache/mailet/DsnParameters.java:
##########
@@ -44,7 +44,6 @@
 
 /**
  * Represents DSN parameters attached to the envelope of an Email transiting 
over SMTP
- *

Review Comment:
   unrelated



##########
server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/futurerelease/FutureReleaseMailParameterHook.java:
##########
@@ -0,0 +1,107 @@
+/****************************************************************
+ * 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.smtpserver.futurerelease;
+
+import static org.apache.james.protocols.api.ProtocolSession.State.Transaction;
+import static 
org.apache.james.smtpserver.futurerelease.FutureReleaseParameters.HOLDFOR_PARAMETER;
+import static 
org.apache.james.smtpserver.futurerelease.FutureReleaseParameters.HOLDUNTIL_PARAMETER;
+import static 
org.apache.james.smtpserver.futurerelease.FutureReleaseParameters.MAX_HOLD_FOR_SUPPORTED;
+
+import java.time.Duration;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.time.format.DateTimeParseException;
+
+import org.apache.james.protocols.api.ProtocolSession;
+import org.apache.james.protocols.smtp.SMTPSession;
+import org.apache.james.protocols.smtp.hook.HookResult;
+import org.apache.james.protocols.smtp.hook.MailParametersHook;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class FutureReleaseMailParameterHook implements MailParametersHook {
+
+    private static final int UTC_TIMESTAMP_LENGTH = 20;
+    private static final int ZONED_TIMESTAMP_LENGTH = 25;
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(FutureReleaseMailParameterHook.class);
+    public static final 
ProtocolSession.AttachmentKey<FutureReleaseParameters.HoldFor> 
FUTURERELEASE_HOLDFOR = 
ProtocolSession.AttachmentKey.of("FUTURERELEASE_HOLDFOR", 
FutureReleaseParameters.HoldFor.class);
+    private static final Instant now = 
ZonedDateTime.parse("2023-04-13T11:00:00Z").toInstant();
+    public static final long INVALID_HOLDUNTIL_VALUE = -1L;
+    private static final long NUMBER_PARSE_EXCEPTION = -2L;
+    private static final long DATE_TIME_FORMAT_EXCEPTION = -3L;
+    private static final long INVALID_PARAM_NAME = -4L;

Review Comment:
   We are not in `C` we do not need to use numbers to encode exceptions.



##########
server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/futurerelease/FutureReleaseMailParameterHook.java:
##########
@@ -0,0 +1,107 @@
+/****************************************************************
+ * 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.smtpserver.futurerelease;
+
+import static org.apache.james.protocols.api.ProtocolSession.State.Transaction;
+import static 
org.apache.james.smtpserver.futurerelease.FutureReleaseParameters.HOLDFOR_PARAMETER;
+import static 
org.apache.james.smtpserver.futurerelease.FutureReleaseParameters.HOLDUNTIL_PARAMETER;
+import static 
org.apache.james.smtpserver.futurerelease.FutureReleaseParameters.MAX_HOLD_FOR_SUPPORTED;
+
+import java.time.Duration;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.time.format.DateTimeParseException;
+
+import org.apache.james.protocols.api.ProtocolSession;
+import org.apache.james.protocols.smtp.SMTPSession;
+import org.apache.james.protocols.smtp.hook.HookResult;
+import org.apache.james.protocols.smtp.hook.MailParametersHook;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class FutureReleaseMailParameterHook implements MailParametersHook {
+
+    private static final int UTC_TIMESTAMP_LENGTH = 20;
+    private static final int ZONED_TIMESTAMP_LENGTH = 25;
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(FutureReleaseMailParameterHook.class);
+    public static final 
ProtocolSession.AttachmentKey<FutureReleaseParameters.HoldFor> 
FUTURERELEASE_HOLDFOR = 
ProtocolSession.AttachmentKey.of("FUTURERELEASE_HOLDFOR", 
FutureReleaseParameters.HoldFor.class);
+    private static final Instant now = 
ZonedDateTime.parse("2023-04-13T11:00:00Z").toInstant();
+    public static final long INVALID_HOLDUNTIL_VALUE = -1L;
+    private static final long NUMBER_PARSE_EXCEPTION = -2L;
+    private static final long DATE_TIME_FORMAT_EXCEPTION = -3L;
+    private static final long INVALID_PARAM_NAME = -4L;
+
+    @Override
+    public HookResult doMailParameter(SMTPSession session, String paramName, 
String paramValue) {
+        long requestedHoldFor = evaluateHoldFor(paramName, paramValue);
+        if (requestedHoldFor > MAX_HOLD_FOR_SUPPORTED) {
+            LOGGER.debug("HoldFor is greater than max-future-release-interval 
or holdUntil exceeded max-future-release-date-time");
+            return HookResult.DENY;
+        }
+        if (requestedHoldFor < 0) {
+            LOGGER.debug("HoldFor value is negative or holdUntil value is 
before now");
+            return HookResult.DENY;
+        }
+        if (session.getAttachment(FUTURERELEASE_HOLDFOR, 
Transaction).isPresent()) {
+            LOGGER.debug("Mail parameter cannot contains both holdFor and 
holdUntil parameters");
+            return HookResult.DENY;
+        }
+        session.setAttachment(FUTURERELEASE_HOLDFOR, 
FutureReleaseParameters.HoldFor.of(requestedHoldFor), Transaction);
+        return HookResult.DECLINED;
+    }
+
+    private static Long evaluateHoldFor(String paramName, String paramValue) {
+        try {
+            if (paramName.equals(HOLDFOR_PARAMETER)) {
+                return Long.parseLong(paramValue);
+            }
+            if 
(selectDateTimeFormatter(paramValue).equals(DateTimeFormatter.ISO_DATE)) {
+                return INVALID_HOLDUNTIL_VALUE;
+            }
+            if (paramName.equals(HOLDUNTIL_PARAMETER)) {
+                DateTimeFormatter formatter = 
selectDateTimeFormatter(paramValue);
+                return Duration.between(now, ZonedDateTime.parse(paramValue, 
formatter).toInstant()).toSeconds();
+            }
+        } catch (NumberFormatException e) {
+            LOGGER.debug("Failed to parse number format");
+            return NUMBER_PARSE_EXCEPTION;
+        } catch (DateTimeParseException e) {
+            LOGGER.debug("Failed to parse date format");
+            return DATE_TIME_FORMAT_EXCEPTION;
+        }
+        return INVALID_PARAM_NAME;
+    }
+
+    private static DateTimeFormatter selectDateTimeFormatter(String dateTime) {
+        if (dateTime.length() == UTC_TIMESTAMP_LENGTH) {
+            return DateTimeFormatter.ISO_INSTANT.withZone(ZoneId.of("Z"));
+        }
+        if (dateTime.length() == ZONED_TIMESTAMP_LENGTH) {
+            return DateTimeFormatter.ISO_OFFSET_DATE_TIME;
+        }
+        return DateTimeFormatter.ISO_DATE;
+    }
+    @Override
+    public String[] getMailParamNames() {

Review Comment:
   ```suggestion
       }
       
       @Override
       public String[] getMailParamNames() {
   ```



##########
server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/FutureReleaseTest.java:
##########
@@ -0,0 +1,359 @@
+/****************************************************************
+ * 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.smtpserver;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Base64;
+
+import org.apache.commons.configuration2.BaseHierarchicalConfiguration;
+import org.apache.commons.net.smtp.SMTPClient;
+import org.apache.james.UserEntityValidator;
+import org.apache.james.core.Domain;
+import org.apache.james.core.Username;
+import org.apache.james.dnsservice.api.DNSService;
+import org.apache.james.dnsservice.api.InMemoryDNSService;
+import org.apache.james.domainlist.api.DomainList;
+import org.apache.james.domainlist.lib.DomainListConfiguration;
+import org.apache.james.domainlist.memory.MemoryDomainList;
+import org.apache.james.filesystem.api.FileSystem;
+import org.apache.james.mailbox.Authorizator;
+import org.apache.james.mailrepository.api.MailRepositoryStore;
+import org.apache.james.mailrepository.api.Protocol;
+import org.apache.james.mailrepository.memory.MailRepositoryStoreConfiguration;
+import org.apache.james.mailrepository.memory.MemoryMailRepository;
+import org.apache.james.mailrepository.memory.MemoryMailRepositoryStore;
+import org.apache.james.mailrepository.memory.MemoryMailRepositoryUrlStore;
+import org.apache.james.mailrepository.memory.SimpleMailRepositoryLoader;
+import org.apache.james.metrics.api.Metric;
+import org.apache.james.metrics.api.MetricFactory;
+import org.apache.james.metrics.tests.RecordingMetricFactory;
+import org.apache.james.protocols.api.utils.ProtocolServerUtils;
+import org.apache.james.protocols.lib.mock.MockProtocolHandlerLoader;
+import org.apache.james.queue.api.MailQueueFactory;
+import org.apache.james.queue.api.RawMailQueueItemDecoratorFactory;
+import org.apache.james.queue.memory.MemoryMailQueueFactory;
+import org.apache.james.rrt.api.AliasReverseResolver;
+import org.apache.james.rrt.api.CanSendFrom;
+import org.apache.james.rrt.api.RecipientRewriteTable;
+import org.apache.james.rrt.api.RecipientRewriteTableConfiguration;
+import org.apache.james.rrt.lib.AliasReverseResolverImpl;
+import org.apache.james.rrt.lib.CanSendFromImpl;
+import org.apache.james.rrt.memory.MemoryRecipientRewriteTable;
+import org.apache.james.server.core.configuration.Configuration;
+import org.apache.james.server.core.configuration.FileConfigurationProvider;
+import org.apache.james.server.core.filesystem.FileSystemImpl;
+import org.apache.james.smtpserver.futurerelease.FutureReleaseParameters;
+import org.apache.james.smtpserver.netty.SMTPServer;
+import org.apache.james.smtpserver.netty.SmtpMetricsImpl;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.memory.MemoryUsersRepository;
+import org.assertj.core.api.SoftAssertions;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import com.google.common.collect.ImmutableList;
+import com.google.inject.TypeLiteral;
+
+import nl.jqno.equalsverifier.EqualsVerifier;
+
+public class FutureReleaseTest {
+    public static final String LOCAL_DOMAIN = "example.local";
+    public static final Username BOB = Username.of("bob@localhost");
+    public static final String PASSWORD = "bobpwd";
+
+    protected MemoryDomainList domainList;
+    protected MemoryUsersRepository usersRepository;
+    protected SMTPServerTest.AlterableDNSServer dnsServer;
+    protected MemoryMailRepositoryStore mailRepositoryStore;
+    protected FileSystemImpl fileSystem;
+    protected Configuration configuration;
+    protected MockProtocolHandlerLoader chain;
+    protected MemoryMailQueueFactory queueFactory;
+    protected MemoryMailQueueFactory.MemoryCacheableMailQueue queue;
+
+    private SMTPServer smtpServer;
+
+    @BeforeEach
+    void setUp() throws Exception {
+        domainList = new MemoryDomainList(new InMemoryDNSService());
+        domainList.configure(DomainListConfiguration.DEFAULT);
+
+        domainList.addDomain(Domain.of(LOCAL_DOMAIN));
+        domainList.addDomain(Domain.of("examplebis.local"));
+        usersRepository = MemoryUsersRepository.withVirtualHosting(domainList);
+        usersRepository.addUser(BOB, PASSWORD);
+
+        createMailRepositoryStore();
+
+        setUpFakeLoader();
+        setUpSMTPServer();
+
+        smtpServer.configure(FileConfigurationProvider.getConfig(
+            
ClassLoader.getSystemResourceAsStream("smtpserver-futurerelease.xml")));
+        smtpServer.init();
+    }
+
+    protected void createMailRepositoryStore() throws Exception {
+        configuration = Configuration.builder()
+            .workingDirectory("../")
+            .configurationFromClasspath()
+            .build();
+        fileSystem = new FileSystemImpl(configuration.directories());
+        MemoryMailRepositoryUrlStore urlStore = new 
MemoryMailRepositoryUrlStore();
+
+        MailRepositoryStoreConfiguration configuration = 
MailRepositoryStoreConfiguration.forItems(
+            new MailRepositoryStoreConfiguration.Item(
+                ImmutableList.of(new Protocol("memory")),
+                MemoryMailRepository.class.getName(),
+                new BaseHierarchicalConfiguration()));
+
+        mailRepositoryStore = new MemoryMailRepositoryStore(urlStore, new 
SimpleMailRepositoryLoader(), configuration);
+        mailRepositoryStore.init();
+    }
+
+    protected SMTPServer createSMTPServer(SmtpMetricsImpl smtpMetrics) {
+        return new SMTPServer(smtpMetrics);
+    }
+
+    protected void setUpSMTPServer() {
+        SmtpMetricsImpl smtpMetrics = mock(SmtpMetricsImpl.class);
+        when(smtpMetrics.getCommandsMetric()).thenReturn(mock(Metric.class));
+        when(smtpMetrics.getConnectionMetric()).thenReturn(mock(Metric.class));
+        smtpServer = createSMTPServer(smtpMetrics);
+        smtpServer.setDnsService(dnsServer);
+        smtpServer.setFileSystem(fileSystem);
+        smtpServer.setProtocolHandlerLoader(chain);
+    }
+
+    protected void setUpFakeLoader() {
+        dnsServer = new SMTPServerTest.AlterableDNSServer();
+
+        MemoryRecipientRewriteTable rewriteTable = new 
MemoryRecipientRewriteTable();
+        
rewriteTable.setConfiguration(RecipientRewriteTableConfiguration.DEFAULT_ENABLED);
+        AliasReverseResolver aliasReverseResolver = new 
AliasReverseResolverImpl(rewriteTable);
+        CanSendFrom canSendFrom = new CanSendFromImpl(rewriteTable, 
aliasReverseResolver);
+        queueFactory = new MemoryMailQueueFactory(new 
RawMailQueueItemDecoratorFactory());
+        queue = queueFactory.createQueue(MailQueueFactory.SPOOL);
+
+        chain = MockProtocolHandlerLoader.builder()
+            .put(binder -> 
binder.bind(DomainList.class).toInstance(domainList))
+            .put(binder -> binder.bind(new TypeLiteral<MailQueueFactory<?>>() 
{}).toInstance(queueFactory))
+            .put(binder -> 
binder.bind(RecipientRewriteTable.class).toInstance(rewriteTable))
+            .put(binder -> 
binder.bind(CanSendFrom.class).toInstance(canSendFrom))
+            .put(binder -> 
binder.bind(FileSystem.class).toInstance(fileSystem))
+            .put(binder -> 
binder.bind(MailRepositoryStore.class).toInstance(mailRepositoryStore))
+            .put(binder -> binder.bind(DNSService.class).toInstance(dnsServer))
+            .put(binder -> 
binder.bind(UsersRepository.class).toInstance(usersRepository))
+            .put(binder -> 
binder.bind(MetricFactory.class).to(RecordingMetricFactory.class))
+            .put(binder -> 
binder.bind(UserEntityValidator.class).toInstance(UserEntityValidator.NOOP))
+            .put(binder -> binder.bind(Authorizator.class).toInstance((userId, 
otherUserId) -> Authorizator.AuthorizationState.ALLOWED))
+            .build();
+    }
+
+    @AfterEach
+    void tearDown() {
+        smtpServer.destroy();
+    }
+
+    @Test
+    void testEqualsVerifiersForHoldForClass() throws Exception {
+        SMTPClient smtpProtocol = new SMTPClient();
+        InetSocketAddress bindedAddress = new 
ProtocolServerUtils(smtpServer).retrieveBindedAddress();
+        smtpProtocol.connect(bindedAddress.getAddress().getHostAddress(), 
bindedAddress.getPort());
+        authenticate(smtpProtocol);
+        smtpProtocol.sendCommand("EHLO localhost");
+        
EqualsVerifier.forClass(FutureReleaseParameters.HoldFor.class).verify();
+    }
+
+    @Test
+    void testEqualsVerifierForHoldUntilClass() throws Exception {
+        SMTPClient smtpProtocol = new SMTPClient();
+        InetSocketAddress bindedAddress = new 
ProtocolServerUtils(smtpServer).retrieveBindedAddress();
+        smtpProtocol.connect(bindedAddress.getAddress().getHostAddress(), 
bindedAddress.getPort());
+        authenticate(smtpProtocol);
+        smtpProtocol.sendCommand("EHLO localhost");
+        
EqualsVerifier.forClass(FutureReleaseParameters.HoldFor.class).verify();
+    }

Review Comment:
   Equals verifiers can be moved in another class and shall be alone in their 
test class



##########
server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/FutureReleaseTest.java:
##########
@@ -0,0 +1,359 @@
+/****************************************************************
+ * 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.smtpserver;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Base64;
+
+import org.apache.commons.configuration2.BaseHierarchicalConfiguration;
+import org.apache.commons.net.smtp.SMTPClient;
+import org.apache.james.UserEntityValidator;
+import org.apache.james.core.Domain;
+import org.apache.james.core.Username;
+import org.apache.james.dnsservice.api.DNSService;
+import org.apache.james.dnsservice.api.InMemoryDNSService;
+import org.apache.james.domainlist.api.DomainList;
+import org.apache.james.domainlist.lib.DomainListConfiguration;
+import org.apache.james.domainlist.memory.MemoryDomainList;
+import org.apache.james.filesystem.api.FileSystem;
+import org.apache.james.mailbox.Authorizator;
+import org.apache.james.mailrepository.api.MailRepositoryStore;
+import org.apache.james.mailrepository.api.Protocol;
+import org.apache.james.mailrepository.memory.MailRepositoryStoreConfiguration;
+import org.apache.james.mailrepository.memory.MemoryMailRepository;
+import org.apache.james.mailrepository.memory.MemoryMailRepositoryStore;
+import org.apache.james.mailrepository.memory.MemoryMailRepositoryUrlStore;
+import org.apache.james.mailrepository.memory.SimpleMailRepositoryLoader;
+import org.apache.james.metrics.api.Metric;
+import org.apache.james.metrics.api.MetricFactory;
+import org.apache.james.metrics.tests.RecordingMetricFactory;
+import org.apache.james.protocols.api.utils.ProtocolServerUtils;
+import org.apache.james.protocols.lib.mock.MockProtocolHandlerLoader;
+import org.apache.james.queue.api.MailQueueFactory;
+import org.apache.james.queue.api.RawMailQueueItemDecoratorFactory;
+import org.apache.james.queue.memory.MemoryMailQueueFactory;
+import org.apache.james.rrt.api.AliasReverseResolver;
+import org.apache.james.rrt.api.CanSendFrom;
+import org.apache.james.rrt.api.RecipientRewriteTable;
+import org.apache.james.rrt.api.RecipientRewriteTableConfiguration;
+import org.apache.james.rrt.lib.AliasReverseResolverImpl;
+import org.apache.james.rrt.lib.CanSendFromImpl;
+import org.apache.james.rrt.memory.MemoryRecipientRewriteTable;
+import org.apache.james.server.core.configuration.Configuration;
+import org.apache.james.server.core.configuration.FileConfigurationProvider;
+import org.apache.james.server.core.filesystem.FileSystemImpl;
+import org.apache.james.smtpserver.futurerelease.FutureReleaseParameters;
+import org.apache.james.smtpserver.netty.SMTPServer;
+import org.apache.james.smtpserver.netty.SmtpMetricsImpl;
+import org.apache.james.user.api.UsersRepository;
+import org.apache.james.user.memory.MemoryUsersRepository;
+import org.assertj.core.api.SoftAssertions;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import com.google.common.collect.ImmutableList;
+import com.google.inject.TypeLiteral;
+
+import nl.jqno.equalsverifier.EqualsVerifier;
+
+public class FutureReleaseTest {
+    public static final String LOCAL_DOMAIN = "example.local";
+    public static final Username BOB = Username.of("bob@localhost");
+    public static final String PASSWORD = "bobpwd";
+
+    protected MemoryDomainList domainList;
+    protected MemoryUsersRepository usersRepository;
+    protected SMTPServerTest.AlterableDNSServer dnsServer;
+    protected MemoryMailRepositoryStore mailRepositoryStore;
+    protected FileSystemImpl fileSystem;
+    protected Configuration configuration;
+    protected MockProtocolHandlerLoader chain;
+    protected MemoryMailQueueFactory queueFactory;
+    protected MemoryMailQueueFactory.MemoryCacheableMailQueue queue;
+
+    private SMTPServer smtpServer;
+
+    @BeforeEach
+    void setUp() throws Exception {
+        domainList = new MemoryDomainList(new InMemoryDNSService());
+        domainList.configure(DomainListConfiguration.DEFAULT);
+
+        domainList.addDomain(Domain.of(LOCAL_DOMAIN));
+        domainList.addDomain(Domain.of("examplebis.local"));
+        usersRepository = MemoryUsersRepository.withVirtualHosting(domainList);
+        usersRepository.addUser(BOB, PASSWORD);
+
+        createMailRepositoryStore();
+
+        setUpFakeLoader();
+        setUpSMTPServer();
+
+        smtpServer.configure(FileConfigurationProvider.getConfig(
+            
ClassLoader.getSystemResourceAsStream("smtpserver-futurerelease.xml")));
+        smtpServer.init();
+    }
+
+    protected void createMailRepositoryStore() throws Exception {
+        configuration = Configuration.builder()
+            .workingDirectory("../")
+            .configurationFromClasspath()
+            .build();
+        fileSystem = new FileSystemImpl(configuration.directories());
+        MemoryMailRepositoryUrlStore urlStore = new 
MemoryMailRepositoryUrlStore();
+
+        MailRepositoryStoreConfiguration configuration = 
MailRepositoryStoreConfiguration.forItems(
+            new MailRepositoryStoreConfiguration.Item(
+                ImmutableList.of(new Protocol("memory")),
+                MemoryMailRepository.class.getName(),
+                new BaseHierarchicalConfiguration()));
+
+        mailRepositoryStore = new MemoryMailRepositoryStore(urlStore, new 
SimpleMailRepositoryLoader(), configuration);
+        mailRepositoryStore.init();
+    }
+
+    protected SMTPServer createSMTPServer(SmtpMetricsImpl smtpMetrics) {
+        return new SMTPServer(smtpMetrics);
+    }
+
+    protected void setUpSMTPServer() {
+        SmtpMetricsImpl smtpMetrics = mock(SmtpMetricsImpl.class);
+        when(smtpMetrics.getCommandsMetric()).thenReturn(mock(Metric.class));
+        when(smtpMetrics.getConnectionMetric()).thenReturn(mock(Metric.class));
+        smtpServer = createSMTPServer(smtpMetrics);
+        smtpServer.setDnsService(dnsServer);
+        smtpServer.setFileSystem(fileSystem);
+        smtpServer.setProtocolHandlerLoader(chain);
+    }
+
+    protected void setUpFakeLoader() {
+        dnsServer = new SMTPServerTest.AlterableDNSServer();
+
+        MemoryRecipientRewriteTable rewriteTable = new 
MemoryRecipientRewriteTable();
+        
rewriteTable.setConfiguration(RecipientRewriteTableConfiguration.DEFAULT_ENABLED);
+        AliasReverseResolver aliasReverseResolver = new 
AliasReverseResolverImpl(rewriteTable);
+        CanSendFrom canSendFrom = new CanSendFromImpl(rewriteTable, 
aliasReverseResolver);
+        queueFactory = new MemoryMailQueueFactory(new 
RawMailQueueItemDecoratorFactory());
+        queue = queueFactory.createQueue(MailQueueFactory.SPOOL);
+
+        chain = MockProtocolHandlerLoader.builder()
+            .put(binder -> 
binder.bind(DomainList.class).toInstance(domainList))
+            .put(binder -> binder.bind(new TypeLiteral<MailQueueFactory<?>>() 
{}).toInstance(queueFactory))
+            .put(binder -> 
binder.bind(RecipientRewriteTable.class).toInstance(rewriteTable))
+            .put(binder -> 
binder.bind(CanSendFrom.class).toInstance(canSendFrom))
+            .put(binder -> 
binder.bind(FileSystem.class).toInstance(fileSystem))
+            .put(binder -> 
binder.bind(MailRepositoryStore.class).toInstance(mailRepositoryStore))
+            .put(binder -> binder.bind(DNSService.class).toInstance(dnsServer))
+            .put(binder -> 
binder.bind(UsersRepository.class).toInstance(usersRepository))
+            .put(binder -> 
binder.bind(MetricFactory.class).to(RecordingMetricFactory.class))
+            .put(binder -> 
binder.bind(UserEntityValidator.class).toInstance(UserEntityValidator.NOOP))
+            .put(binder -> binder.bind(Authorizator.class).toInstance((userId, 
otherUserId) -> Authorizator.AuthorizationState.ALLOWED))
+            .build();
+    }
+
+    @AfterEach
+    void tearDown() {
+        smtpServer.destroy();
+    }
+
+    @Test
+    void testEqualsVerifiersForHoldForClass() throws Exception {
+        SMTPClient smtpProtocol = new SMTPClient();
+        InetSocketAddress bindedAddress = new 
ProtocolServerUtils(smtpServer).retrieveBindedAddress();
+        smtpProtocol.connect(bindedAddress.getAddress().getHostAddress(), 
bindedAddress.getPort());
+        authenticate(smtpProtocol);
+        smtpProtocol.sendCommand("EHLO localhost");
+        
EqualsVerifier.forClass(FutureReleaseParameters.HoldFor.class).verify();
+    }
+
+    @Test
+    void testEqualsVerifierForHoldUntilClass() throws Exception {
+        SMTPClient smtpProtocol = new SMTPClient();
+        InetSocketAddress bindedAddress = new 
ProtocolServerUtils(smtpServer).retrieveBindedAddress();
+        smtpProtocol.connect(bindedAddress.getAddress().getHostAddress(), 
bindedAddress.getPort());
+        authenticate(smtpProtocol);
+        smtpProtocol.sendCommand("EHLO localhost");
+        
EqualsVerifier.forClass(FutureReleaseParameters.HoldFor.class).verify();
+    }
+
+    @Test
+    void ehloShouldAdvertiseFutureReleaseExtension() throws Exception {
+        SMTPClient smtpProtocol = new SMTPClient();
+        InetSocketAddress bindedAddress = new 
ProtocolServerUtils(smtpServer).retrieveBindedAddress();
+        smtpProtocol.connect(bindedAddress.getAddress().getHostAddress(), 
bindedAddress.getPort());
+        authenticate(smtpProtocol);
+        smtpProtocol.sendCommand("EHLO localhost");
+
+        SoftAssertions.assertSoftly(softly -> {
+            softly.assertThat(smtpProtocol.getReplyCode()).isEqualTo(250);
+            softly.assertThat(smtpProtocol.getReplyString()).contains("250 
FUTURERELEASE 86400 2023-04-14T10:00:00Z");
+        });
+    }
+
+    private void authenticate(SMTPClient smtpProtocol) throws IOException {
+        smtpProtocol.sendCommand("AUTH PLAIN");
+        smtpProtocol.sendCommand(Base64.getEncoder().encodeToString(("\0" + 
BOB.asString() + "\0" + PASSWORD + "\0").getBytes(UTF_8)));
+        assertThat(smtpProtocol.getReplyCode())
+            .as("authenticated")
+            .isEqualTo(235);
+    }
+
+    @Test
+    void testSuccessCaseWithHoldForParams() throws Exception {
+        SMTPClient smtpProtocol = new SMTPClient();
+        InetSocketAddress bindedAddress = new 
ProtocolServerUtils(smtpServer).retrieveBindedAddress();
+        smtpProtocol.connect(bindedAddress.getAddress().getHostAddress(), 
bindedAddress.getPort());
+        authenticate(smtpProtocol);
+
+        smtpProtocol.sendCommand("EHLO localhost");
+        smtpProtocol.sendCommand("MAIL FROM: <bob@localhost> HOLDFOR=83200");
+        smtpProtocol.sendCommand("RCPT TO:<rcpt@localhost>");
+        smtpProtocol.sendShortMessageData("Subject: test mail\r\n\r\nTest body 
testSimpleMailSendWithFutureRelease\r\n.\r\n");
+
+        SoftAssertions.assertSoftly(softly -> {
+            softly.assertThat(smtpProtocol.getReplyCode()).isEqualTo(250);
+            softly.assertThat(smtpProtocol.getReplyString()).contains("250");
+        });

Review Comment:
   Please instead assert that the mail ended up in the mail queue



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]


Reply via email to