JAMES-4182 Proposal for changes in BlobStoreDAO interface by chibenwa · Pull Request #2960 · apache/james-project

chibenwa · 2026-03-03T15:40:10Z

Opened to collect initial feedback

See https://issues.apache.org/jira/browse/JAMES-4182 for context

Remaining points:

better validation for metadata
Store metadata in blob stores
- xattr for files
- extra collumn for PG, cassandra
- Object metadata for S3
- Direct storage for memory
- Passing those around for other blobStore DAO
- AES blob store should support metadata

jeantil · 2026-03-10T20:34:57Z

* [ ]  better validation for metadata 
* [ ]  Store metadata in blob stores       
  * [ ]  xattr for files
  * [ ]  extra collumn for PG, cassandra
  * [ ]  Object metadata for S3
  * [ ]  Direct storage for memory
  * [ ]  Passing those around for other blobStore DAO

I think we should only use dedicated properties to store acitvely managed metadata, the remaining metadata as a Map<String,String> can be serializaed with the payload. this avoids possible capability conflicts between blog stores.

jeantil

Looks like a good direction to me, I left some comments, I'm sure there are which you are aware of and will addres, feel free to close on sight

server/blob/blob-api/src/main/java/org/apache/james/blob/api/BlobStoreDAO.java

jeantil · 2026-03-10T20:02:13Z

server/blob/blob-api/src/main/java/org/apache/james/blob/api/BlobStoreDAO.java

+    }
+
+    sealed interface Blob {
+        Map<BlobMetadataName, BlobMetadataValue> metadata();


As mentionned above it would be better for keys to be known,
alternatively this could be an object with kmown fields which james cares about and a map for extra fields which are not interpreted by james

server/blob/blob-api/src/main/java/org/apache/james/blob/api/BlobStoreDAO.java

server/blob/blob-memory/src/main/java/org/apache/james/blob/memory/MemoryBlobStoreDAO.java

server/blob/blob-aes/src/main/java/org/apache/james/blob/aes/AESBlobStoreDAO.java

server/blob/blob-api/src/main/java/org/apache/james/blob/api/BlobStoreDAO.java

chibenwa · 2026-03-11T07:34:14Z

@jeantil do we have an agreement on continuing this work with an open metadata model?

jeantil · 2026-03-12T10:54:25Z

@jeantil do we have an agreement on continuing this work with an open metadata model?

I'm not comfortable working with only the open metadata model. I understand the extension point argument but
I would much prefer an hybrid approach where metadata which is actively used by james is materialized as such in the metadata object and accessed through structured accessors instead of iterating through a map comparing strings every time we need the information
The rest of available metadata can be left as an unstructured map as some kind of extension point

chibenwa · 2026-03-12T14:51:31Z

I would much prefer an hybrid approach where metadata which is actively used by james is materialized as such in the metadata object and accessed through structured accessors instead of iterating through a map comparing strings every time we need the information

I propose:

Keeping the undlying data as (typed) Map<String, String>
Adding convenience factory methods for manipulating known value

Essentially:

record BlobMetadata(Map<BlobMetadataName, BlobMetadataValue> metadata) {
   public static BlobMetadata empty() {...}
   
   public ContentTransferEncoding contentTransferEncoding() {
       return ContentTransferEncoding.fromBlobMetadataValue(metatdata.get(BlobMetadataName.CONTENT_TRANSFER_ENCODING));
   }
   
   public BlobMetadata withMetadata(BlobMetadataName name, BlobMetadataValue value) {
       return new BlobMetadata(ImmutableList.builder()
           .putAll(metadata)
           .put(name, value)
           .build());
   }
   
   public BlobMetadata withContentTransferEncoding(ContentTransferEncoding encoding) {
       return withMetadata(BlobMetadataName.CONTENT_TRANSFER_ENCODING, encoding.asBlobMetadataValue());
   }
}

I think it keeps the underlying data model simple, extensible, easy to work with in DAOs, and it address your concerns and makes James-used metadata a prime citizen.

jeantil · 2026-03-12T14:58:34Z

this looks ok for me, ideally we can enforce usage of an immutable map especially if it remains accessible from outside

chibenwa · 2026-03-12T15:35:25Z

ideally we can enforce usage of an immutable map especially if it remains accessible from outside

Of course ;-)

