[GitHub] [james-project] chibenwa commented on pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

[GitBox <gi...@apache.org>]

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

   > Here is an initial implementation of a JPAMailStore, which will hopefully suffice to make the JPA app work fully with a database, a big relief for everyone that migrated from 2.3.x and was shocked to discover they are forced to use file repositories ;-)
   
   Thanks for tackling the issue!
   
   > The perRecipientHeaders field is currently Java-serialized into a byte array. I saw some other places migrated to using a string delimited by newlines, so that can be done here as well, but seems bad to re-implement it in every component... should be some main utility somewhere that does this in a standardised way, and all components including this one can just use it as a one-liner.
   
   I would support such a refactoring :+1: 
   
   > shouldPreserveDsnParameters fails due to the same issue as with jdbc - because the attributes are not Java-serializeable. See below.
   
   mailet-api comes with a serialization utility. See what other implementations are doing:
   
   https://github.com/apache/james-project/blob/8ba9871f61a0caa421061f7a8362354c758c8c8b/server/mailrepository/mailrepository-cassandra/src/main/java/org/apache/james/mailrepository/cassandra/CassandraMailRepositoryMailDaoV2.java#L278
   
   TLDR: attributes now come with a handy `toJson` method.
   
   Java serialization is hasardous and deserialization glitches can be exploited EG for arbitrary code execution. As a project we spent considerable amount of time and energy migrating away from java serialization.
   
   As such I would prefer newly contributed code to stay clear from this pitfall.
   
   > There are resource leak warnings in the console when running the tests.
   
   Harmless: mails created in tests are not well managed thus creating the warnings. We are decreasing the level of leak detection to avoid "spam". CF https://github.com/apache/james-project/pull/1170
   
   > Would be great if someone in the know can give it a review to see if everything is configured and used correctly.
   
   That is my plan!
   
   > One more thing - is there a tool and/or simple mailet configuration that migrates between different mail repository implementations? It would be useful to clearly document this and/or create a dedicated tool to ease migrations for existing users.
   
   Achievable via webadmin endpoint to reprocess emails. Reprocess eails from repo A into a mail processor that stores things in repoB. Feel free to experiment and document the process.
   
   Best regards,
   
   Benoit TELLIER
   
   


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

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


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