Skip to content

Conversation

@omerfarukicen
Copy link

The parameter attachment.storage.enabled=false can be used to configure whether attachments are stored in the database.

public static final String JOOQ_REACTIVE_TIMEOUT = "jooq.reactive.timeout";
public static final Duration JOOQ_REACTIVE_TIMEOUT_DEFAULT_VALUE = Duration.ofSeconds(10);
public static final String ATTACHMENT_STORAGE_ENABLED = "attachment.storage.enabled";
public static final boolean ATTACHMENT_STORAGE_ENABLED_DEFAULT_VALUE = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

For JMAP users this makes sens to have this true

Maybe we can keep the existing behaviour given we provide an easy option to opt out?

@chibenwa
Copy link
Contributor

The current change do break the delete listener

image

We can likely relax MessageStorer.WithoutAttachment and instead return an empty list of messages.

@omerfarukicen
Copy link
Author

Test classes have been updated, and the default value is set to true.

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.

Hello and thanks for the contribution!

However, why are all the emls being rewritten here?

.then(deleteAttachment(msgId, attachmentDAO))
.then(postgresConfiguration.isAttachmentStorageEnabled()
? deleteAttachment(msgId, attachmentDAO)
: Mono.empty())
Copy link
Contributor

@Arsnael Arsnael Oct 23, 2025

Choose a reason for hiding this comment

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

The conditional operator ain't really much used in James codebase. We usually prefer extracting this into a private method and using an explicit if condition.

Copy link
Author

Choose a reason for hiding this comment

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

Ok. I’ll fix it right away.

…Listener

Extract attachment deletion conditional logic into private method with explicit
if condition, following James code style conventions.
@chibenwa
Copy link
Contributor

chibenwa commented Oct 23, 2025

So far latests fixes looks fine to me.

Can this work be rebased and squashed so that we have a clean history ready for merge?

Screenshot from 2025-10-24 01-16-20

Thanks by advance.

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