Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-13 Thread via GitHub


chibenwa commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1994648347

   How about being clever and reworking `AttachmentId`?
   
   We could have:
- An interface with a Factory for `AttachmentId`
- Current AttachmentId class be renamed as` LegacyAttachmentId` and used by 
Cassandra app, JPA, memory
- Have a new `UuidBackedAttachmentId` used in the Postgres app ?
   


-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-13 Thread via GitHub


hungphan227 closed pull request #2048: JAMES-2586 Fix some jmap postgres 
integration tests
URL: https://github.com/apache/james-project/pull/2048


-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-12 Thread via GitHub


Arsnael commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1993147029

   The JMAP integration test part seems to take often 40 ~ 50 minutes to 
complete which is too long (we consider less than 30 minutes ok)
   
   @hungphan227 Can we investigate again?


-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-12 Thread via GitHub


Arsnael commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1990927785

   Build was green (finally)
   
   I squashed and rebased


-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-11 Thread via GitHub


Arsnael commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1989922553

   11:29:37,710 [INFO] Apache James :: Server :: JMAP RFC-8621 :: Postgres 
Integration Testing FAILURE [46:53 min]
   
   46 mins... Maybe unstable CI or maybe not. Will restart the build to see


-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-10 Thread via GitHub


vttranlina commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1987633242

   > Thanks.
   > 
   > Can you test guys locally on those tests and make sure it passes with 
those fixes? => 
https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-2095/3/testReport/
   
   - `webSocketShouldPushNewMessageWhenChangeSubscriptionOfMailbox` -> failed 
on my local. Related it, I discussed it with @hungphan227 on last Thursday, it 
failed because the test suite code, was not related to Postgres -> should 
update the test code
   
   - `checkShouldTurnFromDegradedToHealthyAfterMoreReadsOnAMissedProjection` -> 
failed
   
   - `shouldFailWithCannotCalculateChangesWhenSingleChangeIsTooLarge` -> passed


-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-10 Thread via GitHub


Arsnael commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1987619546

   Thanks.
   
   Can you test guys locally on those tests and make sure it passes with those 
fixes? => 
https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-2095/3/testReport/
   
   


-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-10 Thread via GitHub


vttranlina commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1987593874

   > fixup! JAMES-2586 Optimize AttachmentLoader
   
   Fixed 
https://github.com/apache/james-project/pull/2048/commits/144ac90c6483eda966301297afcf9c5cc73e55d6
   
   Reason: Hanging when Collections is empty


-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-08 Thread via GitHub


Arsnael commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1985339243

   @hungphan227 @vttranlina I'm going on my PR to test back without the last 
commit, could not fix it. 
   
   Feel free to fix it yourselves if you want it back
   
   For example locally you can try to run `UidSearchOnIndex`, you will see 
quickly it's hanging forever


-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-07 Thread via GitHub


Arsnael commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1517293461


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresMessageIdMapper.java:
##
@@ -130,18 +142,44 @@ public Publisher 
findMetadata(MessageId messageId
 
 @Override
 public Flux findReactive(Collection messageIds, 
MessageMapper.FetchType fetchType) {
-return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
-fetchType)
+return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
 fetchType)
 .flatMap(messageBuilderAndRecord -> {
 SimpleMailboxMessage.Builder messageBuilder = 
messageBuilderAndRecord.getLeft();
+Record record = messageBuilderAndRecord.getRight();
 if (fetchType == MessageMapper.FetchType.FULL) {
-return 
retrieveFullContent(messageBuilderAndRecord.getRight())
-.map(headerAndBodyContent -> 
messageBuilder.content(headerAndBodyContent).build());
+return retrieveFullMessage(messageBuilder, record);
 }
 return Mono.just(messageBuilder.build());
 }, ReactorUtils.DEFAULT_CONCURRENCY);
 }
 
