Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JAMES-3516 Implement SearchThreadIdGuessingAlgorithm #529

Merged

Conversation

quantranhong1999
Copy link
Contributor

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.

Copy link
Contributor

@chibenwa chibenwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This searching implementation is actually ok to be used within server/apps/memory-app ;-)

Copy link
Contributor

@Arsnael Arsnael left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read it

@quantranhong1999
Copy link
Contributor Author

Should we bind SearchThreadIdGuessingAlgorithm.class to MemoryMailboxModule?

@chibenwa
Copy link
Contributor

chibenwa commented Jul 7, 2021

Should we bind SearchThreadIdGuessingAlgorithm.class to MemoryMailboxModule?

IMO yes :-p

@quantranhong1999
Copy link
Contributor Author

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.

@chibenwa
Copy link
Contributor

chibenwa commented Jul 8, 2021

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

@chibenwa
Copy link
Contributor

chibenwa commented Jul 8, 2021

@quantranhong1999
Copy link
Contributor Author

We had a bug. I tried to fix it also.
image

But your fixes are more comprehensive, so thank you and I will take it.

@quantranhong1999
Copy link
Contributor Author

quantranhong1999 commented Jul 8, 2021

By the way when debug setMessagesShouldReturnCreatedMessageWithEmptySubjectWhenSubjectIsEmpty test, I see some thing weird. We have a empty input subject:
image

After parsing using headers.getField("Subject"), we got a null value UnstructureField
image

After unstructuredField.getValue(), it converts null value -> a String with a white space.
image

@quantranhong1999
Copy link
Contributor Author

quantranhong1999 commented Jul 8, 2021

Squash fixups + cherry pick ff5308b (for now) + fix email/import fail test commit.
There are JPA failing tests, I will continue to fix them tomorrow.

@Arsnael
Copy link
Contributor

Arsnael commented Jul 12, 2021

Can you squash the fixup and remove your cherry-pick, as that commit has been merged already?

NaiveThreadIdGuessingAlgorithmImpl -> NaiveThreadIdGuessingAlgorithm
Remove Username, add MailboxSession to guessThreadId method params.
Enable guessThreadIdReactive API.
Experiment guessing threadId by doing search
@chibenwa chibenwa merged commit a899f63 into apache:master Jul 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants