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/11/29 10:38:32 UTC

[GitHub] [james-project] quantranhong1999 opened a new pull request, #1338: JAMES-3754 Storage layer support for IMAP save date extension

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

   RFC: https://www.rfc-editor.org/rfc/rfc8514.html
   TODO:
   - [ ]  Add saveDate in MailboxMessage
   - [ ] AppendCommand should propagate saveDate
   - [ ] Add saveDate in MessageResult too
   - [ ] Copy/move message at mapper layer should set a new value for saveDate
   - [ ] Upgrade instruction for Cassandra tables
   
   Maybe another PR:
   - [ ] Add saveDate field to search index and implement search criteria for IMAP SAVEDBEFORE, SAVEDON, SAVEDSINCE, SAVEDATESUPPORTED
   - [ ] Upgrade instruction for OpenSearch index
   
   Another PR:
   - [ ] Implement IMAP protocol layer (Capability, SEARCH, FETCH)
   - [ ] Modify rspamd report 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] chibenwa commented on a diff in pull request #1338: JAMES-3754 Storage layer support for IMAP save date extension

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


##########
mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageIdMapperTest.java:
##########
@@ -984,6 +990,52 @@ void deletesShouldNotFailUponMissingMessage() {
                 .build());
     }
 
+    @Nested
+    class SaveDateTests {
+        @RepeatedTest(5)

Review Comment:
   @Test only ?



-- 
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 #1338: JAMES-3754 Storage layer support for IMAP save date extension

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

   [org.apache.james.mailbox.cassandra.mail.CassandraMessageMapperRelaxedConsistencyTest$WeakReadConsistency.retrievingMessagesWithALimitShouldLimitTheNumberOfMessages](https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1338/14/testReport/junit/org.apache.james.mailbox.cassandra.mail/CassandraMessageMapperRelaxedConsistencyTest$WeakReadConsistency/retrievingMessagesWithALimitShouldLimitTheNumberOfMessages/)
   
   Related?


-- 
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 #1338: JAMES-3754 Storage layer support for IMAP save date extension

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


##########
upgrade-instructions.md:
##########
@@ -23,6 +23,23 @@ Change list:
 - [SortOrder addition in Identity](#sortorder-addition-in-identity)
 - [TLS host name verification is now enabled by default](#tls-host-name-verification-is-now-enabled-by-default)
 - [Blob Store AES upgraded to PBKDF2WithHmacSHA512](#blob-store-aes-upgraded-to-pbkdf2withhmacsha512)
+- [Adding saveDate column to messageIdTable and imapUidTable](#adding-savedate-column-to-messageidtable-and-imapuidtable)
+
+### Adding saveDate column to messageIdTable and imapUidTable
+
+Date 01/12/2022
+
+JIRA: https://issues.apache.org/jira/browse/JAMES-3754
+
+Concerned product: Distributed James, Cassandra James Server
+
+Add `saveDate` column to `messageIdTable` and `imapUidTable` tables in order to store saveDate data as part of IMAP SAVEDATE extension.
+
+In order to add this `messageIdTable` column you need to run the following CQL commands:
+```
+ALTER TABLE james_keyspace.messageIdTable ADD saveDate timestamp;
+ALTER TABLE james_keyspace.imapUidTable ADD saveDate timestamp;

Review Comment:
   capital case are a pain with Cassandra. `save_date` is better maybe?



-- 
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 #1338: JAMES-3754 Storage layer support for IMAP save date extension

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


##########
third-party/rspamd/src/test/java/org/apache/james/rspamd/route/FeedMessageRouteTest.java:
##########
@@ -158,8 +160,8 @@ private void appendMessage(MailboxPath mailboxPath, Date internalDate) throws Ma
     class FeedSpam {
         @Test
         void taskShouldReportAllSpamMessagesOfAllUsersByDefault() throws MailboxException {
-            appendMessage(BOB_SPAM_MAILBOX, Date.from(NOW));
-            appendMessage(ALICE_SPAM_MAILBOX, Date.from(NOW));
+            appendMessage(BOB_SPAM_MAILBOX, Date.from(NOW), EMPTY_SAVE_DATE);

Review Comment:
   Make the Mailbox use a custom clock rather than passing the SaveDate as an argument!



-- 
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 merged pull request #1338: JAMES-3754 Storage layer support for IMAP save date extension

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


-- 
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 #1338: JAMES-3754 Storage layer support for IMAP save date extension

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

   I'm happy to have done an early review, this was likely some important comments that will end up help you a lot!


-- 
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 #1338: JAMES-3754 Storage layer support for IMAP save date extension

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


##########
mailbox/api/src/main/java/org/apache/james/mailbox/MessageManager.java:
##########
@@ -282,6 +284,16 @@ public Builder withInternalDate(Optional<Date> date) {
                 return this;
             }
 
+            public Builder withSaveDate(Date saveDate) {
+                this.saveDate = Optional.of(saveDate);
+                return this;
+            }
+
+            public Builder withSaveDate(Optional<Date> saveDate) {
+                this.saveDate = saveDate;
+                return this;
+            }

Review Comment:
   Idem, this should be a detail internal to the manager layer.



-- 
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 #1338: JAMES-3754 Storage layer support for IMAP save date extension

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

   
   
    
   --
     | Test Result (2 failures  / ±0)org.apache.james.mailbox.store.mail.model.MessageMapperTest$SaveDateTests.deleteMessageShouldReturnMetaDataContainsSaveDateorg.apache.james.mailbox.cassandra.mail.CassandraMessageMapperTest$ReadRepairsTesting.readingShouldEventuallyFixMissingCountersInconsistencies{CassandraCluster}
   
   [Test Result](https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1338/10/testReport/) (2 failures / ±0)
   
       [org.apache.james.mailbox.store.mail.model.MessageMapperTest$SaveDateTests.deleteMessageShouldReturnMetaDataContainsSaveDate](https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1338/10/testReport/junit/org.apache.james.mailbox.store.mail.model/MessageMapperTest$SaveDateTests/deleteMessageShouldReturnMetaDataContainsSaveDate/)
       [org.apache.james.mailbox.cassandra.mail.CassandraMessageMapperTest$ReadRepairsTesting.readingShouldEventuallyFixMissingCountersInconsistencies{CassandraCluster}](https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1338/10/testReport/junit/org.apache.james.mailbox.cassandra.mail/CassandraMessageMapperTest$ReadRepairsTesting/readingShouldEventuallyFixMissingCountersInconsistencies_CassandraCluster_/)


-- 
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 #1338: JAMES-3754 Storage layer support for IMAP save date extension

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

   Related?


-- 
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 #1338: JAMES-3754 Storage layer support for IMAP save date extension

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


##########
mailbox/api/src/main/java/org/apache/james/mailbox/MessageManager.java:
##########
@@ -232,7 +232,7 @@ public final int hashCode() {
      * @throws MailboxException
      *             when message cannot be appended
      */
-    AppendResult appendMessage(InputStream msgIn, Date internalDate, MailboxSession mailboxSession, boolean isRecent, Flags flags) throws MailboxException;
+    AppendResult appendMessage(InputStream msgIn, Date internalDate, Date saveDate, MailboxSession mailboxSession, boolean isRecent, Flags flags) throws MailboxException;

Review Comment:
   IMO the `MessageManager` and `MessageIdManager` are responsible of setting the saveDate.
   
   We should not leak it on this API



-- 
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 #1338: JAMES-3754 Storage layer support for IMAP save date extension

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


##########
mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/modules/CassandraMessageModule.java:
##########
@@ -114,6 +116,7 @@ public interface CassandraMessageModule {
         .statement(statement -> types -> statement
             .withPartitionKey(CassandraMessageIds.MESSAGE_ID, TIMEUUID)
             .withColumn(CassandraMessageV3Table.INTERNAL_DATE, TIMESTAMP)
+            .withColumn(CassandraMessageV3Table.SAVE_DATE, TIMESTAMP)

Review Comment:
   Save date is mailbox specific. There is no reasons to store it in a cassandra table that is not mailboxe scopped (we should not store it in messagev3)



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

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

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


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


[GitHub] [james-project] quantranhong1999 commented on pull request #1338: JAMES-3754 Storage layer support for IMAP save date extension

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

   Rebased to solve conflicts with 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] Arsnael commented on pull request #1338: JAMES-3754 Storage layer support for IMAP save date extension

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

   Rebase and fix conflict 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