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

[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