+private Mono 
retrieveFullMessage(SimpleMailboxMessage.Builder messageBuilder, Record record) 
{

Review Comment:
   FYI that commit is what is making tests failing/hanging forever on imap mpt 
tests



-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-07 Thread via GitHub


Arsnael commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1517293461


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresMessageIdMapper.java:
##
@@ -130,18 +142,44 @@ public Publisher 
findMetadata(MessageId messageId
 
 @Override
 public Flux findReactive(Collection messageIds, 
MessageMapper.FetchType fetchType) {
-return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
-fetchType)
+return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
 fetchType)
 .flatMap(messageBuilderAndRecord -> {
 SimpleMailboxMessage.Builder messageBuilder = 
messageBuilderAndRecord.getLeft();
+Record record = messageBuilderAndRecord.getRight();
 if (fetchType == MessageMapper.FetchType.FULL) {
-return 
retrieveFullContent(messageBuilderAndRecord.getRight())
-.map(headerAndBodyContent -> 
messageBuilder.content(headerAndBodyContent).build());
+return retrieveFullMessage(messageBuilder, record);
 }
 return Mono.just(messageBuilder.build());
 }, ReactorUtils.DEFAULT_CONCURRENCY);
 }
 
+private Mono 
retrieveFullMessage(SimpleMailboxMessage.Builder messageBuilder, Record record) 
{

Review Comment:
   FYI that commit is what is making tests failing on imap mpt tests



-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-07 Thread via GitHub


Arsnael commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1983105836

   @quantranhong1999 => https://github.com/apache/james-project/pull/2095
   
   My 50 cents, let's see :)


-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-07 Thread via GitHub


Arsnael commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1983038335

   No I don't think so... I think I have a clue, and maybe we are a bit unlucky 
on some of our builds... let me try something and open an other PR, I noticed a 
pattern when such thing happens


-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-07 Thread via GitHub


quantranhong1999 commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1982954684

   > I'm suspecting something being potentially unstable on our side as all the 
timeout builds have occured on different workers in the last few days
   
   Any chance it is related to this change:
   https://github.com/apache/james-project/pull/2072/files
   
   ...


-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-07 Thread via GitHub


Arsnael commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1982919726

   ```
   03:25:25.251 [INFO ] o.a.j.b.p.DockerPostgresSingleton - 2024-03-07 
03:25:25.250 UTC [62] LOG:  checkpoint starting: time
   03:27:57.599 [INFO ] o.a.j.b.p.DockerPostgresSingleton - 2024-03-07 
03:27:57.598 UTC [62] LOG:  checkpoint complete: wrote 1524 buffers (9.3%); 0 
WAL file(s) added, 0 removed, 1 recycled; write=152.270 s, sync=0.001 s, 
total=152.348 s; sync files=0, longest=0.000 s, average=0.000 s; distance=16207 
kB, estimate=16207 kB; lsn=0/28E6CE0, redo lsn=0/28E6CA8
   ```
   Hanging again, same point... do we have an issue here in our pg test with 
the docker postgres signleton extension?


-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-05 Thread via GitHub


vttranlina commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1980085869

   > Related to the ci failed  
bodyStructureShouldSupportSpecificHeaders(GuiceJamesServer) 
   
   we can cherry-pick fixup commit: 
https://github.com/apache/james-project/pull/2093
   
   


-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-05 Thread via GitHub


vttranlina commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1513805508


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresMessageIdMapper.java:
##
@@ -130,18 +142,44 @@ public Publisher 
findMetadata(MessageId messageId
 
 @Override
 public Flux findReactive(Collection messageIds, 
MessageMapper.FetchType fetchType) {
-return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
-fetchType)
+return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
 fetchType)
 .flatMap(messageBuilderAndRecord -> {
 SimpleMailboxMessage.Builder messageBuilder = 
messageBuilderAndRecord.getLeft();
+Record record = messageBuilderAndRecord.getRight();
 if (fetchType == MessageMapper.FetchType.FULL) {
-return 
retrieveFullContent(messageBuilderAndRecord.getRight())
-.map(headerAndBodyContent -> 
messageBuilder.content(headerAndBodyContent).build());
+return retrieveFullMessage(messageBuilder, record);
 }
 return Mono.just(messageBuilder.build());
 }, ReactorUtils.DEFAULT_CONCURRENCY);
 }
 
