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 2021/07/05 10:47:36 UTC
[GitHub] [james-project] quantranhong1999 opened a new pull request #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
quantranhong1999 opened a new pull request #529:
URL: https://github.com/apache/james-project/pull/529
Purpose: Experiment guessing threadId by doing search. Good to experiment but bad for production.
I changed a piece of code in buildSearchQuery. We did compare one to one corresponding field. We should compare many fields.
```
An identical message id [@!RFC5322] appears in both messages in any of the Message-Id, In-Reply-To, and References header fields.
```
--
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 change in pull request #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #529:
URL: https://github.com/apache/james-project/pull/529#discussion_r664342410
##########
File path: mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/SearchThreadIdGuessingAlgorithmTest.java
##########
@@ -0,0 +1,51 @@
+/******************************************************************
+ * 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.mailbox.store.search;
+
+import org.apache.james.mailbox.inmemory.InMemoryCombinationManagerTestSystem;
+import org.apache.james.mailbox.inmemory.InMemoryMessageId;
+import org.apache.james.mailbox.inmemory.manager.InMemoryIntegrationResources;
+import org.apache.james.mailbox.model.MessageId;
+import org.apache.james.mailbox.store.AbstractThreadIdGuessingAlgorithmTest;
+import org.apache.james.mailbox.store.CombinationManagerTestSystem;
+import org.apache.james.mailbox.store.mail.SearchThreadIdGuessingAlgorithm;
+import org.apache.james.mailbox.store.mail.ThreadIdGuessingAlgorithm;
+
+public class SearchThreadIdGuessingAlgorithmTest extends AbstractThreadIdGuessingAlgorithmTest {
Review comment:
`AbstractThreadIdGuessingAlgorithmTest` => `ThreadIdGuessingAlgorithmContract`
##########
File path: mailbox/store/src/test/java/org/apache/james/mailbox/store/AbstractThreadIdGuessingAlgorithmTest.java
##########
@@ -0,0 +1,186 @@
+/******************************************************************
+ * 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.mailbox.store;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.MessageId;
+import org.apache.james.mailbox.model.ThreadId;
+import org.apache.james.mailbox.store.mail.ThreadIdGuessingAlgorithm;
+import org.apache.james.mailbox.store.mail.model.MimeMessageId;
+import org.apache.james.mailbox.store.mail.model.Subject;
+import org.apache.james.mime4j.dom.Message;
+import org.apache.james.mime4j.stream.RawField;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public abstract class AbstractThreadIdGuessingAlgorithmTest {
+ public static final Username USER = Username.of("quan");
+ private MailboxManager mailboxManager;
+ private MessageManager inbox;
+ private ThreadIdGuessingAlgorithm testee;
+ private MailboxSession mailboxSession;
+ private CombinationManagerTestSystem testingData;
+ private MessageId newBasedMessageId;
+
+
+ protected abstract CombinationManagerTestSystem createTestingData();
+
+ protected abstract ThreadIdGuessingAlgorithm initThreadIdGuessingAlgorithm(CombinationManagerTestSystem testingData);
+
+ protected abstract MessageId initNewBasedMessageId();
+
+ @BeforeEach
+ void setUp() throws Exception {
+ testingData = createTestingData();
+ testee = initThreadIdGuessingAlgorithm(testingData);
+ newBasedMessageId = initNewBasedMessageId();
+
+ mailboxManager = testingData.getMailboxManager();
+ mailboxSession = mailboxManager.createSystemSession(USER);
+ mailboxManager.createMailbox(MailboxPath.inbox(USER), mailboxSession);
+ inbox = mailboxManager.getMailbox(MailboxPath.inbox(USER), mailboxSession);
+ }
+
+ @Test
+ void givenNonMailWhenAddAMailThenGuessingThreadIdShouldBasedOnGeneratedMessageId() throws Exception {
+ ThreadId threadId = testee.guessThreadIdReactive(newBasedMessageId, Optional.of(new MimeMessageId("abc")), Optional.empty(), Optional.empty(), Optional.of(new Subject("test")), mailboxSession).block();
+
+ assertThat(threadId.getBaseMessageId()).isEqualTo(newBasedMessageId);
+ }
+
+ @Test
+ void givenOldMailWhenAddNewRelatedMailsThenGuessingThreadIdShouldReturnSameThreadIdWithOldMail() throws Exception {
+ // given old mail
+ MessageManager.AppendResult message = inbox.appendMessage(MessageManager.AppendCommand.from(Message.Builder.of()
+ .setSubject("Test")
+ .setMessageId("Message-ID")
+ .setField(new RawField("In-Reply-To", "someInReplyTo"))
+ .addField(new RawField("References", "references1"))
+ .addField(new RawField("References", "references2"))
+ .setBody("testmail", StandardCharsets.UTF_8)), mailboxSession);
+
+ // add mails related to old message by subject and Message-ID (but this should not happen in real world cause every mail should have an unique MimeMessageId)
+ ThreadId threadId1 = testee.guessThreadIdReactive(newBasedMessageId, Optional.of(new MimeMessageId("Message-ID")), Optional.empty(), Optional.empty(), Optional.of(new Subject("Re: Test")), mailboxSession).block();
+ ThreadId threadId2 = testee.guessThreadIdReactive(newBasedMessageId, Optional.of(new MimeMessageId("someInReplyTo")), Optional.empty(), Optional.empty(), Optional.of(new Subject("Re: Test")), mailboxSession).block();
+ ThreadId threadId3 = testee.guessThreadIdReactive(newBasedMessageId, Optional.of(new MimeMessageId("references1")), Optional.empty(), Optional.empty(), Optional.of(new Subject("Re: Test")), mailboxSession).block();
+
+ // add mails related to old message by subject and In-Reply-To
+ ThreadId threadId4 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.of(new MimeMessageId("Message-ID")), Optional.empty(), Optional.of(new Subject("Re: Test")), mailboxSession).block();
+ ThreadId threadId5 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.of(new MimeMessageId("someInReplyTo")), Optional.empty(), Optional.of(new Subject("Re: Test")), mailboxSession).block();
+ ThreadId threadId6 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.of(new MimeMessageId("references2")), Optional.empty(), Optional.of(new Subject("Fwd: Re: Test")), mailboxSession).block();
+
+ // add mails related to old message by subject and References
+ ThreadId threadId7 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.empty(), Optional.of(List.of(new MimeMessageId("Message-ID"))), Optional.of(new Subject("Fwd: Re: Test")), mailboxSession).block();
+ ThreadId threadId8 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.empty(), Optional.of(List.of(new MimeMessageId("someInReplyTo"))), Optional.of(new Subject("Fwd: Re: Test")), mailboxSession).block();
+ ThreadId threadId9 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.empty(), Optional.of(List.of(new MimeMessageId("references1"), new MimeMessageId("NonRelated-references2"))), Optional.of(new Subject("Fwd: Re: Test")), mailboxSession).block();
+ ThreadId threadId10 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.empty(), Optional.of(List.of(new MimeMessageId("NonRelated-references1"), new MimeMessageId("references2"))), Optional.of(new Subject("Fwd: Re: Test")), mailboxSession).block();
+ ThreadId threadId11 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.empty(), Optional.of(List.of(new MimeMessageId("references1"), new MimeMessageId("references2"))), Optional.of(new Subject("Fwd: Re: Test")), mailboxSession).block();
+
+ // guess ThreadId should return the same ThreadId with old message
+ assertThat(threadId1).isEqualTo(message.getThreadId());
+ assertThat(threadId2).isEqualTo(message.getThreadId());
+ assertThat(threadId3).isEqualTo(message.getThreadId());
+ assertThat(threadId4).isEqualTo(message.getThreadId());
+ assertThat(threadId5).isEqualTo(message.getThreadId());
+ assertThat(threadId6).isEqualTo(message.getThreadId());
+ assertThat(threadId7).isEqualTo(message.getThreadId());
+ assertThat(threadId8).isEqualTo(message.getThreadId());
+ assertThat(threadId9).isEqualTo(message.getThreadId());
+ assertThat(threadId10).isEqualTo(message.getThreadId());
+ assertThat(threadId11).isEqualTo(message.getThreadId());
Review comment:
Maybe you could have a look to parametrized tests to be making this more readable?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on a change in pull request #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on a change in pull request #529:
URL: https://github.com/apache/james-project/pull/529#discussion_r664022277
##########
File path: mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/SearchThreadIdGuessingAlgorithmTest.java
##########
@@ -0,0 +1,153 @@
+/******************************************************************
+ * 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.mailbox.store.search;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageIdManager;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.inmemory.InMemoryMailboxManager;
+import org.apache.james.mailbox.inmemory.InMemoryMessageId;
+import org.apache.james.mailbox.inmemory.manager.InMemoryIntegrationResources;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.ThreadId;
+import org.apache.james.mailbox.store.mail.SearchThreadIdGuessingAlgorithm;
+import org.apache.james.mailbox.store.mail.model.MimeMessageId;
+import org.apache.james.mailbox.store.mail.model.Subject;
+import org.apache.james.mime4j.dom.Message;
+import org.apache.james.mime4j.stream.RawField;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class SearchThreadIdGuessingAlgorithmTest {
Review comment:
So is this what you mean:
- bind SearchThreadIdGuessingAlgorithm to MemoryMailboxModule
- move this test to mailbox/store and extract Cassandra and InMemory version of this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #529:
URL: https://github.com/apache/james-project/pull/529#discussion_r665019724
##########
File path: mailbox/store/src/test/java/org/apache/james/mailbox/store/ThreadIdGuessingAlgorithmContract.java
##########
@@ -39,19 +40,70 @@
import org.apache.james.mime4j.stream.RawField;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
public abstract class ThreadIdGuessingAlgorithmContract {
public static final Username USER = Username.of("quan");
+
+ private static Stream<Arguments> givenOldMailWhenAddNewRelatedMailsThenGuessingThreadIdShouldReturnSameThreadIdWithOldMail() {
Review comment:
Put the parameter methods just above the tests they are related to IMO
--
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 #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #529:
URL: https://github.com/apache/james-project/pull/529#issuecomment-875252197
> Should we bind SearchThreadIdGuessingAlgorithm.class to MemoryMailboxModule?
IMO yes :-p
--
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 a change in pull request #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on a change in pull request #529:
URL: https://github.com/apache/james-project/pull/529#discussion_r667650550
##########
File path: mailbox/store/src/test/java/org/apache/james/mailbox/store/ThreadIdGuessingAlgorithmContract.java
##########
@@ -0,0 +1,205 @@
+/******************************************************************
+ * 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.mailbox.store;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Stream;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.MessageId;
+import org.apache.james.mailbox.model.ThreadId;
+import org.apache.james.mailbox.store.mail.ThreadIdGuessingAlgorithm;
+import org.apache.james.mailbox.store.mail.model.MimeMessageId;
+import org.apache.james.mailbox.store.mail.model.Subject;
+import org.apache.james.mime4j.dom.Message;
+import org.apache.james.mime4j.stream.RawField;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+public abstract class ThreadIdGuessingAlgorithmContract {
+ public static final Username USER = Username.of("quan");
+ private MailboxManager mailboxManager;
+ private MessageManager inbox;
+ private ThreadIdGuessingAlgorithm testee;
+ private MailboxSession mailboxSession;
+ private CombinationManagerTestSystem testingData;
+ private MessageId newBasedMessageId;
+
+ protected abstract CombinationManagerTestSystem createTestingData();
+
+ protected abstract ThreadIdGuessingAlgorithm initThreadIdGuessingAlgorithm(CombinationManagerTestSystem testingData);
+
+ protected abstract MessageId initNewBasedMessageId();
+
+ @BeforeEach
+ void setUp() throws Exception {
+ testingData = createTestingData();
+ testee = initThreadIdGuessingAlgorithm(testingData);
+ newBasedMessageId = initNewBasedMessageId();
+
+ mailboxManager = testingData.getMailboxManager();
+ mailboxSession = mailboxManager.createSystemSession(USER);
+ mailboxManager.createMailbox(MailboxPath.inbox(USER), mailboxSession);
+ inbox = mailboxManager.getMailbox(MailboxPath.inbox(USER), mailboxSession);
+ }
+
+ @Test
+ void givenNonMailWhenAddAMailThenGuessingThreadIdShouldBasedOnGeneratedMessageId() throws Exception {
+ ThreadId threadId = testee.guessThreadIdReactive(newBasedMessageId, Optional.of(new MimeMessageId("abc")), Optional.empty(), Optional.empty(), Optional.of(new Subject("test")), mailboxSession).block();
+
+ assertThat(threadId.getBaseMessageId()).isEqualTo(newBasedMessageId);
+ }
+
+ private static Stream<Arguments> givenOldMailWhenAddNewRelatedMailsThenGuessingThreadIdShouldReturnSameThreadIdWithOldMail() {
Review comment:
No direct call, but yes it's necessary. It can be understood as implicit test data thanks to @ParameterizedTest @MethodSource annotations.
https://www.baeldung.com/parameterized-tests-junit-5 part 4.6
--
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 edited a comment on pull request #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
quantranhong1999 edited a comment on pull request #529:
URL: https://github.com/apache/james-project/pull/529#issuecomment-876354051
Squash fixups + cherry pick https://github.com/apache/james-project/pull/533/commits/ff5308b57a796defe9952434ff3d75618692adf9 (for now) + fix email/import fail test commit.
There are JPA failing tests, I will continue to fix them tomorrow.
--
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 #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on pull request #529:
URL: https://github.com/apache/james-project/pull/529#issuecomment-876292463
By the way when debug **setMessagesShouldReturnCreatedMessageWithEmptySubjectWhenSubjectIsEmpty** test, I see some thing weird. We have a empty input subject:
After parsing using headers.getField("Subject"), we got a null value UnstructureField
After unstructuredField.getValue(), it converts null value -> a String with a white space.
--
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 a change in pull request #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on a change in pull request #529:
URL: https://github.com/apache/james-project/pull/529#discussion_r664022277
##########
File path: mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/SearchThreadIdGuessingAlgorithmTest.java
##########
@@ -0,0 +1,153 @@
+/******************************************************************
+ * 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.mailbox.store.search;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageIdManager;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.inmemory.InMemoryMailboxManager;
+import org.apache.james.mailbox.inmemory.InMemoryMessageId;
+import org.apache.james.mailbox.inmemory.manager.InMemoryIntegrationResources;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.ThreadId;
+import org.apache.james.mailbox.store.mail.SearchThreadIdGuessingAlgorithm;
+import org.apache.james.mailbox.store.mail.model.MimeMessageId;
+import org.apache.james.mailbox.store.mail.model.Subject;
+import org.apache.james.mime4j.dom.Message;
+import org.apache.james.mime4j.stream.RawField;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class SearchThreadIdGuessingAlgorithmTest {
Review comment:
So is this what you mean:
- bind SearchThreadIdGuessingAlgorithm to MemoryMailboxModule
- move this test to mailbox/store and extract Cassandra and InMemory version of this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #529:
URL: https://github.com/apache/james-project/pull/529#issuecomment-876244062
https://issues.apache.org/jira/browse/JAMES-3611
--
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 #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #529:
URL: https://github.com/apache/james-project/pull/529#issuecomment-876230676
```
11:16:22.147 [ERROR] o.a.j.j.h.JMAPApiRoutes - Unexpected error
java.lang.StringIndexOutOfBoundsException: String index out of range: -1
at java.base/java.lang.StringLatin1.charAt(StringLatin1.java:47)
at java.base/java.lang.String.charAt(String.java:693)
at org.apache.james.mailbox.store.search.SearchUtil.removeSubTrailers(SearchUtil.java:413)
at org.apache.james.mailbox.store.search.SearchUtil.getBaseSubject(SearchUtil.java:246)
at org.apache.james.mailbox.store.mail.SearchThreadIdGuessingAlgorithm.lambda$buildSearchQuery$2(SearchThreadIdGuessingAlgorithm.java:81)
at java.base/java.util.Optional.map(Optional.java:265)
at org.apache.james.mailbox.store.mail.SearchThreadIdGuessingAlgorithm.buildSearchQuery(SearchThreadIdGuessingAlgorithm.java:81)
at org.apache.james.mailbox.store.mail.SearchThreadIdGuessingAlgorithm.guessThreadIdReactive(SearchThreadIdGuessingAlgorithm.java:60)
at org.apache.james.mailbox.store.MessageStorer$WithAttachment.appendMessageToStore(MessageStorer.java:106)
at org.apache.james.mailbox.store.StoreMessageManager.createAndDispatchMessage(StoreMessageManager.java:519)
at org.apache.james.mailbox.store.StoreMessageManager.lambda$appendMessage$2(StoreMessageManager.java:409)
at reactor.core.publisher.MonoCallable.call(MonoCallable.java:91)
at reactor.core.publisher.FluxFlatMap.trySubscribeScalarMap(FluxFlatMap.java:126)
at reactor.core.publisher.MonoFlatMap.subscribeOrReturn(MonoFlatMap.java:53)
at reactor.core.publisher.Mono.subscribe(Mono.java:4031)
at reactor.core.publisher.MonoSubscribeOn$SubscribeOnSubscriber.run(MonoSubscribeOn.java:126)
at reactor.core.scheduler.WorkerTask.call(WorkerTask.java:84)
at reactor.core.scheduler.WorkerTask.call(WorkerTask.java:37)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:834)
11:16:22.147 [ERROR] o.a.j.j.h.JMAPApiRoutes - Internal server error
```
WE GOT A BUGGG!!!!
(Not with your code though...)
org.apache.james.jmap.memory.MemorySetMessagesMethodTest.setMessagesShouldReturnCreatedMessageWithEmptySubjectWhenSubjectIsEmpty | 0.52 sec | 1
-- | -- | --
org.apache.james.jmap.memory.MemorySetMessagesMethodTest.setMessagesShouldReturnCreatedMessageWithEmptySubjectWhenSubjectIsNull | 0.46 sec | 1
org.apache.james.jmap.rfc8621.memory.MemoryEmailImportTest.importShouldAddTheMailsInTheMailbox{GuiceJamesServer} | 0.57 sec | 1
org.apache.james.mailbox.MailboxManagerTest$MessageTests.listMessagesMetadataShouldReturnAppendedMessage | 56 ms | 5
org.apache.james.mailbox.MailboxManagerTest$MessageTests.shouldBeAbleToAccessThreadIdOfAMessageAndThatThreadIdShouldWrapsMessageId | 59 ms | 5
org.apache.james.mailbox.MailboxManagerTest$EventTests.moveShouldFireExpungedEventInOriginMailbox | 30 ms | 5
org.apache.james.mailbox.MailboxManagerTest$EventTests.copyShouldFireAddedEventInTargetMailbox | 34 ms | 5
org.apache.james.mailbox.MailboxManagerTest$EventTests.moveShouldFireAddedEventInTargetMailbox | 30 ms | 5
org.apache.james.mailbox.jpa.mail.task.JPARecomputeCurrentQuotasServiceTest.recomputeCurrentQuotasShouldRecomputeCurrentQuotasCorrectlyWhenUserWithMessage | 1 sec | 5
org.apache.james.mailbox.jpa.mail.task.JPARecomputeCurrentQuotasServiceTest.recomputeCurrentQuotasShouldResetCurrentQuotasWhenIncorrectQuotas
--
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 change in pull request #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #529:
URL: https://github.com/apache/james-project/pull/529#discussion_r663984411
##########
File path: mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/NaiveThreadIdGuessingAlgorithm.java
##########
@@ -32,7 +32,12 @@
public class NaiveThreadIdGuessingAlgorithm implements ThreadIdGuessingAlgorithm {
@Override
- public ThreadId guessThreadId(Username username, MessageId messageId, Optional<MimeMessageId> thisMimeMessageId, Optional<MimeMessageId> inReplyTo, Optional<List<MimeMessageId>> references, Optional<Subject> subject) {
+ public ThreadId guessThreadId(MessageId messageId, Optional<MimeMessageId> thisMimeMessageId, Optional<MimeMessageId> inReplyTo, Optional<List<MimeMessageId>> references, Optional<Subject> subject, MailboxSession session) {
return ThreadId.fromBaseMessageId(messageId);
}
+
+ @Override
+ public Mono<ThreadId> guessThreadIdReactive(MessageId messageId, Optional<MimeMessageId> thisMimeMessageId, Optional<MimeMessageId> inReplyTo, Optional<List<MimeMessageId>> references, Optional<Subject> subject, MailboxSession session) {
Review comment:
This is a new interface, let's just have the reactive version!
##########
File path: mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/SearchThreadIdGuessingAlgorithmTest.java
##########
@@ -0,0 +1,153 @@
+/******************************************************************
+ * 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.mailbox.store.search;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageIdManager;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.inmemory.InMemoryMailboxManager;
+import org.apache.james.mailbox.inmemory.InMemoryMessageId;
+import org.apache.james.mailbox.inmemory.manager.InMemoryIntegrationResources;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.ThreadId;
+import org.apache.james.mailbox.store.mail.SearchThreadIdGuessingAlgorithm;
+import org.apache.james.mailbox.store.mail.model.MimeMessageId;
+import org.apache.james.mailbox.store.mail.model.Subject;
+import org.apache.james.mime4j.dom.Message;
+import org.apache.james.mime4j.stream.RawField;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class SearchThreadIdGuessingAlgorithmTest {
Review comment:
Can we extract a junit 5 contract off this?
Locate the junit 5 contract in mailbox/store.
##########
File path: mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/SearchThreadIdGuessingAlgorithmTest.java
##########
@@ -0,0 +1,153 @@
+/******************************************************************
+ * 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.mailbox.store.search;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageIdManager;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.inmemory.InMemoryMailboxManager;
+import org.apache.james.mailbox.inmemory.InMemoryMessageId;
+import org.apache.james.mailbox.inmemory.manager.InMemoryIntegrationResources;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.ThreadId;
+import org.apache.james.mailbox.store.mail.SearchThreadIdGuessingAlgorithm;
+import org.apache.james.mailbox.store.mail.model.MimeMessageId;
+import org.apache.james.mailbox.store.mail.model.Subject;
+import org.apache.james.mime4j.dom.Message;
+import org.apache.james.mime4j.stream.RawField;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class SearchThreadIdGuessingAlgorithmTest {
Review comment:
No...
I mean have this test suite as an interface, and default methods (we call that a contract). The test then implements the contract and tells which implementation to use. The contract is in mailbox/store but the test can be in mailbox/scanning-search without issues.
##########
File path: mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/SearchThreadIdGuessingAlgorithmTest.java
##########
@@ -0,0 +1,51 @@
+/******************************************************************
+ * 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.mailbox.store.search;
+
+import org.apache.james.mailbox.inmemory.InMemoryCombinationManagerTestSystem;
+import org.apache.james.mailbox.inmemory.InMemoryMessageId;
+import org.apache.james.mailbox.inmemory.manager.InMemoryIntegrationResources;
+import org.apache.james.mailbox.model.MessageId;
+import org.apache.james.mailbox.store.AbstractThreadIdGuessingAlgorithmTest;
+import org.apache.james.mailbox.store.CombinationManagerTestSystem;
+import org.apache.james.mailbox.store.mail.SearchThreadIdGuessingAlgorithm;
+import org.apache.james.mailbox.store.mail.ThreadIdGuessingAlgorithm;
+
+public class SearchThreadIdGuessingAlgorithmTest extends AbstractThreadIdGuessingAlgorithmTest {
Review comment:
`AbstractThreadIdGuessingAlgorithmTest` => `ThreadIdGuessingAlgorithmContract`
##########
File path: mailbox/store/src/test/java/org/apache/james/mailbox/store/AbstractThreadIdGuessingAlgorithmTest.java
##########
@@ -0,0 +1,186 @@
+/******************************************************************
+ * 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.mailbox.store;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.MessageId;
+import org.apache.james.mailbox.model.ThreadId;
+import org.apache.james.mailbox.store.mail.ThreadIdGuessingAlgorithm;
+import org.apache.james.mailbox.store.mail.model.MimeMessageId;
+import org.apache.james.mailbox.store.mail.model.Subject;
+import org.apache.james.mime4j.dom.Message;
+import org.apache.james.mime4j.stream.RawField;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public abstract class AbstractThreadIdGuessingAlgorithmTest {
+ public static final Username USER = Username.of("quan");
+ private MailboxManager mailboxManager;
+ private MessageManager inbox;
+ private ThreadIdGuessingAlgorithm testee;
+ private MailboxSession mailboxSession;
+ private CombinationManagerTestSystem testingData;
+ private MessageId newBasedMessageId;
+
+
+ protected abstract CombinationManagerTestSystem createTestingData();
+
+ protected abstract ThreadIdGuessingAlgorithm initThreadIdGuessingAlgorithm(CombinationManagerTestSystem testingData);
+
+ protected abstract MessageId initNewBasedMessageId();
+
+ @BeforeEach
+ void setUp() throws Exception {
+ testingData = createTestingData();
+ testee = initThreadIdGuessingAlgorithm(testingData);
+ newBasedMessageId = initNewBasedMessageId();
+
+ mailboxManager = testingData.getMailboxManager();
+ mailboxSession = mailboxManager.createSystemSession(USER);
+ mailboxManager.createMailbox(MailboxPath.inbox(USER), mailboxSession);
+ inbox = mailboxManager.getMailbox(MailboxPath.inbox(USER), mailboxSession);
+ }
+
+ @Test
+ void givenNonMailWhenAddAMailThenGuessingThreadIdShouldBasedOnGeneratedMessageId() throws Exception {
+ ThreadId threadId = testee.guessThreadIdReactive(newBasedMessageId, Optional.of(new MimeMessageId("abc")), Optional.empty(), Optional.empty(), Optional.of(new Subject("test")), mailboxSession).block();
+
+ assertThat(threadId.getBaseMessageId()).isEqualTo(newBasedMessageId);
+ }
+
+ @Test
+ void givenOldMailWhenAddNewRelatedMailsThenGuessingThreadIdShouldReturnSameThreadIdWithOldMail() throws Exception {
+ // given old mail
+ MessageManager.AppendResult message = inbox.appendMessage(MessageManager.AppendCommand.from(Message.Builder.of()
+ .setSubject("Test")
+ .setMessageId("Message-ID")
+ .setField(new RawField("In-Reply-To", "someInReplyTo"))
+ .addField(new RawField("References", "references1"))
+ .addField(new RawField("References", "references2"))
+ .setBody("testmail", StandardCharsets.UTF_8)), mailboxSession);
+
+ // add mails related to old message by subject and Message-ID (but this should not happen in real world cause every mail should have an unique MimeMessageId)
+ ThreadId threadId1 = testee.guessThreadIdReactive(newBasedMessageId, Optional.of(new MimeMessageId("Message-ID")), Optional.empty(), Optional.empty(), Optional.of(new Subject("Re: Test")), mailboxSession).block();
+ ThreadId threadId2 = testee.guessThreadIdReactive(newBasedMessageId, Optional.of(new MimeMessageId("someInReplyTo")), Optional.empty(), Optional.empty(), Optional.of(new Subject("Re: Test")), mailboxSession).block();
+ ThreadId threadId3 = testee.guessThreadIdReactive(newBasedMessageId, Optional.of(new MimeMessageId("references1")), Optional.empty(), Optional.empty(), Optional.of(new Subject("Re: Test")), mailboxSession).block();
+
+ // add mails related to old message by subject and In-Reply-To
+ ThreadId threadId4 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.of(new MimeMessageId("Message-ID")), Optional.empty(), Optional.of(new Subject("Re: Test")), mailboxSession).block();
+ ThreadId threadId5 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.of(new MimeMessageId("someInReplyTo")), Optional.empty(), Optional.of(new Subject("Re: Test")), mailboxSession).block();
+ ThreadId threadId6 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.of(new MimeMessageId("references2")), Optional.empty(), Optional.of(new Subject("Fwd: Re: Test")), mailboxSession).block();
+
+ // add mails related to old message by subject and References
+ ThreadId threadId7 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.empty(), Optional.of(List.of(new MimeMessageId("Message-ID"))), Optional.of(new Subject("Fwd: Re: Test")), mailboxSession).block();
+ ThreadId threadId8 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.empty(), Optional.of(List.of(new MimeMessageId("someInReplyTo"))), Optional.of(new Subject("Fwd: Re: Test")), mailboxSession).block();
+ ThreadId threadId9 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.empty(), Optional.of(List.of(new MimeMessageId("references1"), new MimeMessageId("NonRelated-references2"))), Optional.of(new Subject("Fwd: Re: Test")), mailboxSession).block();
+ ThreadId threadId10 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.empty(), Optional.of(List.of(new MimeMessageId("NonRelated-references1"), new MimeMessageId("references2"))), Optional.of(new Subject("Fwd: Re: Test")), mailboxSession).block();
+ ThreadId threadId11 = testee.guessThreadIdReactive(newBasedMessageId, Optional.empty(), Optional.empty(), Optional.of(List.of(new MimeMessageId("references1"), new MimeMessageId("references2"))), Optional.of(new Subject("Fwd: Re: Test")), mailboxSession).block();
+
+ // guess ThreadId should return the same ThreadId with old message
+ assertThat(threadId1).isEqualTo(message.getThreadId());
+ assertThat(threadId2).isEqualTo(message.getThreadId());
+ assertThat(threadId3).isEqualTo(message.getThreadId());
+ assertThat(threadId4).isEqualTo(message.getThreadId());
+ assertThat(threadId5).isEqualTo(message.getThreadId());
+ assertThat(threadId6).isEqualTo(message.getThreadId());
+ assertThat(threadId7).isEqualTo(message.getThreadId());
+ assertThat(threadId8).isEqualTo(message.getThreadId());
+ assertThat(threadId9).isEqualTo(message.getThreadId());
+ assertThat(threadId10).isEqualTo(message.getThreadId());
+ assertThat(threadId11).isEqualTo(message.getThreadId());
Review comment:
Maybe you could have a look to parametrized tests to be making this more readable?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #529:
URL: https://github.com/apache/james-project/pull/529#discussion_r664166525
##########
File path: mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/SearchThreadIdGuessingAlgorithmTest.java
##########
@@ -0,0 +1,153 @@
+/******************************************************************
+ * 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.mailbox.store.search;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageIdManager;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.inmemory.InMemoryMailboxManager;
+import org.apache.james.mailbox.inmemory.InMemoryMessageId;
+import org.apache.james.mailbox.inmemory.manager.InMemoryIntegrationResources;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.ThreadId;
+import org.apache.james.mailbox.store.mail.SearchThreadIdGuessingAlgorithm;
+import org.apache.james.mailbox.store.mail.model.MimeMessageId;
+import org.apache.james.mailbox.store.mail.model.Subject;
+import org.apache.james.mime4j.dom.Message;
+import org.apache.james.mime4j.stream.RawField;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class SearchThreadIdGuessingAlgorithmTest {
Review comment:
No...
I mean have this test suite as an interface, and default methods (we call that a contract). The test then implements the contract and tells which implementation to use. The contract is in mailbox/store but the test can be in mailbox/scanning-search without issues.
--
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 edited a comment on pull request #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
quantranhong1999 edited a comment on pull request #529:
URL: https://github.com/apache/james-project/pull/529#issuecomment-876292463
By the way when debug **setMessagesShouldReturnCreatedMessageWithEmptySubjectWhenSubjectIsEmpty** test, I see some thing weird. We have **a empty input subject**:
After parsing using headers.getField("Subject"), we got a null value UnstructureField
After unstructuredField.getValue(), it converts null value -> **a String with a white space**.
--
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 #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on pull request #529:
URL: https://github.com/apache/james-project/pull/529#issuecomment-875230236
Should we bind SearchThreadIdGuessingAlgorithm.class to MemoryMailboxModule?
--
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 #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on pull request #529:
URL: https://github.com/apache/james-project/pull/529#issuecomment-876354051
Squash fixups + add https://github.com/apache/james-project/pull/533/commits/ff5308b57a796defe9952434ff3d75618692adf9 (for now) + fix email/import fail test commit.
There are JPA failing tests, I will continue to fix them tomorrow.
--
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 #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
Arsnael commented on pull request #529:
URL: https://github.com/apache/james-project/pull/529#issuecomment-877961680
Can you squash the fixup and remove your cherry-pick, as that commit has been merged already?
--
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 #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on pull request #529:
URL: https://github.com/apache/james-project/pull/529#issuecomment-875304757
Ok, so it looks like guice handles the loop easily.
Do we need to put SearchThreadIdGuessingAlgorithm into InMemoryIntegrationResources? We have two loop constructors there and some lower level tests than HTTP call could not use SearchThreadIdGuessingAlgorithm.
--
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 #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
chibenwa merged pull request #529:
URL: https://github.com/apache/james-project/pull/529
--
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 change in pull request #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #529:
URL: https://github.com/apache/james-project/pull/529#discussion_r663985549
##########
File path: mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/SearchThreadIdGuessingAlgorithmTest.java
##########
@@ -0,0 +1,153 @@
+/******************************************************************
+ * 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.mailbox.store.search;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageIdManager;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.inmemory.InMemoryMailboxManager;
+import org.apache.james.mailbox.inmemory.InMemoryMessageId;
+import org.apache.james.mailbox.inmemory.manager.InMemoryIntegrationResources;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.ThreadId;
+import org.apache.james.mailbox.store.mail.SearchThreadIdGuessingAlgorithm;
+import org.apache.james.mailbox.store.mail.model.MimeMessageId;
+import org.apache.james.mailbox.store.mail.model.Subject;
+import org.apache.james.mime4j.dom.Message;
+import org.apache.james.mime4j.stream.RawField;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class SearchThreadIdGuessingAlgorithmTest {
Review comment:
Can we extract a junit 5 contract off this?
Locate the junit 5 contract in mailbox/store.
--
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 change in pull request #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #529:
URL: https://github.com/apache/james-project/pull/529#discussion_r663984411
##########
File path: mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/NaiveThreadIdGuessingAlgorithm.java
##########
@@ -32,7 +32,12 @@
public class NaiveThreadIdGuessingAlgorithm implements ThreadIdGuessingAlgorithm {
@Override
- public ThreadId guessThreadId(Username username, MessageId messageId, Optional<MimeMessageId> thisMimeMessageId, Optional<MimeMessageId> inReplyTo, Optional<List<MimeMessageId>> references, Optional<Subject> subject) {
+ public ThreadId guessThreadId(MessageId messageId, Optional<MimeMessageId> thisMimeMessageId, Optional<MimeMessageId> inReplyTo, Optional<List<MimeMessageId>> references, Optional<Subject> subject, MailboxSession session) {
return ThreadId.fromBaseMessageId(messageId);
}
+
+ @Override
+ public Mono<ThreadId> guessThreadIdReactive(MessageId messageId, Optional<MimeMessageId> thisMimeMessageId, Optional<MimeMessageId> inReplyTo, Optional<List<MimeMessageId>> references, Optional<Subject> subject, MailboxSession session) {
Review comment:
This is a new interface, let's just have the reactive version!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on a change in pull request #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #529:
URL: https://github.com/apache/james-project/pull/529#discussion_r667621640
##########
File path: mailbox/store/src/test/java/org/apache/james/mailbox/store/ThreadIdGuessingAlgorithmContract.java
##########
@@ -0,0 +1,205 @@
+/******************************************************************
+ * 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.mailbox.store;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Stream;
+
+import org.apache.james.core.Username;
+import org.apache.james.mailbox.MailboxManager;
+import org.apache.james.mailbox.MailboxSession;
+import org.apache.james.mailbox.MessageManager;
+import org.apache.james.mailbox.model.MailboxPath;
+import org.apache.james.mailbox.model.MessageId;
+import org.apache.james.mailbox.model.ThreadId;
+import org.apache.james.mailbox.store.mail.ThreadIdGuessingAlgorithm;
+import org.apache.james.mailbox.store.mail.model.MimeMessageId;
+import org.apache.james.mailbox.store.mail.model.Subject;
+import org.apache.james.mime4j.dom.Message;
+import org.apache.james.mime4j.stream.RawField;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+public abstract class ThreadIdGuessingAlgorithmContract {
+ public static final Username USER = Username.of("quan");
+ private MailboxManager mailboxManager;
+ private MessageManager inbox;
+ private ThreadIdGuessingAlgorithm testee;
+ private MailboxSession mailboxSession;
+ private CombinationManagerTestSystem testingData;
+ private MessageId newBasedMessageId;
+
+ protected abstract CombinationManagerTestSystem createTestingData();
+
+ protected abstract ThreadIdGuessingAlgorithm initThreadIdGuessingAlgorithm(CombinationManagerTestSystem testingData);
+
+ protected abstract MessageId initNewBasedMessageId();
+
+ @BeforeEach
+ void setUp() throws Exception {
+ testingData = createTestingData();
+ testee = initThreadIdGuessingAlgorithm(testingData);
+ newBasedMessageId = initNewBasedMessageId();
+
+ mailboxManager = testingData.getMailboxManager();
+ mailboxSession = mailboxManager.createSystemSession(USER);
+ mailboxManager.createMailbox(MailboxPath.inbox(USER), mailboxSession);
+ inbox = mailboxManager.getMailbox(MailboxPath.inbox(USER), mailboxSession);
+ }
+
+ @Test
+ void givenNonMailWhenAddAMailThenGuessingThreadIdShouldBasedOnGeneratedMessageId() throws Exception {
+ ThreadId threadId = testee.guessThreadIdReactive(newBasedMessageId, Optional.of(new MimeMessageId("abc")), Optional.empty(), Optional.empty(), Optional.of(new Subject("test")), mailboxSession).block();
+
+ assertThat(threadId.getBaseMessageId()).isEqualTo(newBasedMessageId);
+ }
+
+ private static Stream<Arguments> givenOldMailWhenAddNewRelatedMailsThenGuessingThreadIdShouldReturnSameThreadIdWithOldMail() {
Review comment:
I don't see anywhere to call it, is it necessary?
--
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 #529: JAMES-3516 Implement SearchThreadIdGuessingAlgorithm
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on pull request #529:
URL: https://github.com/apache/james-project/pull/529#issuecomment-876270028
We had a bug. I tried to fix it also.
But your fixes are more comprehensive, so thank you and I will take it.
--
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