Skip to content

add Thread/get support & References to previous method results#65

Merged
alagane merged 7 commits intolinagora:mainfrom
qyb:localdev
Jun 21, 2022
Merged

add Thread/get support & References to previous method results#65
alagane merged 7 commits intolinagora:mainfrom
qyb:localdev

Conversation

@qyb
Copy link
Copy Markdown
Contributor

@qyb qyb commented Jun 15, 2022

No description provided.

@alagane
Copy link
Copy Markdown
Member

alagane commented Jun 16, 2022

Hello, isn't the method thread_get part missing from the index.ts file?

@qyb
Copy link
Copy Markdown
Contributor Author

qyb commented Jun 16, 2022

Hello, isn't the method thread_get part missing from the index.ts file?

I'm trying to add thread test with james:memory. I want to implement one request combining 4 methods (Email/query + Email/get + Thread/get + Email/get) as the description in https://jmap.io/client.html#cold-boot

But I cannot get thread work. I forged some .eml files with related Message-ID/In-Reply-To/References header. Then I use james-cli ImportEml command to import them. But the system didn't recognize threads. May you give me some tips?

@chibenwa
Copy link
Copy Markdown
Member

But I cannot get thread work. I forged some .eml files with related Message-ID/In-Reply-To/References header. Then I use james-cli ImportEml command to import them. But the system didn't recognize threads. May you give me some tips?

@quantranhong1999 ?

@chibenwa
Copy link
Copy Markdown
Member

@qyb can you share the EML in question? (attached ZIP)

@qyb
Copy link
Copy Markdown
Contributor Author

qyb commented Jun 17, 2022

But I cannot get thread work. I forged some .eml files with related Message-ID/In-Reply-To/References header. Then I use james-cli ImportEml command to import them. But the system didn't recognize threads. May you give me some tips?

@quantranhong1999 ?

I have done my research about memory-app ThreadIdGuessingAlgorithm implementation. Maybe it was just a naiveAlgorighm?

Anyway, it won't stop the test's logical.:)

@quantranhong1999
Copy link
Copy Markdown
Member

I have done my research about memory-app ThreadIdGuessingAlgorithm implementation. Maybe it was just a naiveAlgorighm?

We are using SearchThreadIdGuessingAlgorithm (search on memory) implementation for memory-app.

I want to implement one request combining 4 methods (Email/query + Email/get + Thread/get + Email/get) as the description in https://jmap.io/client.html#cold-boot. But I cannot get thread work.

As far as I know, James does not support collapseThreads option for Email/query yet so it would not work like the example for now. For short the thread paging is not support by James by now.

@qyb
Copy link
Copy Markdown
Contributor Author

qyb commented Jun 17, 2022

I have done my research about memory-app ThreadIdGuessingAlgorithm implementation. Maybe it was just a naiveAlgorighm?

We are using SearchThreadIdGuessingAlgorithm (search on memory) implementation for memory-app.

docker exec e26fd3fdb97c james-cli AddUser user1@localhost password

provisioning mailbox (by fetchSession/login...)

docker exec e26fd3fdb97c james-cli ListUserMailboxes user1@localhost
docker exec e26fd3fdb97c james-cli ImportEml "#private" user1@localhost INBOX /root/conf/1.eml
docker exec e26fd3fdb97c james-cli ImportEml "#private" user1@localhost Sent /root/conf/2.eml
docker exec e26fd3fdb97c james-cli ImportEml "#private" user1@localhost INBOX /root/conf/3.eml

I expect INBOX should be only 1 thread. but mailbox_get shows two threads in INBOX
eml.zip
.

@quantranhong1999
Copy link
Copy Markdown
Member

What is the James memory version you are using? BTW can you try Email/query and then Email/get threadId properties to see the mapping between messageId and threadId first?

@qyb
Copy link
Copy Markdown
Contributor Author

qyb commented Jun 17, 2022

As far as I know, James does not support collapseThreads option for Email/query yet so it would not work like the example for now. For short the thread paging is not support by James by now.

Yes, you're right. I imported three EML files by JMAP Email/import method. They were recognized as one threadId successfully. But collapseThreads result is still multiple threads.

Anyway. I'll add Email/import and Thread/get method in the following commits

@quantranhong1999
Copy link
Copy Markdown
Member

Good to hear that :')

@chibenwa
Copy link
Copy Markdown
Member

But collapseThreads result is still multiple threads.

We (in James) did not implement it just yet (as it would require to push aggregation down to the search engine)

Contributions welcomed for the willing ;-)

@qyb
Copy link
Copy Markdown
Contributor Author

qyb commented Jun 17, 2022

But collapseThreads result is still multiple threads.

We (in James) did not implement it just yet (as it would require to push aggregation down to the search engine)

Contributions welcomed for the willing ;-)

Now, this pull request just remains ResultReference test. :)

chibenwa
chibenwa previously approved these changes Jun 17, 2022
Copy link
Copy Markdown
Member

@alagane alagane left a comment

Choose a reason for hiding this comment

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

How is the keystore generated?
I used a keypair on my pull request #62
I don't know which method is better, but I personally prefair to not have binary files is possible.
I think the best method would be to generate the keypair/keystore when running tests if none is found on the directory, and ignore it on .gitignore

@qyb
Copy link
Copy Markdown
Contributor Author

qyb commented Jun 19, 2022

How is the keystore generated? I used a keypair on my pull request #62 I don't know which method is better, but I personally prefair to not have binary files is possible. I think the best method would be to generate the keypair/keystore when running tests if none is found on the directory, and ignore it on .gitignore

https://www.npmjs.com/package/keypair is our need.

I'll remove ResultReference object from this pr because now I think it should be in another pull request. This PR is too complex and putting multiple arguments in one request is another complex thing.

qyb and others added 2 commits June 19, 2022 13:21
Co-authored-by: Alexandre Lagane <550970+alagane@users.noreply.github.com>
@alagane
Copy link
Copy Markdown
Member

alagane commented Jun 20, 2022

Ready to merge for me.
Thank you for your contribution!

@qyb qyb requested a review from chibenwa June 20, 2022 20:26
Copy link
Copy Markdown
Member

@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 looks great to me!

@alagane alagane merged commit c3b87a8 into linagora:main Jun 21, 2022
@qyb qyb deleted the localdev branch June 21, 2022 09:52
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