+private Mono 
retrieveFullMessage(SimpleMailboxMessage.Builder messageBuilder, Record record) 
{

Review Comment:
   I don't have permission, you can fetch my code, and cherry-pick this commit



-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-05 Thread via GitHub


hungphan227 commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1513797407


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresMessageIdMapper.java:
##
@@ -130,18 +142,44 @@ public Publisher 
findMetadata(MessageId messageId
 
 @Override
 public Flux findReactive(Collection messageIds, 
MessageMapper.FetchType fetchType) {
-return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
-fetchType)
+return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
 fetchType)
 .flatMap(messageBuilderAndRecord -> {
 SimpleMailboxMessage.Builder messageBuilder = 
messageBuilderAndRecord.getLeft();
+Record record = messageBuilderAndRecord.getRight();
 if (fetchType == MessageMapper.FetchType.FULL) {
-return 
retrieveFullContent(messageBuilderAndRecord.getRight())
-.map(headerAndBodyContent -> 
messageBuilder.content(headerAndBodyContent).build());
+return retrieveFullMessage(messageBuilder, record);
 }
 return Mono.just(messageBuilder.build());
 }, ReactorUtils.DEFAULT_CONCURRENCY);
 }
 
+private Mono 
retrieveFullMessage(SimpleMailboxMessage.Builder messageBuilder, Record record) 
{

Review Comment:
   great. tks. could you push it to this PR?



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

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

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-05 Thread via GitHub


vttranlina commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1980061620

   Related to the ci failed
   ```
   bodyStructureShouldSupportSpecificHeaders(GuiceJamesServer) – 
org.apache.james.jmap.rfc8621.postgres.PostgresEmailGetMethodTest
   
   JSON documents are different:
   Different value found in node 
"methodResponses[0][1].list[0].bodyStructure.blobId", expected: <"1_1"> but 
was: <"018e11c3-8f00-7d96-a046-4f8beef18834_1">.
   ```
   
   I see we get the same in the Distributed test (master branch)
   
![image](https://github.com/apache/james-project/assets/81145350/d378ca91-c2be-4950-b665-59b06d382970)
   
   It looks like comes from test cases, not related to Postgres or not
   //
   I'm investigating 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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-05 Thread via GitHub


vttranlina commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1513787434


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresMessageIdMapper.java:
##
@@ -130,18 +142,44 @@ public Publisher 
findMetadata(MessageId messageId
 
 @Override
 public Flux findReactive(Collection messageIds, 
MessageMapper.FetchType fetchType) {
-return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
-fetchType)
+return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
 fetchType)
 .flatMap(messageBuilderAndRecord -> {
 SimpleMailboxMessage.Builder messageBuilder = 
messageBuilderAndRecord.getLeft();
+Record record = messageBuilderAndRecord.getRight();
 if (fetchType == MessageMapper.FetchType.FULL) {
-return 
retrieveFullContent(messageBuilderAndRecord.getRight())
-.map(headerAndBodyContent -> 
messageBuilder.content(headerAndBodyContent).build());
+return retrieveFullMessage(messageBuilder, record);
 }
 return Mono.just(messageBuilder.build());
 }, ReactorUtils.DEFAULT_CONCURRENCY);
 }
 
+private Mono 
retrieveFullMessage(SimpleMailboxMessage.Builder messageBuilder, Record record) 
{

Review Comment:
   caught
   
   - Current `AttachmentLoader` get each by each `MessageAttachmentMetadata` by 
id 
   - Your new code: get list MessageAttachmentMetadata by list id  :+1:
   
   I have a refactor for that  
https://github.com/apache/james-project/commit/437550fd687ae4cbfd6986cbb712f2d940453061
   can you look 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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-05 Thread via GitHub


vttranlina commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1513757850


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/dao/PostgresAttachmentDAO.java:
##
@@ -80,9 +83,20 @@ public Mono> 
getAttachment(AttachmentId attachm
 blobIdFactory.from(row.get(PostgresAttachmentTable.BLOB_ID;
 }
 
+public Flux getAttachments(Collection 
attachmentIds) {
+return postgresExecutor.executeRows(dslContext -> 
Flux.from(dslContext.selectFrom(PostgresAttachmentTable.TABLE_NAME)

Review Comment:
   fair enough



-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-05 Thread via GitHub


vttranlina commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1513738873


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresMessageIdMapper.java:
##
@@ -130,18 +142,44 @@ public Publisher 
findMetadata(MessageId messageId
 
 @Override
 public Flux findReactive(Collection messageIds, 
MessageMapper.FetchType fetchType) {
-return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
-fetchType)
+return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
 fetchType)
 .flatMap(messageBuilderAndRecord -> {
 SimpleMailboxMessage.Builder messageBuilder = 
messageBuilderAndRecord.getLeft();
+Record record = messageBuilderAndRecord.getRight();
 if (fetchType == MessageMapper.FetchType.FULL) {
-return 
retrieveFullContent(messageBuilderAndRecord.getRight())
-.map(headerAndBodyContent -> 
messageBuilder.content(headerAndBodyContent).build());
+return retrieveFullMessage(messageBuilder, record);
 }
 return Mono.just(messageBuilder.build());
 }, ReactorUtils.DEFAULT_CONCURRENCY);
 }
 
+private Mono 
retrieveFullMessage(SimpleMailboxMessage.Builder messageBuilder, Record record) 
{

Review Comment:
   let me try it a bit



-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-05 Thread via GitHub


hungphan227 commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1513733102


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresMessageIdMapper.java:
##
@@ -130,18 +142,44 @@ public Publisher 
findMetadata(MessageId messageId
 
 @Override
 public Flux findReactive(Collection messageIds, 
MessageMapper.FetchType fetchType) {
-return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
-fetchType)
+return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
 fetchType)
 .flatMap(messageBuilderAndRecord -> {
 SimpleMailboxMessage.Builder messageBuilder = 
messageBuilderAndRecord.getLeft();
+Record record = messageBuilderAndRecord.getRight();
 if (fetchType == MessageMapper.FetchType.FULL) {
-return 
retrieveFullContent(messageBuilderAndRecord.getRight())
-.map(headerAndBodyContent -> 
messageBuilder.content(headerAndBodyContent).build());
+return retrieveFullMessage(messageBuilder, record);
 }
 return Mono.just(messageBuilder.build());
 }, ReactorUtils.DEFAULT_CONCURRENCY);
 }
 
+private Mono 
retrieveFullMessage(SimpleMailboxMessage.Builder messageBuilder, Record record) 
{

Review Comment:
   I remember that AttachmentLoader's method does not fit this case



-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-05 Thread via GitHub


hungphan227 commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1513733102


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresMessageIdMapper.java:
##
@@ -130,18 +142,44 @@ public Publisher 
findMetadata(MessageId messageId
 
 @Override
 public Flux findReactive(Collection messageIds, 
MessageMapper.FetchType fetchType) {
-return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
-fetchType)
+return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
 fetchType)
 .flatMap(messageBuilderAndRecord -> {
 SimpleMailboxMessage.Builder messageBuilder = 
messageBuilderAndRecord.getLeft();
+Record record = messageBuilderAndRecord.getRight();
 if (fetchType == MessageMapper.FetchType.FULL) {
-return 
retrieveFullContent(messageBuilderAndRecord.getRight())
-.map(headerAndBodyContent -> 
messageBuilder.content(headerAndBodyContent).build());
+return retrieveFullMessage(messageBuilder, record);
 }
 return Mono.just(messageBuilder.build());
 }, ReactorUtils.DEFAULT_CONCURRENCY);
 }
 
+private Mono 
retrieveFullMessage(SimpleMailboxMessage.Builder messageBuilder, Record record) 
{

Review Comment:
   I remember that AttachmentLoader's method that does not fit this case



##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/dao/PostgresAttachmentDAO.java:
##
@@ -80,9 +83,20 @@ public Mono> 
getAttachment(AttachmentId attachm
 blobIdFactory.from(row.get(PostgresAttachmentTable.BLOB_ID;
 }
 
+public Flux getAttachments(Collection 
attachmentIds) {
+return postgresExecutor.executeRows(dslContext -> 
Flux.from(dslContext.selectFrom(PostgresAttachmentTable.TABLE_NAME)

Review Comment:
   I think the number of attachments of a single message is not so big that we 
have to partition IN list 



-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-05 Thread via GitHub


vttranlina commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1512598925


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/dao/PostgresAttachmentDAO.java:
##
@@ -80,9 +83,20 @@ public Mono> 
getAttachment(AttachmentId attachm
 blobIdFactory.from(row.get(PostgresAttachmentTable.BLOB_ID;
 }
 
+public Flux getAttachments(Collection 
attachmentIds) {
+return postgresExecutor.executeRows(dslContext -> 
Flux.from(dslContext.selectFrom(PostgresAttachmentTable.TABLE_NAME)

Review Comment:
   As my above commented
   when using `IN` , we should partition it when `IN` size is large
   I don't see any reply about it? wdyt
   
   https://github.com/apache/james-project/pull/2048#discussion_r1502032559



##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresMessageIdMapper.java:
##
@@ -130,18 +142,44 @@ public Publisher 
findMetadata(MessageId messageId
 
 @Override
 public Flux findReactive(Collection messageIds, 
MessageMapper.FetchType fetchType) {
-return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
-fetchType)
+return 
mailboxMessageDAO.findMessagesByMessageIds(messageIds.stream().map(PostgresMessageId.class::cast).collect(ImmutableList.toImmutableList()),
 fetchType)
 .flatMap(messageBuilderAndRecord -> {
 SimpleMailboxMessage.Builder messageBuilder = 
messageBuilderAndRecord.getLeft();
+Record record = messageBuilderAndRecord.getRight();
 if (fetchType == MessageMapper.FetchType.FULL) {
-return 
retrieveFullContent(messageBuilderAndRecord.getRight())
-.map(headerAndBodyContent -> 
messageBuilder.content(headerAndBodyContent).build());
+return retrieveFullMessage(messageBuilder, record);
 }
 return Mono.just(messageBuilder.build());
 }, ReactorUtils.DEFAULT_CONCURRENCY);
 }
 
+private Mono 
retrieveFullMessage(SimpleMailboxMessage.Builder messageBuilder, Record record) 
{

Review Comment:
   I see a lot of methods were written for adding attachment metadata to 
`SimpleMailboxMessage.Builder`.
   it is similar to what we did in 
[AttachmentLoader](https://github.com/apache/james-project/blob/3243752f862aa9cf4dff9474878837368f4442c4/mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/AttachmentLoader.java)
   
   Can we reuse 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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-03-03 Thread via GitHub


Arsnael commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1975924591

   Please rebase


-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-02-28 Thread via GitHub


quantranhong1999 commented on PR #2048:
URL: https://github.com/apache/james-project/pull/2048#issuecomment-1970391358

   org.apache.james.events.PostgresEventDeadLettersTest 
   
   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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-02-26 Thread via GitHub


hungphan227 commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1502284797


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/dao/PostgresAttachmentDAO.java:
##
@@ -80,9 +84,21 @@ public Mono> 
getAttachment(AttachmentId attachm
 blobIdFactory.from(row.get(PostgresAttachmentTable.BLOB_ID;
 }
 
+public Flux getAttachments(Collection 
attachmentIds) {
+return postgresExecutor.executeRows(dslContext -> 
Flux.from(dslContext.selectFrom(PostgresAttachmentTable.TABLE_NAME)
+
.where(PostgresAttachmentTable.ID_AS_UUID.in(attachmentIds.stream().map(AttachmentId::asUUID).collect(ImmutableList.toImmutableList())

Review Comment:
   Removed ID_AS_UUID. Use ID as pk (change type from UUID to VARCHAR)



-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-02-26 Thread via GitHub


hungphan227 commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1502284352


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresAttachmentModule.java:
##
@@ -35,20 +35,22 @@ public interface PostgresAttachmentModule {
 interface PostgresAttachmentTable {
 
 Table TABLE_NAME = DSL.table("attachment");
-Field ID = DSL.field("id", SQLDataType.UUID.notNull());
+Field ID_AS_UUID = DSL.field("id_as_uuid", 
SQLDataType.UUID.notNull());
+Field ID = DSL.field("id", SQLDataType.VARCHAR.notNull());

Review Comment:
   Removed ID_AS_UUID. Use ID as pk (change type from UUID to VARCHAR)



-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-02-26 Thread via GitHub


vttranlina commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1502211561


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/dao/PostgresAttachmentDAO.java:
##
@@ -80,9 +84,21 @@ public Mono> 
getAttachment(AttachmentId attachm
 blobIdFactory.from(row.get(PostgresAttachmentTable.BLOB_ID;
 }
 
+public Flux getAttachments(Collection 
attachmentIds) {
+return postgresExecutor.executeRows(dslContext -> 
Flux.from(dslContext.selectFrom(PostgresAttachmentTable.TABLE_NAME)
+
.where(PostgresAttachmentTable.ID_AS_UUID.in(attachmentIds.stream().map(AttachmentId::asUUID).collect(ImmutableList.toImmutableList())

Review Comment:
   (1) updated
   Understood why you need ID and ID_AS_UUID
   I think we can use a single column for it. (`ID`, maybe. And change type 
from UUID -> VARCHAR)
   it is enough. 
   



-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-02-25 Thread via GitHub


vttranlina commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1502032559


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/dao/PostgresAttachmentDAO.java:
##
@@ -80,9 +84,21 @@ public Mono> 
getAttachment(AttachmentId attachm
 blobIdFactory.from(row.get(PostgresAttachmentTable.BLOB_ID;
 }
 
+public Flux getAttachments(Collection 
attachmentIds) {
+return postgresExecutor.executeRows(dslContext -> 
Flux.from(dslContext.selectFrom(PostgresAttachmentTable.TABLE_NAME)
+
.where(PostgresAttachmentTable.ID_AS_UUID.in(attachmentIds.stream().map(AttachmentId::asUUID).collect(ImmutableList.toImmutableList())

Review Comment:
   1. Why do we don't use directly "AttachmentId.getId()" in `where` clause?
   2. When using "IN" clause, we should check the size of IN, if large, we 
should partition it, see the same 
`PostgresMailboxMessageDAO#findMessagesByMailboxIdAndUIDs`



-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-02-25 Thread via GitHub


hungphan227 commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1502017082


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresAttachmentModule.java:
##
@@ -35,20 +35,22 @@ public interface PostgresAttachmentModule {
 interface PostgresAttachmentTable {
 
 Table TABLE_NAME = DSL.table("attachment");
-Field ID = DSL.field("id", SQLDataType.UUID.notNull());
+Field ID_AS_UUID = DSL.field("id_as_uuid", 
SQLDataType.UUID.notNull());
+Field ID = DSL.field("id", SQLDataType.VARCHAR.notNull());

Review Comment:
   No, this is not "just to pass IT". In 
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/dao/PostgresAttachmentDAO.java
 line 87:
   
   ```
   public Flux getAttachments(Collection 
attachmentIds) {
   return postgresExecutor.executeRows(dslContext -> 
Flux.from(dslContext.selectFrom(PostgresAttachmentTable.TABLE_NAME)
   
.where(PostgresAttachmentTable.ID_AS_UUID.in(attachmentIds.stream().map(AttachmentId::asUUID).collect(ImmutableList.toImmutableList())
   .map(row -> AttachmentMetadata.builder()
   
.attachmentId(AttachmentId.from(row.get(PostgresAttachmentTable.ID)))
   .type(row.get(PostgresAttachmentTable.TYPE))
   
.messageId(PostgresMessageId.Factory.of(row.get(PostgresAttachmentTable.MESSAGE_ID)))
   .size(row.get(PostgresAttachmentTable.SIZE))
   .build());
   }
   ```
   
   If we do not have ID_AS_UUID, we have to use a hashmap to save AttachmentId 
- UUID (UUID.nameUUIDFromBytes(id.getBytes(StandardCharsets.UTF_8))) which is 
not good option 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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-02-25 Thread via GitHub


Arsnael commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1502013047


##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresAttachmentModule.java:
##
@@ -35,20 +35,22 @@ public interface PostgresAttachmentModule {
 interface PostgresAttachmentTable {
 
 Table TABLE_NAME = DSL.table("attachment");
-Field ID = DSL.field("id", SQLDataType.UUID.notNull());
+Field ID_AS_UUID = DSL.field("id_as_uuid", 
SQLDataType.UUID.notNull());
+Field ID = DSL.field("id", SQLDataType.VARCHAR.notNull());

Review Comment:
   If the tests fails because the structure is different, I would prefer here 
that we duplicate the test suite and adapt it to postgres attachment table 
structure instead?



-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-02-25 Thread via GitHub


hungphan227 commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1502005171


##
server/protocols/jmap-rfc-8621-integration-tests/postgres-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/postgres/PostgresThreadGetTest.java:
##
@@ -88,31 +60,11 @@ public class PostgresThreadGetTest extends PostgresBase 
implements ThreadGetCont
 .overrideWith(new TestJMAPServerModule()))
 .build();
 
-@AfterEach
-void tearDown() throws IOException {
-client.close();
-}
-
 @Override
 public void awaitMessageCount(List mailboxIds, SearchQuery 
query, long messageCount) {
-awaitForOpenSearch(queryConverter.from(mailboxIds, query), 
messageCount);
 }
 
 @Override
 public void initOpenSearchClient() {
-client = MailboxIndexCreationUtil.prepareDefaultClient(
-openSearch.getDockerOpenSearch().clientProvider().get(),
-openSearch.getDockerOpenSearch().configuration());
-}
-
-private void awaitForOpenSearch(Query query, long totalHits) {

Review Comment:
   In postgres implementation, Thread feature use postgres instead of opensearch



-- 
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-unsubscr...@james.apache.org

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


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



Re: [PR] JAMES-2586 Fix some jmap postgres integration tests [james-project]

2024-02-25 Thread via GitHub


quantranhong1999 commented on code in PR #2048:
URL: https://github.com/apache/james-project/pull/2048#discussion_r1501987659


##
server/protocols/jmap-rfc-8621-integration-tests/postgres-jmap-rfc-8621-integration-tests/src/test/java/org/apache/james/jmap/rfc8621/postgres/PostgresThreadGetTest.java:
##
@@ -88,31 +60,11 @@ public class PostgresThreadGetTest extends PostgresBase 
implements ThreadGetCont
 .overrideWith(new TestJMAPServerModule()))
 .build();
 
-@AfterEach
-void tearDown() throws IOException {
-client.close();
-}
-
 @Override
 public void awaitMessageCount(List mailboxIds, SearchQuery 
query, long messageCount) {
-awaitForOpenSearch(queryConverter.from(mailboxIds, query), 
messageCount);
 }
 
 @Override
 public void initOpenSearchClient() {
-client = MailboxIndexCreationUtil.prepareDefaultClient(
-openSearch.getDockerOpenSearch().clientProvider().get(),
-openSearch.getDockerOpenSearch().configuration());
-}
-
-private void awaitForOpenSearch(Query query, long totalHits) {

Review Comment:
   Why remove this?



##
mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresAttachmentModule.java:
##
@@ -35,20 +35,22 @@ public interface PostgresAttachmentModule {
 interface PostgresAttachmentTable {
 
 Table TABLE_NAME = DSL.table("attachment");
-Field ID = DSL.field("id", SQLDataType.UUID.notNull());
+Field ID_AS_UUID = DSL.field("id_as_uuid", 
SQLDataType.UUID.notNull());
+Field ID = DSL.field("id", SQLDataType.VARCHAR.notNull());

Review Comment:
   So we need the "redundant" ID field just to pass IT? Any other way?



##
server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailQueryMethodContract.scala:
##
@@ -7034,22 +7034,24 @@ trait EmailQueryMethodContract {
  |"c1"]]
  |}""".stripMargin
 
-val response = `given`
-  .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
-  .body(request)
-.when
-  .post
-.`then`
-  .statusCode(SC_OK)
-  .contentType(JSON)
-  .extract
-  .body
-  .asString
+awaitAtMostTenSeconds.untilAsserted { () =>
+  val response = `given`
+.header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+.body(request)
+.when
+.post
+.`then`

Review Comment:
   indent



-- 
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-unsubscr...@james.apache.org

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


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