You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2022/08/31 21:53:04 UTC

[GitHub] [james-project] amichair opened a new pull request, #1176: JAMES-2656 - Add initial JPAMailRepository implementation

amichair opened a new pull request, #1176:
URL: https://github.com/apache/james-project/pull/1176

   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 ;-)
   
   This is an initial implementation that is hopefully good enough to be useful, but still has room for improvement. After merging others can help incrementally improve on it as necessary. Specifically:
   
   - It passes MailRepositoryContract as suggested in JAMES-2656, except for two disabled tests (which are both also disabled in JDBCMailRepository, so it's no worse than that):
     - mailRepositoriesShouldBeURLIsolated seems to fail only because the mock Mail has no lastUpdated and state fields set - I'm not sure if these fields are supposed to be set and this is just a bug in the test object itself, or if it's a valid scenario that should be handled (allow nulls in these fields). I currently included also the fix for the test, but awaiting feedback on what the proper solution should be. When adding the two fields in the test, it passes.
     - shouldPreserveDsnParameters fails due to the same issue as with jdbc - because the attributes are not Java-serializeable. See below.
   
   Future improvements:
   - The body is currently defined as a byte array, i.e. is held in memory. JPA supports streaming for a handful of databases. I suppose for those we can do something like what was done for JPAMailboxMessage/JPAStreamingMailboxMessage - have two subclasses, one with byte array and one with stream, and somehow configure which to use (I didn't see where that is configured).
   - 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.
   - The message attributes are currently Java-serialized into a byte array. The Mail interface requires attribute values to be serializeable, but apparently James violated this contract when using DSN parameters, which is why the corresponding test fails. This should probably be fixed in the DSN implementation, since it violates the contract. In any case, I saw some other component moved to json serialization for the attributes, so maybe we can get that to work here as well (I gave it a shot but ran into issues so dropped it for now). Here too, better to have a centralized utility that takes care of everything instead of re-implementing in every component.
   - There are resource leak warnings in the console when running the tests. Not sure where they come from, though I see them also when running tests for other repository types, so I don't know if this is really an issue or a false alarm, or how important or not it is at this point.
   - It would have been nice to have a separate table per repository, but as far as I can tell, JPA doesn't support dynamic table names that way, so all the repos are in one table, with a column (indexed) specifying the repo name. No biggie.
   - I haven't worked with Guice or JPA before, so maybe I missed something. Would be great if someone in the know can give it a review to see if everything is configured and used correctly.
   - There may be room for other performance improvements if needed.
   - In addition to the contract tests, I ran a few manual tests, and it seems ok. Would be great if someone has a more realistic scenario or production server to run it on as a fuzz test to cover edge cases, concurrency, load, etc.
   
   To summarise, although I detailed my thoughts and dilemmas regarding the implementation, the bottom line is that it's probably good enough for most scenarios and a good starting point for future contributions. It's the big first step that nobody took in a few years, and hopefully once merged it will be easier for other devs and users to improve upon.
   
   Your thoughts?
   


-- 
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


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

Posted by GitBox <gi...@apache.org>.
amichair commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1248604363

   Rebased again. I'm not sure what you mean about the prefix - I added it to the initial commit of the jpa repository implementation, do you want it on all the commits? or to squash all the related commits into one with this prefix? (excluding the commits with cleanup of other classes)


-- 
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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1236591333

   > (btw the serialization changes I didn't backport to jdbc repository so it won't break backwards compatibility with existing databases)
   
   Yes that's perfect we should avoid breaking changes.
   
   I'll have a look somewhere this week at the attribute stuff.
   
   Regards,
   
   Benoit


-- 
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


[GitHub] [james-project] amichair commented on a diff in pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

Posted by GitBox <gi...@apache.org>.
amichair commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960551426


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;

Review Comment:
   This assumption is also made in some of the other repository impls (again - this was based on the JDBC impl), I think the assumption is that the MailRepositoryUrl parsing already verifies the generic url syntax, and the repository impl only needs to deal with the details in the path part?



-- 
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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960586329


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>

Review Comment:
   >  Should I just delete this comment in all of them and leave this to the external documentation?
   
   Good proposal.
   
   Do it in a separate 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-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


[GitHub] [james-project] amichair commented on a diff in pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

Posted by GitBox <gi...@apache.org>.
amichair commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960873153


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,305 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.api.MailRepositoryUrl;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    private String repositoryName;
+
+    private final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {

Review Comment:
   I'll follow the project's style guidelines, whatever they may be.



##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,305 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.api.MailRepositoryUrl;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    private String repositoryName;
+
+    private final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String destination = configuration.getString("[@destinationURL]");
+        MailRepositoryUrl url = MailRepositoryUrl.from(destination); // also validates url and standardizes slashes
+        repositoryName = url.getPath().asString();
+        if (repositoryName.isEmpty()) {
+            throw new ConfigurationException(
+                "Malformed destinationURL - Must be of the format 'jpa://<repositoryName>'.  Was passed " + url);
+        }
+        LOGGER.debug("Parsed URL: repositoryName = '{}'", repositoryName);
+    }
+
+    /**
+     * Initialises the JPA repository.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Override
+    @PostConstruct
+    public void init() throws Exception {
+        LOGGER.debug("{}.initialize()", getClass().getName());
+        list();
+    }
+
+    @Override
+    public MailKey store(Mail mail) throws MessagingException {
+        MailKey key = MailKey.forMail(mail);
+        EntityManager em = em();
+        try {
+            JPAMail m = new JPAMail();

Review Comment:
   I'll follow the project's style guidelines, whatever they may be.



-- 
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


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

Posted by GitBox <gi...@apache.org>.
amichair commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1246584936

   I added a couple of optimizations in a separate commit (removeAll direct query and json fields not lazy), then CI build failed for unrelated reasons (cassandra something), then rebased on current master.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
amichair commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1308238326

   Were there any issues found with this implementation? Is it ready for release?


-- 
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


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

Posted by GitBox <gi...@apache.org>.
amichair commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1248927750

   oh it's all squashed into one, I intended to do the jpamailrepository separately in one commit and the file/jdbc/jpaurl cleanup in separate commits as they are. Too late I suppose.


-- 
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


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

Posted by 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


[GitHub] [james-project] amichair commented on a diff in pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

Posted by GitBox <gi...@apache.org>.
amichair commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960759422


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;
+        repositoryName = url.substring(start);
+        if (repositoryName.isEmpty()) {
+            String exceptionBuffer = "Malformed destinationURL - Must be of the format '" + "jpa://<repositoryName>'.  Was passed " + url;
+            throw new ConfigurationException(exceptionBuffer);
+        }
+        LOGGER.debug("Parsed URL: repositoryName = '{}'", repositoryName);
+    }
+
+    /**
+     * Initialises the JPA repository.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Override
+    @PostConstruct
+    public void init() throws Exception {
+        LOGGER.debug("{}.initialize()", getClass().getName());
+        list();
+    }
+
+    @Override
+    public MailKey store(Mail mail) throws MessagingException {
+        MailKey key = MailKey.forMail(mail);
+        EntityManager em = em();
+        try {
+            JPAMail m = new JPAMail();
+            m.setRepositoryName(repositoryName);
+            m.setMessageName(mail.getName());
+            m.setMessageState(mail.getState());
+            m.setErrorMessage(mail.getErrorMessage());
+            if (!mail.getMaybeSender().isNullSender()) {
+                m.setSender(mail.getMaybeSender().get().toString());
+            }
+            StringBuilder recipients = new StringBuilder();
+            for (Iterator<MailAddress> i = mail.getRecipients().iterator(); i.hasNext();) {
+                recipients.append(i.next().toString());
+                if (i.hasNext()) {
+                    recipients.append("\r\n");
+                }
+            }
+            m.setRecipients(recipients.toString());
+            m.setRemoteHost(mail.getRemoteHost());
+            m.setRemoteAddr(mail.getRemoteAddr());
+            if (!mail.getPerRecipientSpecificHeaders().getHeadersByRecipient().isEmpty()) {
+                byte[] bytes = SerializationUtils.serialize(mail.getPerRecipientSpecificHeaders());
+                m.setPerRecipientHeaders(bytes);
+            }
+            m.setLastUpdated(new Timestamp(mail.getLastUpdated().getTime()));
+            m.setMessageBody(getBody(mail));
+            if (mail.hasAttributes()) {
+                m.setMessageAttributes(serializeAttributes(mail));
+            }
+            EntityTransaction transaction = em.getTransaction();
+            transaction.begin();
+            m = em.merge(m);
+            transaction.commit();
+            return key;
+        } catch (MessagingException e) {
+            LOGGER.error("Exception caught while storing mail {}", key, e);
+            throw e;
+        } catch (Exception e) {
+            LOGGER.error("Exception caught while storing mail {}", key, e);
+            throw new MessagingException("Exception caught while storing mail " + key, e);
+        } finally {
+            EntityManagerUtils.safelyClose(em);
+        }
+    }
+
+    private byte[] getBody(Mail mail) throws MessagingException, IOException {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();

Review Comment:
   I meant the module's pom. Maybe I'm being stingy. How about if it's initialized to capacity mail.getMessageSize()? (should have done that anyway)



-- 
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


[GitHub] [james-project] amichair commented on a diff in pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

Posted by GitBox <gi...@apache.org>.
amichair commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960588655


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;
+        repositoryName = url.substring(start);
+        if (repositoryName.isEmpty()) {
+            String exceptionBuffer = "Malformed destinationURL - Must be of the format '" + "jpa://<repositoryName>'.  Was passed " + url;
+            throw new ConfigurationException(exceptionBuffer);
+        }
+        LOGGER.debug("Parsed URL: repositoryName = '{}'", repositoryName);
+    }
+
+    /**
+     * Initialises the JPA repository.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Override
+    @PostConstruct
+    public void init() throws Exception {
+        LOGGER.debug("{}.initialize()", getClass().getName());
+        list();
+    }
+
+    @Override
+    public MailKey store(Mail mail) throws MessagingException {
+        MailKey key = MailKey.forMail(mail);
+        EntityManager em = em();
+        try {
+            JPAMail m = new JPAMail();
+            m.setRepositoryName(repositoryName);
+            m.setMessageName(mail.getName());
+            m.setMessageState(mail.getState());
+            m.setErrorMessage(mail.getErrorMessage());
+            if (!mail.getMaybeSender().isNullSender()) {
+                m.setSender(mail.getMaybeSender().get().toString());
+            }
+            StringBuilder recipients = new StringBuilder();
+            for (Iterator<MailAddress> i = mail.getRecipients().iterator(); i.hasNext();) {

Review Comment:
   also copied from JDBC repo, I'll try with streams (maybe it will look better), lmk if u like.



-- 
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


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

Posted by GitBox <gi...@apache.org>.
amichair commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1243048825

   oh thanks!
   I thought we're going for a more generalized serialization solution rather than have this component implement the serialization on its own like other components do... but I suppose as long as the db format stays fixed it can be refactored later. Better ship it now than wait for a redesign :-)
   Although, if we're doing it this way, might as well also replace the recipients headers serialization I made with JSON, no need to have two different serialization schemes here. I'll try to do it tomorrow, if you concur.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960634438


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;

Review Comment:
   You are likely right, though to make such an assumption it requires quite a good understanding of the overall system?
   
   Does an extra assertion hurts?
   
   It would get rid of that temporal coupling...



-- 
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


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

Posted by GitBox <gi...@apache.org>.
amichair commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1326002535

   Is there an ETA on the release that includes the JPAMailRepository?


-- 
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


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

Posted by GitBox <gi...@apache.org>.
amichair commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1241762059

   ok, so just waiting for @chibenwa's answer regarding the attributes serialization. Anything else u need me to do?


-- 
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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1242927626

   Here is a little commit for JSON serialisation of mail attributes so that we get rid of Java serialisation.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1248803848

   We quite like having the JIRA in all commit messages.
   
   Anyway here I think it makes sense to squash.
   
   Thanks a lot for the great contribution. If there is some things you had in James 2.3 and need here, don't hesitate to contribute them.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1247907468

   Can I ask you to rebase one more time and put the JIRA in the commit messages as a prefix?
   
   `JAMES-2656 My message...`
   
   Also https://ci-builds.apache.org/job/james/job/ApacheJames/job/PR-1176/9/testReport/junit/org.apache.james.mailrepository.jpa/JPAMailRepositoryTest/storingAndRemovingMessagesConcurrentlyShouldLeadToConsistentResult_3_/
   
   ```
   org.apache.james.mailrepository.jpa.JPAMailRepositoryTest.storingAndRemovingMessagesConcurrentlyShouldLeadToConsistentResult[3]
   
   java.lang.RuntimeException: com.github.fge.lambdas.ThrownByLambdaException: javax.mail.MessagingException: Exception while removing message(s): Unable to obtain an object lock on "DELETE FROM APP.JAMES_MAIL_STORE WHERE EXISTS (SELECT 1 FROM APP.JAMES_MAIL_STORE t0 WHERE (t0.REPOSITORY_NAME = ? AND t0.MESSAGE_NAME IN (?)) AND (t0.MESSAGE_NAME = APP.JAMES_MAIL_STORE.MESSAGE_NAME AND t0.REPOSITORY_NAME = APP.JAMES_MAIL_STORE.REPOSITORY_NAME)) [java.lang.String]".;
     nested exception is:
    <openjpa-3.2.0-r6f721f6 nonfatal store error> org.apache.openjpa.persistence.OptimisticLockException: Unable to obtain an object lock on "DELETE FROM APP.JAMES_MAIL_STORE WHERE EXISTS (SELECT 1 FROM APP.JAMES_MAIL_STORE t0 WHERE (t0.REPOSITORY_NAME = ? AND t0.MESSAGE_NAME IN (?)) AND (t0.MESSAGE_NAME = APP.JAMES_MAIL_STORE.MESSAGE_NAME AND t0.REPOSITORY_NAME = APP.JAMES_MAIL_STORE.REPOSITORY_NAME)) [java.lang.String]".
   FailedObject: DELETE FROM APP.JAMES_MAIL_STORE WHERE EXISTS (SELECT 1 FROM APP.JAMES_MAIL_STORE t0 WHERE (t0.REPOSITORY_NAME = ? AND t0.MESSAGE_NAME IN (?)) AND (t0.MESSAGE_NAME = APP.JAMES_MAIL_STORE.MESSAGE_NAME AND t0.REPOSITORY_NAME = APP.JAMES_MAIL_STORE.REPOSITORY_NAME)) [java.lang.String]
   ```
   
   To my understanding the test is too violent... Maybe we could @Disable it for now?


-- 
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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960817541


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,305 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.api.MailRepositoryUrl;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    private String repositoryName;
+
+    private final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String destination = configuration.getString("[@destinationURL]");
+        MailRepositoryUrl url = MailRepositoryUrl.from(destination); // also validates url and standardizes slashes
+        repositoryName = url.getPath().asString();
+        if (repositoryName.isEmpty()) {
+            throw new ConfigurationException(
+                "Malformed destinationURL - Must be of the format 'jpa://<repositoryName>'.  Was passed " + url);
+        }
+        LOGGER.debug("Parsed URL: repositoryName = '{}'", repositoryName);
+    }
+
+    /**
+     * Initialises the JPA repository.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Override
+    @PostConstruct
+    public void init() throws Exception {
+        LOGGER.debug("{}.initialize()", getClass().getName());
+        list();
+    }
+
+    @Override
+    public MailKey store(Mail mail) throws MessagingException {
+        MailKey key = MailKey.forMail(mail);
+        EntityManager em = em();
+        try {
+            JPAMail m = new JPAMail();

Review Comment:
   ```suggestion
               JPAMail jpaMail = new JPAMail();
   ```



##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,305 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.api.MailRepositoryUrl;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    private String repositoryName;
+
+    private final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {

Review Comment:
   ```suggestion
       protected EntityManager entityManager() {
   ```
   
   (we've got auto completes in IDE, let's leverage self speaking method names?)



-- 
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


[GitHub] [james-project] amichair commented on a diff in pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

Posted by GitBox <gi...@apache.org>.
amichair commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960701714


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;

Review Comment:
   Fixed in both jdbc and jpa repos. I don't care much for the double validation, but I love the code simplification that ensued ;-)



-- 
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


[GitHub] [james-project] amichair commented on a diff in pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

Posted by GitBox <gi...@apache.org>.
amichair commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960539087


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>

Review Comment:
   This was copied form the JDBC repo (I loosely based the new impl on it), and is also in the file repo class. Should I just delete this comment in all of them and leave this to the external documentation? Do we put this kind of thing in every component's javadoc?



##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;

Review Comment:
   ok



-- 
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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960632628


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;
+        repositoryName = url.substring(start);
+        if (repositoryName.isEmpty()) {
+            String exceptionBuffer = "Malformed destinationURL - Must be of the format '" + "jpa://<repositoryName>'.  Was passed " + url;
+            throw new ConfigurationException(exceptionBuffer);
+        }
+        LOGGER.debug("Parsed URL: repositoryName = '{}'", repositoryName);
+    }
+
+    /**
+     * Initialises the JPA repository.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Override
+    @PostConstruct
+    public void init() throws Exception {
+        LOGGER.debug("{}.initialize()", getClass().getName());
+        list();
+    }
+
+    @Override
+    public MailKey store(Mail mail) throws MessagingException {
+        MailKey key = MailKey.forMail(mail);
+        EntityManager em = em();
+        try {
+            JPAMail m = new JPAMail();
+            m.setRepositoryName(repositoryName);
+            m.setMessageName(mail.getName());
+            m.setMessageState(mail.getState());
+            m.setErrorMessage(mail.getErrorMessage());
+            if (!mail.getMaybeSender().isNullSender()) {
+                m.setSender(mail.getMaybeSender().get().toString());
+            }
+            StringBuilder recipients = new StringBuilder();
+            for (Iterator<MailAddress> i = mail.getRecipients().iterator(); i.hasNext();) {
+                recipients.append(i.next().toString());
+                if (i.hasNext()) {
+                    recipients.append("\r\n");
+                }
+            }
+            m.setRecipients(recipients.toString());
+            m.setRemoteHost(mail.getRemoteHost());
+            m.setRemoteAddr(mail.getRemoteAddr());
+            if (!mail.getPerRecipientSpecificHeaders().getHeadersByRecipient().isEmpty()) {
+                byte[] bytes = SerializationUtils.serialize(mail.getPerRecipientSpecificHeaders());
+                m.setPerRecipientHeaders(bytes);
+            }
+            m.setLastUpdated(new Timestamp(mail.getLastUpdated().getTime()));
+            m.setMessageBody(getBody(mail));
+            if (mail.hasAttributes()) {
+                m.setMessageAttributes(serializeAttributes(mail));
+            }
+            EntityTransaction transaction = em.getTransaction();
+            transaction.begin();
+            m = em.merge(m);
+            transaction.commit();
+            return key;
+        } catch (MessagingException e) {
+            LOGGER.error("Exception caught while storing mail {}", key, e);
+            throw e;
+        } catch (Exception e) {
+            LOGGER.error("Exception caught while storing mail {}", key, e);
+            throw new MessagingException("Exception caught while storing mail " + key, e);
+        } finally {
+            EntityManagerUtils.safelyClose(em);
+        }
+    }
+
+    private byte[] getBody(Mail mail) throws MessagingException, IOException {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();

Review Comment:
   commons-io is already a james dependency :-)
   
   Mails can be quite big ~20MB so IMO it makes sense to try to reduce the footprint, especially if easy solutions exists.



-- 
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


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

Posted by GitBox <gi...@apache.org>.
amichair commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1237185834

   Great, thanks!
   If it's a generic enough serialization solution, it could be applied to the perRecipientHeaders as well. If not, see my comment above about it, if you have any input on how to improve my arbitrary (currently implemented) solution :-)


-- 
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


[GitHub] [james-project] amichair commented on a diff in pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

Posted by GitBox <gi...@apache.org>.
amichair commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960851314


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;
+        repositoryName = url.substring(start);
+        if (repositoryName.isEmpty()) {
+            String exceptionBuffer = "Malformed destinationURL - Must be of the format '" + "jpa://<repositoryName>'.  Was passed " + url;
+            throw new ConfigurationException(exceptionBuffer);
+        }
+        LOGGER.debug("Parsed URL: repositoryName = '{}'", repositoryName);
+    }
+
+    /**
+     * Initialises the JPA repository.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Override
+    @PostConstruct
+    public void init() throws Exception {
+        LOGGER.debug("{}.initialize()", getClass().getName());
+        list();
+    }
+
+    @Override
+    public MailKey store(Mail mail) throws MessagingException {
+        MailKey key = MailKey.forMail(mail);
+        EntityManager em = em();
+        try {
+            JPAMail m = new JPAMail();
+            m.setRepositoryName(repositoryName);
+            m.setMessageName(mail.getName());
+            m.setMessageState(mail.getState());
+            m.setErrorMessage(mail.getErrorMessage());
+            if (!mail.getMaybeSender().isNullSender()) {
+                m.setSender(mail.getMaybeSender().get().toString());
+            }
+            StringBuilder recipients = new StringBuilder();
+            for (Iterator<MailAddress> i = mail.getRecipients().iterator(); i.hasNext();) {
+                recipients.append(i.next().toString());
+                if (i.hasNext()) {
+                    recipients.append("\r\n");
+                }
+            }
+            m.setRecipients(recipients.toString());
+            m.setRemoteHost(mail.getRemoteHost());
+            m.setRemoteAddr(mail.getRemoteAddr());
+            if (!mail.getPerRecipientSpecificHeaders().getHeadersByRecipient().isEmpty()) {
+                byte[] bytes = SerializationUtils.serialize(mail.getPerRecipientSpecificHeaders());

Review Comment:
   Implemented custom serialization of the recipients headers into a string. To avoid escaping issues, used only CR/CRCR as delimiters. What do u think of the encoding? simple and future proof enough? should the recipient in the first line appear as-is, or as a header (e.g. "recipient: address" to make it easier to parse all lines as headers in the future? Any other ideas for different/better encoding?



-- 
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


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

Posted by GitBox <gi...@apache.org>.
amichair commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1233736251

   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.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
Arsnael commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1241767382

   > ok, so just waiting for @chibenwa's answer regarding the attributes serialization. Anything else u need me to do?
   
   The serialization seems the last piece of the puzzle so except for that, I think we are good :)


-- 
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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1248927825

   > btw is there an ETA for this release?
   
   The main issue that remains to be fixed is IMO https://github.com/apache/james-project/pull/1185
   
   Once merged I'd like to go for 1-2 weeks on internal production plateform and fully feel confident for a 3.8.0 release.
   
   So ETA ~1month. Is this OK to you?


-- 
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


[GitHub] [james-project] amichair commented on a diff in pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

Posted by GitBox <gi...@apache.org>.
amichair commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960572969


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;
+        repositoryName = url.substring(start);
+        if (repositoryName.isEmpty()) {
+            String exceptionBuffer = "Malformed destinationURL - Must be of the format '" + "jpa://<repositoryName>'.  Was passed " + url;
+            throw new ConfigurationException(exceptionBuffer);
+        }
+        LOGGER.debug("Parsed URL: repositoryName = '{}'", repositoryName);
+    }
+
+    /**
+     * Initialises the JPA repository.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Override
+    @PostConstruct
+    public void init() throws Exception {
+        LOGGER.debug("{}.initialize()", getClass().getName());
+        list();
+    }
+
+    @Override
+    public MailKey store(Mail mail) throws MessagingException {
+        MailKey key = MailKey.forMail(mail);
+        EntityManager em = em();
+        try {
+            JPAMail m = new JPAMail();
+            m.setRepositoryName(repositoryName);
+            m.setMessageName(mail.getName());
+            m.setMessageState(mail.getState());
+            m.setErrorMessage(mail.getErrorMessage());
+            if (!mail.getMaybeSender().isNullSender()) {
+                m.setSender(mail.getMaybeSender().get().toString());
+            }
+            StringBuilder recipients = new StringBuilder();
+            for (Iterator<MailAddress> i = mail.getRecipients().iterator(); i.hasNext();) {
+                recipients.append(i.next().toString());
+                if (i.hasNext()) {
+                    recipients.append("\r\n");
+                }
+            }
+            m.setRecipients(recipients.toString());
+            m.setRemoteHost(mail.getRemoteHost());
+            m.setRemoteAddr(mail.getRemoteAddr());
+            if (!mail.getPerRecipientSpecificHeaders().getHeadersByRecipient().isEmpty()) {
+                byte[] bytes = SerializationUtils.serialize(mail.getPerRecipientSpecificHeaders());
+                m.setPerRecipientHeaders(bytes);
+            }
+            m.setLastUpdated(new Timestamp(mail.getLastUpdated().getTime()));
+            m.setMessageBody(getBody(mail));
+            if (mail.hasAttributes()) {
+                m.setMessageAttributes(serializeAttributes(mail));
+            }
+            EntityTransaction transaction = em.getTransaction();
+            transaction.begin();
+            m = em.merge(m);
+            transaction.commit();
+            return key;
+        } catch (MessagingException e) {
+            LOGGER.error("Exception caught while storing mail {}", key, e);
+            throw e;
+        } catch (Exception e) {
+            LOGGER.error("Exception caught while storing mail {}", key, e);
+            throw new MessagingException("Exception caught while storing mail " + key, e);
+        } finally {
+            EntityManagerUtils.safelyClose(em);
+        }
+    }
+
+    private byte[] getBody(Mail mail) throws MessagingException, IOException {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();

Review Comment:
   I doubt you'll ever notice the difference, and personally I don't think it's worth adding a dependency just for this, but I'll change if u want :-)



-- 
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


[GitHub] [james-project] amichair commented on a diff in pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

Posted by GitBox <gi...@apache.org>.
amichair commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960568938


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;
+        repositoryName = url.substring(start);
+        if (repositoryName.isEmpty()) {
+            String exceptionBuffer = "Malformed destinationURL - Must be of the format '" + "jpa://<repositoryName>'.  Was passed " + url;

Review Comment:
   inlined and reformatted



-- 
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


[GitHub] [james-project] amichair commented on a diff in pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

Posted by GitBox <gi...@apache.org>.
amichair commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960574596


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/MimeMessageJPASource.java:
##########
@@ -0,0 +1,35 @@
+package org.apache.james.mailrepository.jpa;

Review Comment:
   My bad.



-- 
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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960446162


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;
+        repositoryName = url.substring(start);
+        if (repositoryName.isEmpty()) {
+            String exceptionBuffer = "Malformed destinationURL - Must be of the format '" + "jpa://<repositoryName>'.  Was passed " + url;

Review Comment:
   ```suggestion
               String exceptionBuffer = "Malformed destinationURL - Must be of the format 'jpa://<repositoryName>'.  Was passed " + url;
   ```



##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;

Review Comment:
   if url does not contain `://` will return `-1` and start would be considered 2. 
   
   This can conduct to invalid behavior.
   
   We likely should verify that `://` is contained in the first place.



##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/MimeMessageJPASource.java:
##########
@@ -0,0 +1,35 @@
+package org.apache.james.mailrepository.jpa;

Review Comment:
   We are missing the compulsory ASF V2 header license.



##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/MimeMessageJPASource.java:
##########
@@ -0,0 +1,35 @@
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+import org.apache.james.server.core.MimeMessageSource;
+
+public class MimeMessageJPASource implements MimeMessageSource {
+
+    JPAMailRepository jpaMailRepository;
+    String key;
+    byte[] body;

Review Comment:
   private final ?



##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;
+        repositoryName = url.substring(start);
+        if (repositoryName.isEmpty()) {
+            String exceptionBuffer = "Malformed destinationURL - Must be of the format '" + "jpa://<repositoryName>'.  Was passed " + url;
+            throw new ConfigurationException(exceptionBuffer);
+        }
+        LOGGER.debug("Parsed URL: repositoryName = '{}'", repositoryName);
+    }
+
+    /**
+     * Initialises the JPA repository.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Override
+    @PostConstruct
+    public void init() throws Exception {
+        LOGGER.debug("{}.initialize()", getClass().getName());
+        list();
+    }
+
+    @Override
+    public MailKey store(Mail mail) throws MessagingException {
+        MailKey key = MailKey.forMail(mail);
+        EntityManager em = em();
+        try {
+            JPAMail m = new JPAMail();
+            m.setRepositoryName(repositoryName);
+            m.setMessageName(mail.getName());
+            m.setMessageState(mail.getState());
+            m.setErrorMessage(mail.getErrorMessage());
+            if (!mail.getMaybeSender().isNullSender()) {
+                m.setSender(mail.getMaybeSender().get().toString());
+            }
+            StringBuilder recipients = new StringBuilder();
+            for (Iterator<MailAddress> i = mail.getRecipients().iterator(); i.hasNext();) {
+                recipients.append(i.next().toString());
+                if (i.hasNext()) {
+                    recipients.append("\r\n");
+                }
+            }
+            m.setRecipients(recipients.toString());
+            m.setRemoteHost(mail.getRemoteHost());
+            m.setRemoteAddr(mail.getRemoteAddr());
+            if (!mail.getPerRecipientSpecificHeaders().getHeadersByRecipient().isEmpty()) {
+                byte[] bytes = SerializationUtils.serialize(mail.getPerRecipientSpecificHeaders());

Review Comment:
   As stated before we should avoid relying on java serialization



##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;

Review Comment:
   idem private



##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;

Review Comment:
   This field should be private



##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>

Review Comment:
   Outdated. now it is mailetcontainer.xml
   
   ```
   
   <mailrepositorystore>
       <defaultProtocol>jpa</defaultProtocol>
       <mailrepositories>
           <mailrepository class="org.apache.james.mailrepository.jpa.JPAMailRepository">
               <protocols>
                   <protocol>jpa</protocol>
               </protocols>
           </mailrepository>
       </mailrepositories>
   </mailrepositorystore>
   ```



##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;
+        repositoryName = url.substring(start);
+        if (repositoryName.isEmpty()) {
+            String exceptionBuffer = "Malformed destinationURL - Must be of the format '" + "jpa://<repositoryName>'.  Was passed " + url;
+            throw new ConfigurationException(exceptionBuffer);
+        }
+        LOGGER.debug("Parsed URL: repositoryName = '{}'", repositoryName);
+    }
+
+    /**
+     * Initialises the JPA repository.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Override
+    @PostConstruct
+    public void init() throws Exception {
+        LOGGER.debug("{}.initialize()", getClass().getName());
+        list();
+    }
+
+    @Override
+    public MailKey store(Mail mail) throws MessagingException {
+        MailKey key = MailKey.forMail(mail);
+        EntityManager em = em();
+        try {
+            JPAMail m = new JPAMail();
+            m.setRepositoryName(repositoryName);
+            m.setMessageName(mail.getName());
+            m.setMessageState(mail.getState());
+            m.setErrorMessage(mail.getErrorMessage());
+            if (!mail.getMaybeSender().isNullSender()) {
+                m.setSender(mail.getMaybeSender().get().toString());
+            }
+            StringBuilder recipients = new StringBuilder();
+            for (Iterator<MailAddress> i = mail.getRecipients().iterator(); i.hasNext();) {

Review Comment:
   `for(MailAddress rcpt: mail.getRecipient())` so that we don't need to manage indexes?



##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;
+        repositoryName = url.substring(start);
+        if (repositoryName.isEmpty()) {
+            String exceptionBuffer = "Malformed destinationURL - Must be of the format '" + "jpa://<repositoryName>'.  Was passed " + url;
+            throw new ConfigurationException(exceptionBuffer);
+        }
+        LOGGER.debug("Parsed URL: repositoryName = '{}'", repositoryName);
+    }
+
+    /**
+     * Initialises the JPA repository.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Override
+    @PostConstruct
+    public void init() throws Exception {
+        LOGGER.debug("{}.initialize()", getClass().getName());
+        list();
+    }
+
+    @Override
+    public MailKey store(Mail mail) throws MessagingException {
+        MailKey key = MailKey.forMail(mail);
+        EntityManager em = em();
+        try {
+            JPAMail m = new JPAMail();
+            m.setRepositoryName(repositoryName);
+            m.setMessageName(mail.getName());
+            m.setMessageState(mail.getState());
+            m.setErrorMessage(mail.getErrorMessage());
+            if (!mail.getMaybeSender().isNullSender()) {
+                m.setSender(mail.getMaybeSender().get().toString());
+            }
+            StringBuilder recipients = new StringBuilder();
+            for (Iterator<MailAddress> i = mail.getRecipients().iterator(); i.hasNext();) {
+                recipients.append(i.next().toString());
+                if (i.hasNext()) {
+                    recipients.append("\r\n");
+                }
+            }
+            m.setRecipients(recipients.toString());
+            m.setRemoteHost(mail.getRemoteHost());
+            m.setRemoteAddr(mail.getRemoteAddr());
+            if (!mail.getPerRecipientSpecificHeaders().getHeadersByRecipient().isEmpty()) {
+                byte[] bytes = SerializationUtils.serialize(mail.getPerRecipientSpecificHeaders());
+                m.setPerRecipientHeaders(bytes);
+            }
+            m.setLastUpdated(new Timestamp(mail.getLastUpdated().getTime()));
+            m.setMessageBody(getBody(mail));
+            if (mail.hasAttributes()) {
+                m.setMessageAttributes(serializeAttributes(mail));
+            }
+            EntityTransaction transaction = em.getTransaction();
+            transaction.begin();
+            m = em.merge(m);
+            transaction.commit();
+            return key;
+        } catch (MessagingException e) {
+            LOGGER.error("Exception caught while storing mail {}", key, e);
+            throw e;
+        } catch (Exception e) {
+            LOGGER.error("Exception caught while storing mail {}", key, e);
+            throw new MessagingException("Exception caught while storing mail " + key, e);
+        } finally {
+            EntityManagerUtils.safelyClose(em);
+        }
+    }
+
+    private byte[] getBody(Mail mail) throws MessagingException, IOException {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        if (mail instanceof MimeMessageWrapper) {
+            // we need to force the loading of the message from the
+            // stream as we want to override the old message
+            ((MimeMessageWrapper) mail).loadMessage();
+            ((MimeMessageWrapper) mail).writeTo(out, out, null, true);
+        } else {
+            mail.getMessage().writeTo(out);
+        }
+        return out.toByteArray();
+    }
+
+    private byte[] serializeAttributes(Mail mail) {
+        Map<String, Object> attributes = mail.attributes().collect(Collectors.toMap(
+            attribute -> attribute.getName().asString(),
+            attribute -> attribute.getValue().value()));
+        return SerializationUtils.serialize((Serializable)attributes);

Review Comment:
   Idem lets avoid java serialization...



##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;
+        repositoryName = url.substring(start);
+        if (repositoryName.isEmpty()) {
+            String exceptionBuffer = "Malformed destinationURL - Must be of the format '" + "jpa://<repositoryName>'.  Was passed " + url;
+            throw new ConfigurationException(exceptionBuffer);
+        }
+        LOGGER.debug("Parsed URL: repositoryName = '{}'", repositoryName);
+    }
+
+    /**
+     * Initialises the JPA repository.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Override
+    @PostConstruct
+    public void init() throws Exception {
+        LOGGER.debug("{}.initialize()", getClass().getName());
+        list();
+    }
+
+    @Override
+    public MailKey store(Mail mail) throws MessagingException {
+        MailKey key = MailKey.forMail(mail);
+        EntityManager em = em();
+        try {
+            JPAMail m = new JPAMail();
+            m.setRepositoryName(repositoryName);
+            m.setMessageName(mail.getName());
+            m.setMessageState(mail.getState());
+            m.setErrorMessage(mail.getErrorMessage());
+            if (!mail.getMaybeSender().isNullSender()) {
+                m.setSender(mail.getMaybeSender().get().toString());
+            }
+            StringBuilder recipients = new StringBuilder();
+            for (Iterator<MailAddress> i = mail.getRecipients().iterator(); i.hasNext();) {
+                recipients.append(i.next().toString());
+                if (i.hasNext()) {
+                    recipients.append("\r\n");
+                }
+            }
+            m.setRecipients(recipients.toString());
+            m.setRemoteHost(mail.getRemoteHost());
+            m.setRemoteAddr(mail.getRemoteAddr());
+            if (!mail.getPerRecipientSpecificHeaders().getHeadersByRecipient().isEmpty()) {
+                byte[] bytes = SerializationUtils.serialize(mail.getPerRecipientSpecificHeaders());
+                m.setPerRecipientHeaders(bytes);
+            }
+            m.setLastUpdated(new Timestamp(mail.getLastUpdated().getTime()));
+            m.setMessageBody(getBody(mail));
+            if (mail.hasAttributes()) {
+                m.setMessageAttributes(serializeAttributes(mail));
+            }
+            EntityTransaction transaction = em.getTransaction();
+            transaction.begin();
+            m = em.merge(m);
+            transaction.commit();
+            return key;
+        } catch (MessagingException e) {
+            LOGGER.error("Exception caught while storing mail {}", key, e);
+            throw e;
+        } catch (Exception e) {
+            LOGGER.error("Exception caught while storing mail {}", key, e);
+            throw new MessagingException("Exception caught while storing mail " + key, e);
+        } finally {
+            EntityManagerUtils.safelyClose(em);
+        }
+    }
+
+    private byte[] getBody(Mail mail) throws MessagingException, IOException {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();

Review Comment:
   Use a memory friendlier `UsynchronizedByteArrayOutputStream` from our firends at `commons-io` ?



-- 
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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r968319751


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -204,28 +204,45 @@ private String serializePerRecipientHeaders(PerRecipientHeaders perRecipientHead
         if (map.isEmpty()) {
             return null;
         }
-        StringBuilder data = new StringBuilder(map.size() * 1024);
+        ObjectNode node = JsonNodeFactory.instance.objectNode();
         for (Map.Entry<MailAddress, Collection<PerRecipientHeaders.Header>> entry : map.entrySet()) {
-            data.append(entry.getKey().asString()).append('\n');
-            entry.getValue().forEach(header -> data.append(header.asString()).append('\n'));
-            data.append('\n');
+            String recipient = entry.getKey().asString();
+            ObjectNode headers = node.putObject(recipient);
+            entry.getValue().forEach(header -> headers.put(header.getName(), header.getValue()));
         }
-        return data.toString();
+        return node.toString();
     }
 
-    private PerRecipientHeaders deserializePerRecipientHeaders(String data) throws AddressException {
+    private PerRecipientHeaders deserializePerRecipientHeaders(String data) {
         if (data == null || data.isEmpty()) {
             return null;
         }
         PerRecipientHeaders perRecipientHeaders = new PerRecipientHeaders();
-        for (String entry : data.split("\n\n")) {
-            String[] lines = entry.split("\n");
-            MailAddress address = new MailAddress(lines[0]);
-            for (int i = 1; i < lines.length; i++) {
-                perRecipientHeaders.addHeaderForRecipient(PerRecipientHeaders.Header.fromString(lines[i]), address);
+        try {
+            JsonNode node = OBJECT_MAPPER.readTree(data);
+            if (node instanceof ObjectNode) {
+                ObjectNode objectNode = (ObjectNode) node;
+                Iterators.toStream(objectNode.fields()).forEach(recipientEntry -> {
+                    try {
+                        MailAddress recipient = new MailAddress(recipientEntry.getKey());
+                        Iterators.toStream(recipientEntry.getValue().fields()).forEach(
+                            headerEntry -> {
+                                PerRecipientHeaders.Header header = PerRecipientHeaders.Header.builder()
+                                    .name(headerEntry.getKey())
+                                    .value(headerEntry.getValue().textValue())
+                                    .build();
+                                perRecipientHeaders.addHeaderForRecipient(header, recipient);
+                            });
+                    } catch (AddressException ae) {
+                        throw new IllegalArgumentException("invalid recipient address", ae);
+                    }
+                });

Review Comment:
   It starts making sense to extract that recipientEntry lambda to a named method?



-- 
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


[GitHub] [james-project] amichair commented on a diff in pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

Posted by GitBox <gi...@apache.org>.
amichair commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960575289


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/MimeMessageJPASource.java:
##########
@@ -0,0 +1,35 @@
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+import org.apache.james.server.core.MimeMessageSource;
+
+public class MimeMessageJPASource implements MimeMessageSource {
+
+    JPAMailRepository jpaMailRepository;
+    String key;
+    byte[] body;

Review Comment:
   ok



-- 
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


[GitHub] [james-project] amichair commented on a diff in pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

Posted by GitBox <gi...@apache.org>.
amichair commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960851314


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;
+        repositoryName = url.substring(start);
+        if (repositoryName.isEmpty()) {
+            String exceptionBuffer = "Malformed destinationURL - Must be of the format '" + "jpa://<repositoryName>'.  Was passed " + url;
+            throw new ConfigurationException(exceptionBuffer);
+        }
+        LOGGER.debug("Parsed URL: repositoryName = '{}'", repositoryName);
+    }
+
+    /**
+     * Initialises the JPA repository.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Override
+    @PostConstruct
+    public void init() throws Exception {
+        LOGGER.debug("{}.initialize()", getClass().getName());
+        list();
+    }
+
+    @Override
+    public MailKey store(Mail mail) throws MessagingException {
+        MailKey key = MailKey.forMail(mail);
+        EntityManager em = em();
+        try {
+            JPAMail m = new JPAMail();
+            m.setRepositoryName(repositoryName);
+            m.setMessageName(mail.getName());
+            m.setMessageState(mail.getState());
+            m.setErrorMessage(mail.getErrorMessage());
+            if (!mail.getMaybeSender().isNullSender()) {
+                m.setSender(mail.getMaybeSender().get().toString());
+            }
+            StringBuilder recipients = new StringBuilder();
+            for (Iterator<MailAddress> i = mail.getRecipients().iterator(); i.hasNext();) {
+                recipients.append(i.next().toString());
+                if (i.hasNext()) {
+                    recipients.append("\r\n");
+                }
+            }
+            m.setRecipients(recipients.toString());
+            m.setRemoteHost(mail.getRemoteHost());
+            m.setRemoteAddr(mail.getRemoteAddr());
+            if (!mail.getPerRecipientSpecificHeaders().getHeadersByRecipient().isEmpty()) {
+                byte[] bytes = SerializationUtils.serialize(mail.getPerRecipientSpecificHeaders());

Review Comment:
   Implemented custom serialization of the recipients headers into a string. To avoid escaping issues, used only CR/CRCR as delimiters. What do u think of the encoding? simple and future proof enough? should the recipient in the first line appear as-is, or as a header (e.g. "recipient: address") to make it easier to parse all lines as headers in the future? Any other ideas for different/better encoding?



-- 
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


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

Posted by GitBox <gi...@apache.org>.
chibenwa merged PR #1176:
URL: https://github.com/apache/james-project/pull/1176


-- 
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


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

Posted by GitBox <gi...@apache.org>.
amichair commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1248929382

   Sure, sounds like a good plan. Would be great to let it run in production for 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-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


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

Posted by "amichair (via GitHub)" <gi...@apache.org>.
amichair commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1473377118

   When can we expect this to be released in any new or backported version?


-- 
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


[GitHub] [james-project] amichair commented on a diff in pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

Posted by GitBox <gi...@apache.org>.
amichair commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960539770


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;

Review Comment:
   ok



-- 
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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960814824


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;
+        repositoryName = url.substring(start);
+        if (repositoryName.isEmpty()) {
+            String exceptionBuffer = "Malformed destinationURL - Must be of the format '" + "jpa://<repositoryName>'.  Was passed " + url;
+            throw new ConfigurationException(exceptionBuffer);
+        }
+        LOGGER.debug("Parsed URL: repositoryName = '{}'", repositoryName);
+    }
+
+    /**
+     * Initialises the JPA repository.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Override
+    @PostConstruct
+    public void init() throws Exception {
+        LOGGER.debug("{}.initialize()", getClass().getName());
+        list();
+    }
+
+    @Override
+    public MailKey store(Mail mail) throws MessagingException {
+        MailKey key = MailKey.forMail(mail);
+        EntityManager em = em();
+        try {
+            JPAMail m = new JPAMail();
+            m.setRepositoryName(repositoryName);
+            m.setMessageName(mail.getName());
+            m.setMessageState(mail.getState());
+            m.setErrorMessage(mail.getErrorMessage());
+            if (!mail.getMaybeSender().isNullSender()) {
+                m.setSender(mail.getMaybeSender().get().toString());
+            }
+            StringBuilder recipients = new StringBuilder();
+            for (Iterator<MailAddress> i = mail.getRecipients().iterator(); i.hasNext();) {
+                recipients.append(i.next().toString());
+                if (i.hasNext()) {
+                    recipients.append("\r\n");
+                }
+            }
+            m.setRecipients(recipients.toString());
+            m.setRemoteHost(mail.getRemoteHost());
+            m.setRemoteAddr(mail.getRemoteAddr());
+            if (!mail.getPerRecipientSpecificHeaders().getHeadersByRecipient().isEmpty()) {
+                byte[] bytes = SerializationUtils.serialize(mail.getPerRecipientSpecificHeaders());
+                m.setPerRecipientHeaders(bytes);
+            }
+            m.setLastUpdated(new Timestamp(mail.getLastUpdated().getTime()));
+            m.setMessageBody(getBody(mail));
+            if (mail.hasAttributes()) {
+                m.setMessageAttributes(serializeAttributes(mail));
+            }
+            EntityTransaction transaction = em.getTransaction();
+            transaction.begin();
+            m = em.merge(m);
+            transaction.commit();
+            return key;
+        } catch (MessagingException e) {
+            LOGGER.error("Exception caught while storing mail {}", key, e);
+            throw e;
+        } catch (Exception e) {
+            LOGGER.error("Exception caught while storing mail {}", key, e);
+            throw new MessagingException("Exception caught while storing mail " + key, e);
+        } finally {
+            EntityManagerUtils.safelyClose(em);
+        }
+    }
+
+    private byte[] getBody(Mail mail) throws MessagingException, IOException {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();

Review Comment:
   > How about if it's initialized to capacity mail.getMessageSize()? (should have done that anyway)
   
   :+1: 



-- 
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


[GitHub] [james-project] amichair commented on a diff in pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

Posted by GitBox <gi...@apache.org>.
amichair commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960615752


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;

Review Comment:
   I thought the repos are always created and looked up via MailRepositoryStore which works with parsed urls, matches the protocols, registers the repos, etc. - is this not the case? Where are they instantiated directly, and is this correct behavior?



-- 
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


[GitHub] [james-project] amichair commented on a diff in pull request #1176: JAMES-2656 - Add initial JPAMailRepository implementation

Posted by GitBox <gi...@apache.org>.
amichair commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960851314


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;
+        repositoryName = url.substring(start);
+        if (repositoryName.isEmpty()) {
+            String exceptionBuffer = "Malformed destinationURL - Must be of the format '" + "jpa://<repositoryName>'.  Was passed " + url;
+            throw new ConfigurationException(exceptionBuffer);
+        }
+        LOGGER.debug("Parsed URL: repositoryName = '{}'", repositoryName);
+    }
+
+    /**
+     * Initialises the JPA repository.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Override
+    @PostConstruct
+    public void init() throws Exception {
+        LOGGER.debug("{}.initialize()", getClass().getName());
+        list();
+    }
+
+    @Override
+    public MailKey store(Mail mail) throws MessagingException {
+        MailKey key = MailKey.forMail(mail);
+        EntityManager em = em();
+        try {
+            JPAMail m = new JPAMail();
+            m.setRepositoryName(repositoryName);
+            m.setMessageName(mail.getName());
+            m.setMessageState(mail.getState());
+            m.setErrorMessage(mail.getErrorMessage());
+            if (!mail.getMaybeSender().isNullSender()) {
+                m.setSender(mail.getMaybeSender().get().toString());
+            }
+            StringBuilder recipients = new StringBuilder();
+            for (Iterator<MailAddress> i = mail.getRecipients().iterator(); i.hasNext();) {
+                recipients.append(i.next().toString());
+                if (i.hasNext()) {
+                    recipients.append("\r\n");
+                }
+            }
+            m.setRecipients(recipients.toString());
+            m.setRemoteHost(mail.getRemoteHost());
+            m.setRemoteAddr(mail.getRemoteAddr());
+            if (!mail.getPerRecipientSpecificHeaders().getHeadersByRecipient().isEmpty()) {
+                byte[] bytes = SerializationUtils.serialize(mail.getPerRecipientSpecificHeaders());

Review Comment:
   Implemented custom serialization of the recipients headers into a string. To avoid escaping issues, used only CR/CRCR as delimiters. What do u think of the encoding? simple and future proof enough? should the recipient in the first line appear as-is, or as a header (e.g. "recipient: <address>" to make it easier to parse all lines as headers in the future? Any other ideas for different/better encoding?



-- 
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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on code in PR #1176:
URL: https://github.com/apache/james-project/pull/1176#discussion_r960588301


##########
server/data/data-jpa/src/main/java/org/apache/james/mailrepository/jpa/JPAMailRepository.java:
##########
@@ -0,0 +1,322 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+
+package org.apache.james.mailrepository.jpa;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.Serializable;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.StringTokenizer;
+import java.util.stream.Collectors;
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+import javax.mail.MessagingException;
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.EntityTransaction;
+import javax.persistence.NoResultException;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.apache.commons.lang3.SerializationUtils;
+import org.apache.james.backends.jpa.EntityManagerUtils;
+import org.apache.james.core.MailAddress;
+import org.apache.james.lifecycle.api.Configurable;
+import org.apache.james.mailrepository.api.Initializable;
+import org.apache.james.mailrepository.api.MailKey;
+import org.apache.james.mailrepository.api.MailRepository;
+import org.apache.james.mailrepository.jpa.model.JPAMail;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.server.core.MimeMessageWrapper;
+import org.apache.mailet.Attribute;
+import org.apache.mailet.Mail;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * Implementation of a MailRepository on a database via JPA.
+ * 
+ * <p>
+ * Requires a configuration element in the .conf.xml file of the form:
+ * 
+ * <pre>
+ *  &lt;repository destinationURL="jpa://&lt;repository_name&gt;"
+ *              type="MAIL" /&gt;
+ *  &lt;/repository&gt;
+ * </pre>
+ * </p>
+ */
+public class JPAMailRepository implements MailRepository, Configurable, Initializable {
+    private static final Logger LOGGER = LoggerFactory.getLogger(JPAMailRepository.class);
+
+    String repositoryName;
+
+    final EntityManagerFactory entityManagerFactory;
+
+    @Inject
+    public JPAMailRepository(EntityManagerFactory entityManagerFactory) {
+        this.entityManagerFactory = entityManagerFactory;
+    }
+
+    public String getRepositoryName() {
+        return repositoryName;
+    }
+
+    // note: caller must close the returned EntityManager when done using it
+    protected EntityManager em() {
+        return entityManagerFactory.createEntityManager();
+    }
+
+    @Override
+    public void configure(HierarchicalConfiguration<ImmutableNode> configuration) throws ConfigurationException {
+        LOGGER.debug("{}.configure()", getClass().getName());
+        String url = configuration.getString("[@destinationURL]");
+
+        if (url.endsWith("/")) {
+            url = url.substring(0, url.length() - 1);
+        }
+        int start = url.indexOf("://") + 3;

Review Comment:
   > (again - this was based on the JDBC impl)
   
   I do consider JDBC impl legacy and below our current coding standards to be honnest.
   
   >  think the assumption is that the MailRepositoryUrl  l parsing already verifies the generic url syntax, and the repository impl only needs to deal with the details in the path part?
   
   I see that it comes from a conf file as a String, we should not infer any type of validation is enforced.
   
   



-- 
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


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

Posted by GitBox <gi...@apache.org>.
amichair commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1234523300

   Thanks for the in-depth review and super-quick responses! It's really appreciated!
   
   I think everything was addressed, the only thing remaining is the attributes serialization. I'm not sure what you were referring to in "mailet-api comes with a serialization utility", and the example in CassandraMailRepositoryMailDaoV2 seems to do some of its own serializing/deserializing, and there is another class with a modified copy&paste version of it as well. But both only take it as far as a Map, and that too needs to be serialized to the database somehow... as I mentioned initially, the serializations policy seems a bit patchy rather than a cohesive design and reusable implementation. I was hoping for something as simple as SerializationUtils.serialize just with strings :-)
   
   What do you recommend to do at the moment with the JPA attributes?
   
   (btw the serialization changes I didn't backport to jdbc repository so it won't break backwards compatibility with existing databases)


-- 
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


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

Posted by GitBox <gi...@apache.org>.
amichair commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1248925664

   Ooops.... I was about to sqash but I see it's already merged :-)
   
   btw is there an ETA for this release?
   
   Regarding that log, I'm not convinced the problem is with the test, maybe it's a real concurrency bug. Requires some delving into locking in JPA and/or Derby. If there's someone experienced with this, it would be nice if they can have a look. Some wild guesses: maybe another index will help narrow the lock to specific rows, maybe add a specific remove-one query instead of the 'in' clause (but the issue will remain with collections, so not a full solution), maybe add a lock mode parameter to queries... need to figure it out. The biggest issue is that I can't reproduce it locally, so hard to investigate or test solutions.


-- 
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


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

Posted by GitBox <gi...@apache.org>.
chibenwa commented on PR #1176:
URL: https://github.com/apache/james-project/pull/1176#issuecomment-1241895248

   > ok, so just waiting for @chibenwa's answer regarding the attributes serialization. Anything else u need me to do?
   
   Yes I definitly put this high on the afternoon todo 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-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