You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "Maxxx873 (via GitHub)" <gi...@apache.org> on 2024/07/03 13:52:59 UTC
[PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Maxxx873 opened a new pull request, #2337:
URL: https://github.com/apache/james-project/pull/2337
[JAMES-3824](https://issues.apache.org/jira/projects/JAMES/issues/JAMES-3824?filter=allopenissues)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #2337:
URL: https://github.com/apache/james-project/pull/2337#issuecomment-2206409721
(a rebase is needed to solve conflict with distributed server doc relocation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2337:
URL: https://github.com/apache/james-project/pull/2337#discussion_r1693957765
##########
docs/modules/servers/partials/RemoteDelivery.adoc:
##########
@@ -74,6 +74,9 @@ within the mailet code.<br>
Note: This facility should be used with extreme care by expert users with a thorough knowledge of the relevant RFCs and
the ability to perform their own problem resolutions.
* *debug* (optional) - a Boolean (true/false) indicating whether debugging is on. Default is false.
+* *usePriority* (optional) - a Boolean value (true/false) that indicates whether email priority support is enabled.
+When this option is enabled, any email that does not have a specified priority will be assigned the highest priority upon
+delivery and the lowest priority during retry attempts. Default is false.
Review Comment:
If enabled, I think we should not communicate priorities for mails that do not have one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "Maxxx873 (via GitHub)" <gi...@apache.org>.
Maxxx873 commented on code in PR #2337:
URL: https://github.com/apache/james-project/pull/2337#discussion_r1700819883
##########
docs/modules/servers/partials/RemoteDelivery.adoc:
##########
@@ -74,6 +74,9 @@ within the mailet code.<br>
Note: This facility should be used with extreme care by expert users with a thorough knowledge of the relevant RFCs and
the ability to perform their own problem resolutions.
* *debug* (optional) - a Boolean (true/false) indicating whether debugging is on. Default is false.
+* *usePriority* (optional) - a Boolean value (true/false) that indicates whether email priority support is enabled.
+When this option is enabled, any email that does not have a specified priority will be assigned the highest priority upon
+delivery and the lowest priority during retry attempts. Default is false.
Review Comment:
> If this property `usePriority` is enabled but does not affect emails without priorities set, I don't quite understand what it is for. Based on my understanding, this property was initially designed to hardcode email priorities through configuration, with the highest priority assigned on send and the lowest on retry. To maintain the original intent while also supporting the new priority functionality, I propose using this property to assign priorities to emails that currently lack a specified priority.
>
> I also have a couple of alternative suggestions:
>
> 1. Always assign the highest priority when sending and the lowest when retrying, irrespective of any priority specified in the email, if `usePriority` is enabled. .
> 2. Eliminate this property entirely and only consider the priority if it is explicitly set in the email.
>
> What are your thoughts on these options?
I chose the third option. I left only the lowest priority for retry attempts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #2337:
URL: https://github.com/apache/james-project/pull/2337#issuecomment-2264380325
A last obe test for remoteDelivery
GIVEN usePriority is false
WHEN i send a mail with priority 8
THEN It gets transmitted without priority
(See comlent above)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2337:
URL: https://github.com/apache/james-project/pull/2337#discussion_r1693958068
##########
server/mailet/mailets/src/test/java/org/apache/james/transport/mailets/remote/delivery/DeliveryRunnableTest.java:
##########
@@ -240,6 +245,29 @@ void deliveryTemporaryFailureShouldResetDeliveryCountOnNonErrorState() throws Ex
verify(mailQueue).enQueue(FakeMail.builder()
.name("name")
.attribute(DeliveryRetriesHelper.makeAttribute(1))
+ .attribute(MailPrioritySupport.LOW_PRIORITY_ATTRIBUTE)
+ .attribute(new Attribute(IS_DELIVERY_PERMANENT_ERROR, AttributeValue.of(false)))
+ .state(Mail.ERROR)
+ .lastUpdated(FIXED_DATE)
+ .build(),
+ Duration.ofSeconds(1));
+ verifyNoMoreInteractions(mailQueue);
+ }
+
+ @Test
+ void deliveryTemporaryFailureShouldRetryDeliveryWithPreservedPriority() throws Exception {
Review Comment:
We should ensure that all outgoing components are able to set priorities based on the outgoing queue content too.
See https://github.com/apache/james-project/tree/master/server/mailet/remote-delivery-integration-testing for writing such tests.
I would expect that James transmit the mails precisely with the priority I did submit it with.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "Maxxx873 (via GitHub)" <gi...@apache.org>.
Maxxx873 commented on PR #2337:
URL: https://github.com/apache/james-project/pull/2337#issuecomment-2206793598
As always, I'm amazed at the level of expertise in the code review, thank you! I will be happy to continue working on all comments
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2337:
URL: https://github.com/apache/james-project/pull/2337#discussion_r1701106480
##########
docs/modules/servers/partials/RemoteDelivery.adoc:
##########
@@ -74,6 +74,9 @@ within the mailet code.<br>
Note: This facility should be used with extreme care by expert users with a thorough knowledge of the relevant RFCs and
the ability to perform their own problem resolutions.
* *debug* (optional) - a Boolean (true/false) indicating whether debugging is on. Default is false.
+* *usePriority* (optional) - a Boolean value (true/false) that indicates whether email priority support is enabled.
+When this option is enabled, any email that does not have a specified priority will be assigned the highest priority upon
+delivery and the lowest priority during retry attempts. Default is false.
Review Comment:
> only consider the priority if it is explicitly set in the email.
This is what I meant.
The property is however useful when set to false as it allows tiurning off priorities when dealing with unknown third party systems when performing remote delivery mx resolution.
(Prioritiesmakes sens localy or with known 3rd party)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #2337:
URL: https://github.com/apache/james-project/pull/2337#issuecomment-2255358155
Also I would like to see
```
@Test
void remoteDeliveryShouldNotSetupDefaultValueWhenNone(DockerMockSmtp mockSmtp) throws Exception {
AuthenticatingSMTPClient smtpClient = new AuthenticatingSMTPClient("TLS", "UTF-8");
try {
smtpClient.connect("localhost", jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort().getValue());
smtpClient.auth(PLAIN, FROM, PASSWORD);
smtpClient.ehlo(DEFAULT_DOMAIN);
smtpClient.mail("<" + FROM + ">");
smtpClient.rcpt("<" + RECIPIENT + ">");
smtpClient.sendShortMessageData("A short message...");
} finally {
smtpClient.disconnect();
}
calmlyAwait.atMost(TEN_SECONDS).untilAsserted(() -> assertThat(mockSmtp.getConfigurationClient().listMails())
.hasSize(1)
.extracting(Mail::getEnvelope)
.containsExactly(Mail.Envelope.builder()
.from(new MailAddress(FROM))
.addRecipient(Mail.Recipient.builder()
.address(new MailAddress(RECIPIENT))
.build())
.build()));
}
```
As a test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "Maxxx873 (via GitHub)" <gi...@apache.org>.
Maxxx873 commented on code in PR #2337:
URL: https://github.com/apache/james-project/pull/2337#discussion_r1694267523
##########
docs/modules/servers/partials/RemoteDelivery.adoc:
##########
@@ -74,6 +74,9 @@ within the mailet code.<br>
Note: This facility should be used with extreme care by expert users with a thorough knowledge of the relevant RFCs and
the ability to perform their own problem resolutions.
* *debug* (optional) - a Boolean (true/false) indicating whether debugging is on. Default is false.
+* *usePriority* (optional) - a Boolean value (true/false) that indicates whether email priority support is enabled.
+When this option is enabled, any email that does not have a specified priority will be assigned the highest priority upon
+delivery and the lowest priority during retry attempts. Default is false.
Review Comment:
If this property `usePriority` is enabled but does not affect emails without priorities set, I don't quite understand what it is for. Based on my understanding, this property was initially designed to hardcode email priorities through configuration, with the highest priority assigned on send and the lowest on retry. To maintain the original intent while also supporting the new priority functionality, I propose using this property to assign priorities to emails that currently lack a specified priority.
I also have a couple of alternative suggestions:
1. Always assign the highest priority when sending and the lowest when retrying, irrespective of any priority specified in the email, if `usePriority` is enabled. .
2. Eliminate this property entirely and only consider the priority if it is explicitly set in the email.
What are your thoughts on these options?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #2337:
URL: https://github.com/apache/james-project/pull/2337#issuecomment-2267460423
Can you squash fixups please?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2337:
URL: https://github.com/apache/james-project/pull/2337#discussion_r1664240338
##########
server/apps/distributed-app/docs/modules/ROOT/pages/configure/smtp-hooks.adoc:
##########
@@ -318,6 +318,25 @@ The Distributed server has optional support for FUTURERELEASE (link:https://www.
</smtpserver>
....
Review Comment:
Please document the RFC-6710 in the implemented standard section.
CF https://github.com/apache/james-project/blob/master/docs/modules/servers/pages/distributed/architecture/implemented-standards.adoc#smtp
##########
server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SmtpMtPriorityMessageHookTest.java:
##########
@@ -0,0 +1,147 @@
+/****************************************************************
+ * 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.apache.james.smtpserver.SMTPServerTestSystem.BOB;
+import static org.apache.james.smtpserver.SMTPServerTestSystem.PASSWORD;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Base64;
+
+import org.apache.commons.net.smtp.SMTPClient;
+import org.apache.mailet.AttributeName;
+import org.apache.mailet.Mail;
+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 org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+class SmtpMtPriorityMessageHookTest {
Review Comment:
I do not see test that shows rejection of high value MT priority for not connected users.
##########
server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/priority/SmtpMtPriorityParameterHook.java:
##########
@@ -0,0 +1,77 @@
+/****************************************************************
+ * 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.priority;
+
+import static org.apache.james.protocols.api.ProtocolSession.State.Transaction;
+import static org.apache.james.smtpserver.priority.SmtpMtPriorityParameters.MT_PRIORITY_PARAMETER;
+
+import java.util.Optional;
+
+import org.apache.james.protocols.api.ProtocolSession;
+import org.apache.james.protocols.smtp.SMTPRetCode;
+import org.apache.james.protocols.smtp.SMTPSession;
+import org.apache.james.protocols.smtp.hook.HookResult;
+import org.apache.james.protocols.smtp.hook.HookReturnCode;
+import org.apache.james.protocols.smtp.hook.MailParametersHook;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SmtpMtPriorityParameterHook implements MailParametersHook {
+ private static final Logger LOGGER = LoggerFactory.getLogger(SmtpMtPriorityParameterHook.class);
+
+ public static final ProtocolSession.AttachmentKey<SmtpMtPriorityParameters.MtPriority> MT_PRIORITY_ATTACHMENT_KEY =
+ ProtocolSession.AttachmentKey.of(MT_PRIORITY_PARAMETER, SmtpMtPriorityParameters.MtPriority.class);
+
+ @Override
+ public HookResult doMailParameter(SMTPSession session, String paramName, String paramValue) {
+ if (session.getAttachment(MT_PRIORITY_ATTACHMENT_KEY, Transaction).isPresent()) {
+ LOGGER.debug("The Mail parameter cannot contain more than one MT-PRIORITY parameter at the same time");
+ return HookResult.builder()
+ .smtpReturnCode(SMTPRetCode.SYNTAX_ERROR_ARGUMENTS)
+ .hookReturnCode(HookReturnCode.deny())
+ .smtpDescription("The Mail parameter cannot contain more than one MT-PRIORITY parameter at the same time")
+ .build();
+ }
+ if (paramName.equals(MT_PRIORITY_PARAMETER)) {
+ try {
+ Optional<SmtpMtPriorityParameters.MtPriority> mtPriority = SmtpMtPriorityParameters.MtPriority.from(paramValue);
+ if (mtPriority.filter(p -> session.getUsername() == null && p.getPriorityValue() > 0).isPresent()) {
Review Comment:
Who is able to set which priory is actually quite crucial and saying "high priorities are only for connected users" seems somehow arbitrary. (We can be more severe than the spec and allow eg some high priority just to some individuals)
I would encapsulate the check into a dedicated method of smtp session - so that it could get easily overloaded if need be:
```
default boolean isAuthorized(MtPriority p) {
return session.getUsername() != null && p.getPriorityValue() <= 0;
}
```
And then:
```
mtPriority.ifPresent(p -> {
if (!session.isAuthorized(p)) {
// reject
}
session.setAttachment(MT_PRIORITY_ATTACHMENT_KEY, p, Transaction))
});
```
(possibly even extracting the resulting lambda as a method)
##########
server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SmtpMtPriorityMessageHookTest.java:
##########
@@ -0,0 +1,147 @@
+/****************************************************************
+ * 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.apache.james.smtpserver.SMTPServerTestSystem.BOB;
+import static org.apache.james.smtpserver.SMTPServerTestSystem.PASSWORD;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Base64;
+
+import org.apache.commons.net.smtp.SMTPClient;
+import org.apache.mailet.AttributeName;
+import org.apache.mailet.Mail;
+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 org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+class SmtpMtPriorityMessageHookTest {
+
+ private final SMTPServerTestSystem testSystem = new SMTPServerTestSystem();
+ private static final AttributeName MAIL_PRIORITY_ATTRIBUTE_NAME = AttributeName.of("MAIL_PRIORITY");
+
+ @BeforeEach
+ void setUp() throws Exception {
+ testSystem.setUp("smtpserver-mtPriority.xml");
+ }
+
+ @AfterEach
+ void tearDown() {
+ testSystem.smtpServer.destroy();
+ }
+
+ @Test
+ void ehloShouldAdvertiseMtPriorityExtension() throws Exception {
+ SMTPClient smtpProtocol = new SMTPClient();
+ InetSocketAddress bindedAddress = testSystem.getBindedAddress();
+ 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 MT-PRIORITY");
+ });
+ }
+
+ @ParameterizedTest
+ @CsvSource({"-9,0", "-8,0", "-7,0", "-6,0", "-5,0", "-4,0", "-3,0", "-2,0", "-1,0", "0,0", "1,1",
+ "2,2", "3,3", "4,4", "5,5", "6,6", "7,7", "8,8", "9,9"})
Review Comment:
Yes true actually the mail queue implementations do not support negative priorities as of today yet I believe this is more of a mistake and mis-understanding of legacy implementors rather than a concious choice.
It would be great if we could modify the JMS mailque to support this and alter PriorityMailQueueContract accordingly, and thus having here:
```suggestion
@CsvSource({"-9,-9", "-8,-8", "-7,-7", "-6,-6", "-5,-5", "-4,-4", "-3,-3", "-2,-2", "-1,-1", "0,0", "1,1",
"2,2", "3,3", "4,4", "5,5", "6,6", "7,7", "8,8", "9,9"})
```
While we do not violate the spec (that accounts for such rounding) we could at least plan this as an improvement.
##########
server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/priority/SmtpMtPriorityParameters.java:
##########
@@ -0,0 +1,100 @@
+/****************************************************************
+ * 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.priority;
+
+import static org.apache.james.queue.api.MailPrioritySupport.LOW_PRIORITY;
+
+import java.util.Objects;
+import java.util.Optional;
+
+import org.apache.james.queue.api.MailPrioritySupport;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.AttributeValue;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
+
+public class SmtpMtPriorityParameters {
+ public static final String MT_PRIORITY_PARAMETER = "MT-PRIORITY";
+ public static final int MIN_MT_PRIORITY_VALUE = -9;
+ public static final int MAX_MT_PRIORITY_VALUE = 9;
+
+ public static class MtPriority {
+
+ public static MtPriority of(String value) {
+ Preconditions.checkNotNull(value);
+ Integer priority = Integer.parseInt(value);
+ return new SmtpMtPriorityParameters.MtPriority(priority);
+ }
+
+ public static Optional<MtPriority> from(String value) {
+ if (Integer.parseInt(value) < MIN_MT_PRIORITY_VALUE || Integer.parseInt(value) > MAX_MT_PRIORITY_VALUE) {
+ throw new IllegalArgumentException("Invalid priority: According to RFC-6710 allowed values are from -9 to 9 inclusive");
+ }
+ return Optional.of(value)
+ .map(SmtpMtPriorityParameters.MtPriority::of);
+ }
Review Comment:
Why an optional? We actually always return a value....
Also we call `Integer.parseInt(value)` 3 times, twice in the check, once in `of`.
Suggestion:
- [ ] Move the check (-9 < p < 9) into the constructor
- [ ] And remove `from` method
- [ ] Adapt callers
##########
server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/priority/SmtpMtPriorityParameterHook.java:
##########
@@ -0,0 +1,77 @@
+/****************************************************************
+ * 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.priority;
+
+import static org.apache.james.protocols.api.ProtocolSession.State.Transaction;
+import static org.apache.james.smtpserver.priority.SmtpMtPriorityParameters.MT_PRIORITY_PARAMETER;
+
+import java.util.Optional;
+
+import org.apache.james.protocols.api.ProtocolSession;
+import org.apache.james.protocols.smtp.SMTPRetCode;
+import org.apache.james.protocols.smtp.SMTPSession;
+import org.apache.james.protocols.smtp.hook.HookResult;
+import org.apache.james.protocols.smtp.hook.HookReturnCode;
+import org.apache.james.protocols.smtp.hook.MailParametersHook;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SmtpMtPriorityParameterHook implements MailParametersHook {
+ private static final Logger LOGGER = LoggerFactory.getLogger(SmtpMtPriorityParameterHook.class);
+
+ public static final ProtocolSession.AttachmentKey<SmtpMtPriorityParameters.MtPriority> MT_PRIORITY_ATTACHMENT_KEY =
+ ProtocolSession.AttachmentKey.of(MT_PRIORITY_PARAMETER, SmtpMtPriorityParameters.MtPriority.class);
+
+ @Override
+ public HookResult doMailParameter(SMTPSession session, String paramName, String paramValue) {
+ if (session.getAttachment(MT_PRIORITY_ATTACHMENT_KEY, Transaction).isPresent()) {
+ LOGGER.debug("The Mail parameter cannot contain more than one MT-PRIORITY parameter at the same time");
+ return HookResult.builder()
+ .smtpReturnCode(SMTPRetCode.SYNTAX_ERROR_ARGUMENTS)
+ .hookReturnCode(HookReturnCode.deny())
+ .smtpDescription("The Mail parameter cannot contain more than one MT-PRIORITY parameter at the same time")
+ .build();
+ }
+ if (paramName.equals(MT_PRIORITY_PARAMETER)) {
+ try {
+ Optional<SmtpMtPriorityParameters.MtPriority> mtPriority = SmtpMtPriorityParameters.MtPriority.from(paramValue);
+ if (mtPriority.filter(p -> session.getUsername() == null && p.getPriorityValue() > 0).isPresent()) {
+ LOGGER.debug("Needs to be logged to use positive priorities");
+ session.setAttachment(MT_PRIORITY_ATTACHMENT_KEY, SmtpMtPriorityParameters.MtPriority.defaultValue(), Transaction);
+ return HookResult.DECLINED;
+ }
+ mtPriority.ifPresent(p -> session.setAttachment(MT_PRIORITY_ATTACHMENT_KEY, p, Transaction));
Review Comment:
We would need to have an INFO log about priority setting CF
```
3. The received MT-PRIORITY parameter value SHOULD be logged as part
of any logging of message transactions.
```
##########
server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/priority/SmtpMtPriorityParameters.java:
##########
@@ -0,0 +1,100 @@
+/****************************************************************
+ * 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.priority;
+
+import static org.apache.james.queue.api.MailPrioritySupport.LOW_PRIORITY;
+
+import java.util.Objects;
+import java.util.Optional;
+
+import org.apache.james.queue.api.MailPrioritySupport;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.AttributeValue;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
+
+public class SmtpMtPriorityParameters {
+ public static final String MT_PRIORITY_PARAMETER = "MT-PRIORITY";
+ public static final int MIN_MT_PRIORITY_VALUE = -9;
+ public static final int MAX_MT_PRIORITY_VALUE = 9;
+
+ public static class MtPriority {
+
+ public static MtPriority of(String value) {
+ Preconditions.checkNotNull(value);
+ Integer priority = Integer.parseInt(value);
+ return new SmtpMtPriorityParameters.MtPriority(priority);
+ }
+
+ public static Optional<MtPriority> from(String value) {
+ if (Integer.parseInt(value) < MIN_MT_PRIORITY_VALUE || Integer.parseInt(value) > MAX_MT_PRIORITY_VALUE) {
+ throw new IllegalArgumentException("Invalid priority: According to RFC-6710 allowed values are from -9 to 9 inclusive");
+ }
+ return Optional.of(value)
+ .map(SmtpMtPriorityParameters.MtPriority::of);
+ }
+
+ public static MtPriority defaultValue() {
+ return new SmtpMtPriorityParameters.MtPriority(0);
+ }
Review Comment:
This can be a constant:
```
public static final MtPriority DEFAULT = new MtPriority(0);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "Maxxx873 (via GitHub)" <gi...@apache.org>.
Maxxx873 commented on code in PR #2337:
URL: https://github.com/apache/james-project/pull/2337#discussion_r1670980936
##########
server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SmtpMtPriorityMessageHookTest.java:
##########
@@ -0,0 +1,147 @@
+/****************************************************************
+ * 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.apache.james.smtpserver.SMTPServerTestSystem.BOB;
+import static org.apache.james.smtpserver.SMTPServerTestSystem.PASSWORD;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Base64;
+
+import org.apache.commons.net.smtp.SMTPClient;
+import org.apache.mailet.AttributeName;
+import org.apache.mailet.Mail;
+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 org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+class SmtpMtPriorityMessageHookTest {
+
+ private final SMTPServerTestSystem testSystem = new SMTPServerTestSystem();
+ private static final AttributeName MAIL_PRIORITY_ATTRIBUTE_NAME = AttributeName.of("MAIL_PRIORITY");
+
+ @BeforeEach
+ void setUp() throws Exception {
+ testSystem.setUp("smtpserver-mtPriority.xml");
+ }
+
+ @AfterEach
+ void tearDown() {
+ testSystem.smtpServer.destroy();
+ }
+
+ @Test
+ void ehloShouldAdvertiseMtPriorityExtension() throws Exception {
+ SMTPClient smtpProtocol = new SMTPClient();
+ InetSocketAddress bindedAddress = testSystem.getBindedAddress();
+ 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 MT-PRIORITY");
+ });
+ }
+
+ @ParameterizedTest
+ @CsvSource({"-9,0", "-8,0", "-7,0", "-6,0", "-5,0", "-4,0", "-3,0", "-2,0", "-1,0", "0,0", "1,1",
+ "2,2", "3,3", "4,4", "5,5", "6,6", "7,7", "8,8", "9,9"})
Review Comment:
If all goes well, I plan to implement this improvement in a separate PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "Maxxx873 (via GitHub)" <gi...@apache.org>.
Maxxx873 commented on PR #2337:
URL: https://github.com/apache/james-project/pull/2337#issuecomment-2266134571
@chibenwa Thank you very much for taking the time to review the code and provide such detailed feedback. I appreciate your insights. And if anyone else has additional comments, I am happy to work on them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "Maxxx873 (via GitHub)" <gi...@apache.org>.
Maxxx873 commented on code in PR #2337:
URL: https://github.com/apache/james-project/pull/2337#discussion_r1670989657
##########
server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SmtpMtPriorityMessageHookTest.java:
##########
@@ -0,0 +1,147 @@
+/****************************************************************
+ * 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.apache.james.smtpserver.SMTPServerTestSystem.BOB;
+import static org.apache.james.smtpserver.SMTPServerTestSystem.PASSWORD;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Base64;
+
+import org.apache.commons.net.smtp.SMTPClient;
+import org.apache.mailet.AttributeName;
+import org.apache.mailet.Mail;
+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 org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+class SmtpMtPriorityMessageHookTest {
Review Comment:
in this `shouldSetDefaultPriorityWhenUnauthenticated(String priorityValue)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on code in PR #2337:
URL: https://github.com/apache/james-project/pull/2337#discussion_r1677166745
##########
protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/ReceivedDataLineFilter.java:
##########
@@ -165,7 +171,7 @@ protected Response onHeadersLine(SMTPSession session, byte[] line, LineHandler<S
*/
private Response addHeaders(SMTPSession session, byte[] line, LineHandler<SMTPSession> next) {
Response response;
- for (Header header: headers(session)) {
+ for (Header header : headers(session)) {
Review Comment:
Checkstyle
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "Maxxx873 (via GitHub)" <gi...@apache.org>.
Maxxx873 commented on PR #2337:
URL: https://github.com/apache/james-project/pull/2337#issuecomment-2218376274
I have worked through the main points. I plan to finalize "RemoteDelivery" as part of this PR a little later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "Maxxx873 (via GitHub)" <gi...@apache.org>.
Maxxx873 commented on code in PR #2337:
URL: https://github.com/apache/james-project/pull/2337#discussion_r1702872067
##########
server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SmtpMtPriorityMessageHookTest.java:
##########
@@ -0,0 +1,147 @@
+/****************************************************************
+ * 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.apache.james.smtpserver.SMTPServerTestSystem.BOB;
+import static org.apache.james.smtpserver.SMTPServerTestSystem.PASSWORD;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.util.Base64;
+
+import org.apache.commons.net.smtp.SMTPClient;
+import org.apache.mailet.AttributeName;
+import org.apache.mailet.Mail;
+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 org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+class SmtpMtPriorityMessageHookTest {
+
+ private final SMTPServerTestSystem testSystem = new SMTPServerTestSystem();
+ private static final AttributeName MAIL_PRIORITY_ATTRIBUTE_NAME = AttributeName.of("MAIL_PRIORITY");
+
+ @BeforeEach
+ void setUp() throws Exception {
+ testSystem.setUp("smtpserver-mtPriority.xml");
+ }
+
+ @AfterEach
+ void tearDown() {
+ testSystem.smtpServer.destroy();
+ }
+
+ @Test
+ void ehloShouldAdvertiseMtPriorityExtension() throws Exception {
+ SMTPClient smtpProtocol = new SMTPClient();
+ InetSocketAddress bindedAddress = testSystem.getBindedAddress();
+ 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 MT-PRIORITY");
+ });
+ }
+
+ @ParameterizedTest
+ @CsvSource({"-9,0", "-8,0", "-7,0", "-6,0", "-5,0", "-4,0", "-3,0", "-2,0", "-1,0", "0,0", "1,1",
+ "2,2", "3,3", "4,4", "5,5", "6,6", "7,7", "8,8", "9,9"})
Review Comment:
> Yes true actually the mail queue implementations do not support negative priorities as of today yet I believe this is more of a mistake and mis-understanding of legacy implementors rather than a concious choice.
>
> It would be great if we could modify the JMS mailque to support this and alter PriorityMailQueueContract accordingly, and thus having here:
>
> While we do not violate the spec (that accounts for such rounding) we could at least plan this as an improvement.
I am currently looking for an optimal solution, considering that queues implement the The Jakarta Messaging API.
The Jakarta Messaging API defines ten levels of priority value, with 0 as the lowest priority and 9 as the highest. Any ideas on how to use negative priorities? What might a modification or an alternative of the JMS look like?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael merged PR #2337:
URL: https://github.com/apache/james-project/pull/2337
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
Re: [PR] JAMES-3824 SMTP Extension for Message Transfer Priorities [james-project]
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa commented on PR #2337:
URL: https://github.com/apache/james-project/pull/2337#issuecomment-2207102399
Sure. If you need pointers, especially on how to test "RemoteDelivery" then do not hesitate to ask.
--
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