chibenwa · 2026-03-12T15:36:19Z

I'd like to thank you for the review effort @jeantil

I will put together the effort to align the code with the comments, and continue the implementation likely next week.

chibenwa · 2026-03-19T08:57:30Z

Hello @jeantil

I did rework the metadata model and wrote memory implementation.

I'll carry other with other impelementation when I have time, but wanted to push now to enable early review, in case people have time.

server/blob/blob-api/src/main/java/org/apache/james/blob/api/BlobStoreDAO.java

jeantil · 2026-03-23T19:59:36Z

...blob/blob-api/src/test/java/org/apache/james/blob/api/MetadataAwareBlobStoreDAOContract.java

+
+        Mono.from(testee.save(TEST_BUCKET_NAME, TEST_BLOB_ID, bytesBlob)).block();
+
+        assertThat(Mono.from(testee.readBytes(TEST_BUCKET_NAME, TEST_BLOB_ID)).block().metadata().metadata())


I'll use this one as an example to let you pick the variant you prefer, I'm fine with both :D

Suggested change

assertThat(Mono.from(testee.readBytes(TEST_BUCKET_NAME, TEST_BLOB_ID)).block().metadata().metadata())

assertThat(Mono.from(testee.readBytes(TEST_BUCKET_NAME, TEST_BLOB_ID)).block().metadata().values())

assertThat(Mono.from(testee.readBytes(TEST_BUCKET_NAME, TEST_BLOB_ID)).block().metadata().underlyingMap())

