This is an automated email from the ASF dual-hosted git repository.
rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
The following commit(s) were added to refs/heads/master by this push:
new 37c88ebdbd JAMES-2586 Fix concurrency issue of updating ACL in
RLSSupportPostgresMailboxMapper
37c88ebdbd is described below
commit 37c88ebdbdfa5aa6bb426ce9fbd3c285fa5e985f
Author: hung phan <[email protected]>
AuthorDate: Mon Feb 24 14:09:44 2025 +0700
JAMES-2586 Fix concurrency issue of updating ACL in
RLSSupportPostgresMailboxMapper
---
.../postgres/mail/PostgresMailboxMemberDAO.java | 3 +-
.../mail/RLSSupportPostgresMailboxMapper.java | 47 +++++++++++++---------
.../RLSSupportPostgresMailboxMapperACLTest.java | 32 ++++++++++-----
.../store/mail/model/MailboxMapperACLTest.java | 3 +-
4 files changed, 52 insertions(+), 33 deletions(-)
diff --git
a/mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresMailboxMemberDAO.java
b/mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresMailboxMemberDAO.java
index 5cf73eb29f..b5557ab11f 100644
---
a/mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresMailboxMemberDAO.java
+++
b/mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/PostgresMailboxMemberDAO.java
@@ -52,7 +52,8 @@ public class PostgresMailboxMemberDAO {
.map(username -> dslContext.newRecord(USER_NAME, MAILBOX_ID)
.value1(username.asString())
.value2(mailboxId.asUuid()))
- .toList())));
+ .toList())
+ .onConflictDoNothing()));
}
public Mono<Void> delete(PostgresMailboxId mailboxId, List<Username>
usernames) {
diff --git
a/mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/RLSSupportPostgresMailboxMapper.java
b/mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/RLSSupportPostgresMailboxMapper.java
index aa3db2b311..efa77a24eb 100644
---
a/mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/RLSSupportPostgresMailboxMapper.java
+++
b/mailbox/postgres/src/main/java/org/apache/james/mailbox/postgres/mail/RLSSupportPostgresMailboxMapper.java
@@ -19,20 +19,21 @@
package org.apache.james.mailbox.postgres.mail;
+import java.time.Duration;
import java.util.function.Function;
import org.apache.james.core.Username;
import org.apache.james.mailbox.acl.ACLDiff;
import org.apache.james.mailbox.acl.PositiveUserACLDiff;
+import org.apache.james.mailbox.exception.UnsupportedRightException;
import org.apache.james.mailbox.model.Mailbox;
import org.apache.james.mailbox.model.MailboxACL;
import org.apache.james.mailbox.postgres.PostgresMailboxId;
import org.apache.james.mailbox.postgres.mail.dao.PostgresMailboxDAO;
-import com.github.fge.lambdas.Throwing;
-
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
+import reactor.util.retry.Retry;
public class RLSSupportPostgresMailboxMapper extends PostgresMailboxMapper {
private final PostgresMailboxDAO postgresMailboxDAO;
@@ -56,30 +57,36 @@ public class RLSSupportPostgresMailboxMapper extends
PostgresMailboxMapper {
@Override
public Mono<ACLDiff> updateACL(Mailbox mailbox, MailboxACL.ACLCommand
mailboxACLCommand) {
- MailboxACL oldACL = mailbox.getACL();
- MailboxACL newACL = Throwing.supplier(() ->
oldACL.apply(mailboxACLCommand)).get();
- ACLDiff aclDiff = ACLDiff.computeDiff(oldACL, newACL);
- PositiveUserACLDiff userACLDiff = new PositiveUserACLDiff(aclDiff);
- return upsertACL(mailbox, newACL, aclDiff, userACLDiff);
+ return postgresMailboxDAO.getACL(mailbox.getMailboxId())
+ .flatMap(pairMailboxACLAndVersion -> {
+ try {
+ MailboxACL newACL =
pairMailboxACLAndVersion.getLeft().apply(mailboxACLCommand);
+ return
postgresMailboxDAO.upsertACL(mailbox.getMailboxId(), newACL,
pairMailboxACLAndVersion.getRight())
+
.thenReturn(ACLDiff.computeDiff(pairMailboxACLAndVersion.getLeft(), newACL));
+ } catch (UnsupportedRightException e) {
+ throw new RuntimeException(e);
+ }
+ }).retryWhen(Retry.backoff(3, Duration.ofMillis(100))
+ .filter(throwable -> throwable instanceof
PostgresACLUpsertException))
+ .flatMap(aclDiff -> updateMembersOfMailbox(mailbox, new
PositiveUserACLDiff(aclDiff))
+ .thenReturn(aclDiff));
}
@Override
public Mono<ACLDiff> setACL(Mailbox mailbox, MailboxACL mailboxACL) {
- MailboxACL oldACL = mailbox.getACL();
- ACLDiff aclDiff = ACLDiff.computeDiff(oldACL, mailboxACL);
- PositiveUserACLDiff userACLDiff = new PositiveUserACLDiff(aclDiff);
- return upsertACL(mailbox, mailboxACL, aclDiff, userACLDiff);
+ return postgresMailboxDAO.getACL(mailbox.getMailboxId())
+ .flatMap(pairMailboxACLAndVersion ->
+ postgresMailboxDAO.upsertACL(mailbox.getMailboxId(),
mailboxACL, pairMailboxACLAndVersion.getRight())
+
.thenReturn(ACLDiff.computeDiff(pairMailboxACLAndVersion.getLeft(),
mailboxACL))).retryWhen(Retry.backoff(3, Duration.ofMillis(100))
+ .filter(throwable -> throwable instanceof
PostgresACLUpsertException))
+ .flatMap(aclDiff -> updateMembersOfMailbox(mailbox, new
PositiveUserACLDiff(aclDiff))
+ .thenReturn(aclDiff));
}
- private Mono<ACLDiff> upsertACL(Mailbox mailbox, MailboxACL newACL,
ACLDiff aclDiff, PositiveUserACLDiff userACLDiff) {
- return postgresMailboxDAO.upsertACL(mailbox.getMailboxId(), newACL)
-
.then(postgresMailboxMemberDAO.delete(PostgresMailboxId.class.cast(mailbox.getMailboxId()),
- userACLDiff.removedEntries().map(entry ->
Username.of(entry.getKey().getName())).toList()))
+ private Mono<Void> updateMembersOfMailbox(Mailbox mailbox,
PositiveUserACLDiff userACLDiff) {
+ return
postgresMailboxMemberDAO.delete(PostgresMailboxId.class.cast(mailbox.getMailboxId()),
+ userACLDiff.removedEntries().map(entry ->
Username.of(entry.getKey().getName())).toList())
.then(postgresMailboxMemberDAO.insert(PostgresMailboxId.class.cast(mailbox.getMailboxId()),
- userACLDiff.addedEntries().map(entry ->
Username.of(entry.getKey().getName())).toList()))
- .then(Mono.fromCallable(() -> {
- mailbox.setACL(newACL);
- return aclDiff;
- }));
+ userACLDiff.addedEntries().map(entry ->
Username.of(entry.getKey().getName())).toList()));
}
}
diff --git
a/mailbox/postgres/src/test/java/org/apache/james/mailbox/postgres/mail/RLSSupportPostgresMailboxMapperACLTest.java
b/mailbox/postgres/src/test/java/org/apache/james/mailbox/postgres/mail/RLSSupportPostgresMailboxMapperACLTest.java
index 9811865f8d..3e8edd578e 100644
---
a/mailbox/postgres/src/test/java/org/apache/james/mailbox/postgres/mail/RLSSupportPostgresMailboxMapperACLTest.java
+++
b/mailbox/postgres/src/test/java/org/apache/james/mailbox/postgres/mail/RLSSupportPostgresMailboxMapperACLTest.java
@@ -19,29 +19,39 @@
package org.apache.james.mailbox.postgres.mail;
-import java.util.concurrent.ExecutionException;
+import java.time.Instant;
+import org.apache.james.backends.postgres.PostgresConfiguration;
import org.apache.james.backends.postgres.PostgresExtension;
import org.apache.james.backends.postgres.PostgresModule;
-import org.apache.james.mailbox.postgres.mail.dao.PostgresMailboxDAO;
+import org.apache.james.blob.api.BlobId;
+import org.apache.james.blob.api.BucketName;
+import org.apache.james.blob.api.PlainBlobId;
+import org.apache.james.blob.memory.MemoryBlobStoreDAO;
+import org.apache.james.mailbox.MailboxSessionUtil;
+import org.apache.james.mailbox.StringBackedAttachmentIdFactory;
+import org.apache.james.mailbox.postgres.PostgresMailboxSessionMapperFactory;
+import org.apache.james.mailbox.store.mail.AttachmentIdAssignationStrategy;
import org.apache.james.mailbox.store.mail.MailboxMapper;
import org.apache.james.mailbox.store.mail.model.MailboxMapperACLTest;
-import org.junit.jupiter.api.Disabled;
+import org.apache.james.server.blob.deduplication.DeDuplicationBlobStore;
+import org.apache.james.utils.UpdatableTickingClock;
import org.junit.jupiter.api.extension.RegisterExtension;
class RLSSupportPostgresMailboxMapperACLTest extends MailboxMapperACLTest {
@RegisterExtension
- static PostgresExtension postgresExtension =
PostgresExtension.withoutRowLevelSecurity(PostgresModule.aggregateModules(PostgresMailboxModule.MODULE,
+ static PostgresExtension postgresExtension =
PostgresExtension.withRowLevelSecurity(PostgresModule.aggregateModules(PostgresMailboxModule.MODULE,
PostgresMailboxMemberModule.MODULE));
@Override
protected MailboxMapper createMailboxMapper() {
- return new RLSSupportPostgresMailboxMapper(new
PostgresMailboxDAO(postgresExtension.getDefaultPostgresExecutor()),
- new
PostgresMailboxMemberDAO(postgresExtension.getDefaultPostgresExecutor()));
- }
-
- @Override
- @Disabled("not yet implemented")
- protected void updateAclShouldWorkWellInMultiThreadEnv() throws
ExecutionException, InterruptedException {
+ BlobId.Factory blobIdFactory = new PlainBlobId.Factory();
+ PostgresMailboxSessionMapperFactory
postgresMailboxSessionMapperFactory = new
PostgresMailboxSessionMapperFactory(postgresExtension.getExecutorFactory(),
+ new UpdatableTickingClock(Instant.now()),
+ new DeDuplicationBlobStore(new MemoryBlobStoreDAO(),
BucketName.DEFAULT, blobIdFactory),
+ blobIdFactory,
+
PostgresConfiguration.builder().username("a").password("a").rowLevelSecurityEnabled().byPassRLSUser("b").byPassRLSPassword("b").build(),
+ new AttachmentIdAssignationStrategy.Default(new
StringBackedAttachmentIdFactory()));
+ return
postgresMailboxSessionMapperFactory.getMailboxMapper(MailboxSessionUtil.create(BENWA));
}
}
diff --git
a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperACLTest.java
b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperACLTest.java
index d0e9ebccaf..f2411d8318 100644
---
a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperACLTest.java
+++
b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MailboxMapperACLTest.java
@@ -43,6 +43,7 @@ import org.junit.jupiter.api.Test;
import com.google.common.collect.ImmutableMap;
public abstract class MailboxMapperACLTest {
+ public static Username BENWA = Username.of("[email protected]");
private static final UidValidity UID_VALIDITY = UidValidity.of(42);
private static final Username USER = Username.of("user");
private static final Username USER_1 = Username.of("user1");
@@ -57,7 +58,7 @@ public abstract class MailboxMapperACLTest {
@BeforeEach
void setUp() {
mailboxMapper = createMailboxMapper();
- MailboxPath benwaInboxPath = MailboxPath.forUser(Username.of("benwa"),
"INBOX");
+ MailboxPath benwaInboxPath = MailboxPath.forUser(BENWA, "INBOX");
benwaInboxMailbox = mailboxMapper.create(benwaInboxPath,
UID_VALIDITY).block();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