S3MinioTest::saveShouldWorkWhenValidBlobId failed org.opentest4j.AssertionFailedError: expected: BytesBlob[payload=[B@6bca6c4c, metadata=BlobMetadata[metadata={}]] but was: BytesBlob[payload=[B@507e3e77, metadata=BlobMetadata[metadata={}]] Expected :BytesBlob[payload=[B@6bca6c4c, metadata=BlobMetadata[metadata={}]] Actual :BytesBlob[payload=[B@507e3e77, metadata=BlobMetadata[metadata={}]] By default, java compares byte[] array by their reference, not the inner bytes.

…cketWhenFailingOnDefaultOne test

…nderlying BlobStoreDAO

quantranhong1999 · 2026-04-08T08:28:50Z

Hello,

I would like to help to move on this work.

I rebased this work on master, fixed the compilation, adapted review comments, and fixed some blob store tests.

Status: BlobStoreDAO's API refactoring + memory storage for blob's metadata + test contract for metadata should be ready. (Hopefully, we have a green build)

You can review the appended commits since the JAMES-4182 Fix compilation commit.

I propose to split the blob metadata storage for other implementations (S3, file, Cassandra, Postgres) into other PRs.

Also, should we fire a mailing list thread to draw the potential community's attention and feedback on this topic, if any?

chibenwa · 2026-04-08T09:34:30Z

Good with me!

Thanks for the help on this @quantranhong1999

Arsnael · 2026-04-08T10:20:43Z

Is this related? https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-2960/4/testReport/

quantranhong1999 · 2026-04-08T10:24:40Z

Is this related? https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-2960/4/testReport/

It seems YES. I will have a look tomorrow.

…nput stream size

cf S3 object's metadata key is case-insensitive (normalized to lowercase)

quantranhong1999 · 2026-04-09T09:50:19Z

Green build.

Let's wait for @jeantil review on this :-).

chibenwa · 2026-04-09T09:55:15Z

+1 OK with me

quantranhong1999 · 2026-04-10T09:59:10Z

I propose to split the blob metadata storage for other implementations (S3, file, Cassandra, Postgres) into other PRs.

See WIP: #3001

chibenwa changed the title ~~James 4182 api~~ JAMES-4182 Proposal for changes in BlobStoreDAO interface Mar 3, 2026

chibenwa requested a review from jeantil March 3, 2026 16:25

jeantil reviewed Mar 10, 2026

View reviewed changes

server/blob/blob-api/src/main/java/org/apache/james/blob/api/BlobStoreDAO.java Outdated Show resolved Hide resolved

chibenwa force-pushed the JAMES-4182-api branch from f813896 to afa20c1 Compare March 19, 2026 08:56

jeantil reviewed Mar 22, 2026

View reviewed changes

server/blob/blob-api/src/main/java/org/apache/james/blob/api/BlobStoreDAO.java Outdated Show resolved Hide resolved

jeantil reviewed Mar 23, 2026

View reviewed changes

This was referenced Mar 24, 2026

S3 Compression linagora/james-project#5515

Open

JAMES-4182 Proposal for changes in BlobStoreDAO interface linagora/james-project#5536

Open

Implement ZstdBlobStoreDAO linagora/james-project#5537

Open

chibenwa and others added 12 commits April 7, 2026 11:55

Remove unused ReactiveByteSource

8a43b7c

BlobStoreDAO: saveBlob

7cc2509

BlobStoreDAO: readBytesBlob

68c6f22

BlobStoreDAO: readBlobReactive

77f5a2f

BlobStoreDAO: readBlob

cbaaeb8

BlobStoreDAO: remove blob from method names

f3432ef

TODOs

3444feb

Fix existing contracts

88c77dd

Adapt metadata model following review comments

be67d46

Memory: store Blob metadata

3addef6

JAMES-4182 Fix compilation

3451c8c

quantranhong1999 added 3 commits April 8, 2026 11:54

JAMES-4182 Fix S3BlobStoreDAOTest::readBytesShouldFallbackToDefinedBu…

33826d5

…cketWhenFailingOnDefaultOne test

JAMES-4182 AESBlobStoreDAO should respect metadata information from u…

40e6383

…nderlying BlobStoreDAO

JAMES-4182 Adapt review suggestions

6110c88

quantranhong1999 force-pushed the JAMES-4182-api branch from afa20c1 to 6110c88 Compare April 8, 2026 08:20

quantranhong1999 marked this pull request as ready for review April 8, 2026 08:28

quantranhong1999 added 2 commits April 9, 2026 11:00

JAMES-4182 [FIX] InMemoryUploadRepository: do not eagerly calculate i…

2fd8fec

…nput stream size

JAMES-4182 BlobMetadataName should be case-insensitive

a3d5a9d

cf S3 object's metadata key is case-insensitive (normalized to lowercase)

quantranhong1999 pushed a commit to quantranhong1999/james-project that referenced this pull request Apr 10, 2026

[NO REVIEW] apache#2960

c4d1239

quantranhong1999 mentioned this pull request Apr 10, 2026

JAMES-4182 Implement blob metadata storage(s) #3001

Open

4 tasks


		Mono.from(testee.save(TEST_BUCKET_NAME, TEST_BLOB_ID, bytesBlob)).block();

		assertThat(Mono.from(testee.readBytes(TEST_BUCKET_NAME, TEST_BLOB_ID)).block().metadata().metadata())

Conversation

chibenwa commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeantil commented Mar 10, 2026

Uh oh!

jeantil left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jeantil Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chibenwa commented Mar 11, 2026

Uh oh!

jeantil commented Mar 12, 2026

Uh oh!

chibenwa commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeantil commented Mar 12, 2026

Uh oh!

chibenwa commented Mar 12, 2026

Uh oh!

chibenwa commented Mar 12, 2026

Uh oh!

chibenwa commented Mar 19, 2026

Uh oh!

Uh oh!

jeantil Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

quantranhong1999 commented Apr 8, 2026

Uh oh!

chibenwa commented Apr 8, 2026

Uh oh!

Arsnael commented Apr 8, 2026

Uh oh!

quantranhong1999 commented Apr 8, 2026

Uh oh!

quantranhong1999 commented Apr 9, 2026

Uh oh!

chibenwa commented Apr 9, 2026

Uh oh!

quantranhong1999 commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chibenwa commented Mar 3, 2026 •

edited

Loading

chibenwa commented Mar 12, 2026 •

edited

Loading