You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "HiuKwok (via GitHub)" <gi...@apache.org> on 2023/02/06 12:18:57 UTC
[GitHub] [commons-email] HiuKwok opened a new pull request, #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
HiuKwok opened a new pull request, #135:
URL: https://github.com/apache/commons-email/pull/135
As discussed on the Jira ticket, this is the MR to update MimeMessageParser.createDataSource() to construct the `ByteArrayDataSource` object with input steam instead of actual content in order to handle the situation that email with multiple and bulk attachments.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] garydgregory commented on a diff in pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #135:
URL: https://github.com/apache/commons-email/pull/135#discussion_r1123668685
##########
src/main/java/org/apache/commons/mail/LazyByteArrayDataSource.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.commons.mail;
+
+import javax.activation.DataSource;
+import javax.mail.util.ByteArrayDataSource;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * <p>Wrapper class for ByteArrayDataSource, which contain reference of MimePartDataSource for given attachment.
+ * Both type and name are duplicated stored in this class, in order to delay the load of attachment binary till getInputStream() is called.
+ * </p>
+ *
+ * @since 1.5
+ */
+public class LazyByteArrayDataSource implements DataSource {
+
+ /** InputStream reference for the email attachment binary. */
+ private final InputStream referenceInputStream;
+
+ /** ByteArrayDateSource instance which contain email attachment binary in the form of byte array. */
+ private ByteArrayDataSource ds;
+
+ /** Name of the attachment. */
+ private final String name;
+
+ /** Type of the attachment. */
+ private final String type;
+
+
+ /**
+ * Constructor for this class to read all necessary information for an email attachment.
+ *
+ * @param is the InputStream which represent the attachment binary.
+ * @param type the type of the attachment.
+ * @param name the name of the attachment.
+ */
+ public LazyByteArrayDataSource(InputStream is, String type, String name) {
+ this.referenceInputStream = is;
+ this.type = type;
+ this.name = name;
+ }
+
+ /**
+ * To return an {@code ByteArrayDataSource} instance which represent the email attachment.
+ *
+ * @return An ByteArrayDataSource instance which contain the email attachment.
+ * @throws IOException resolving the email attachment failed
+ */
+ @Override
+ public InputStream getInputStream() throws IOException {
+ if (ds == null) {
+ //Only read attachment data to memory when getInputStream() is called.
+ ds = new ByteArrayDataSource(referenceInputStream, type);
+ ds.setName(name);
+ }
+ return ds.getInputStream();
+ }
+
+ /**
+ * Not supported.
+ *
+ * @return N/A
+ * @since 1.5
Review Comment:
You already use `@since 1.5` on the class, that's all that is needed.
##########
src/main/java/org/apache/commons/mail/LazyByteArrayDataSource.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.commons.mail;
+
+import javax.activation.DataSource;
+import javax.mail.util.ByteArrayDataSource;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * <p>Wrapper class for ByteArrayDataSource, which contain reference of MimePartDataSource for given attachment.
+ * Both type and name are duplicated stored in this class, in order to delay the load of attachment binary till getInputStream() is called.
+ * </p>
+ *
+ * @since 1.5
+ */
+public class LazyByteArrayDataSource implements DataSource {
+
+ /** InputStream reference for the email attachment binary. */
+ private final InputStream referenceInputStream;
+
+ /** ByteArrayDateSource instance which contain email attachment binary in the form of byte array. */
+ private ByteArrayDataSource ds;
+
+ /** Name of the attachment. */
+ private final String name;
+
+ /** Type of the attachment. */
+ private final String type;
+
+
+ /**
+ * Constructor for this class to read all necessary information for an email attachment.
Review Comment:
Constructor... -> Constructs a new instance to read all necessary information for an email attachment.
##########
src/main/java/org/apache/commons/mail/LazyByteArrayDataSource.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.commons.mail;
+
+import javax.activation.DataSource;
+import javax.mail.util.ByteArrayDataSource;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * <p>Wrapper class for ByteArrayDataSource, which contain reference of MimePartDataSource for given attachment.
+ * Both type and name are duplicated stored in this class, in order to delay the load of attachment binary till getInputStream() is called.
+ * </p>
+ *
+ * @since 1.5
+ */
+public class LazyByteArrayDataSource implements DataSource {
+
+ /** InputStream reference for the email attachment binary. */
+ private final InputStream referenceInputStream;
+
+ /** ByteArrayDateSource instance which contain email attachment binary in the form of byte array. */
+ private ByteArrayDataSource ds;
+
+ /** Name of the attachment. */
+ private final String name;
+
+ /** Type of the attachment. */
+ private final String type;
+
+
+ /**
+ * Constructor for this class to read all necessary information for an email attachment.
+ *
+ * @param is the InputStream which represent the attachment binary.
+ * @param type the type of the attachment.
+ * @param name the name of the attachment.
+ */
+ public LazyByteArrayDataSource(InputStream is, String type, String name) {
+ this.referenceInputStream = is;
+ this.type = type;
+ this.name = name;
+ }
+
+ /**
+ * To return an {@code ByteArrayDataSource} instance which represent the email attachment.
+ *
+ * @return An ByteArrayDataSource instance which contain the email attachment.
+ * @throws IOException resolving the email attachment failed
+ */
+ @Override
+ public InputStream getInputStream() throws IOException {
+ if (ds == null) {
Review Comment:
If this class is not meant to be thread-safe, then document that in the class comment.
##########
src/test/java/org/apache/commons/mail/util/MimeMessageParserTest.java:
##########
@@ -498,4 +503,61 @@ public void testParseInlineCID() throws Exception
assertEquals(ds, mimeMessageParser.getAttachmentList().get(0));
}
+ @Test
+ public void testAttachmentNotLoaded() throws Exception
+ {
+ final MimeMessageParser mimeMessageParser = new MimeMessageParser(null);
+ final InputStream inputStream = createMock(InputStream.class);
+ final MimePart mimePart = getMockedMimePart(inputStream);
+
+ // Create data source with mocked data.
+ final DataSource dataSource_new = mimeMessageParser.createDataSource(null,mimePart);
+ // Verify no inputStream.read() is made at this point (Lazy initialization).
+ verify(inputStream);
+ }
+
+
+ @Test
+ public void testAttachmentLoaded() throws Exception
+ {
+ final MimeMessageParser mimeMessageParser = new MimeMessageParser(null);
+ final InputStream inputStream = createMock(InputStream.class);
+ // Despite .getInputStream() called for 3 times, but the desk IO for attachment read should only happen once.
+ expect(inputStream.read(new byte[8192])).andReturn(0).once();
+ final MimePart mimePart = getMockedMimePart(inputStream);
+
+ // Create data source with mocked data.
+ final DataSource dataSource_new = mimeMessageParser.createDataSource(null,mimePart);
Review Comment:
No underscores in names, please, use camel case.
##########
src/main/java/org/apache/commons/mail/LazyByteArrayDataSource.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.commons.mail;
+
+import javax.activation.DataSource;
+import javax.mail.util.ByteArrayDataSource;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * <p>Wrapper class for ByteArrayDataSource, which contain reference of MimePartDataSource for given attachment.
+ * Both type and name are duplicated stored in this class, in order to delay the load of attachment binary till getInputStream() is called.
+ * </p>
+ *
+ * @since 1.5
+ */
+public class LazyByteArrayDataSource implements DataSource {
+
+ /** InputStream reference for the email attachment binary. */
+ private final InputStream referenceInputStream;
+
+ /** ByteArrayDateSource instance which contain email attachment binary in the form of byte array. */
+ private ByteArrayDataSource ds;
+
+ /** Name of the attachment. */
+ private final String name;
+
+ /** Type of the attachment. */
+ private final String type;
+
Review Comment:
One blank line, not 2.
##########
src/main/java/org/apache/commons/mail/LazyByteArrayDataSource.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.commons.mail;
+
+import javax.activation.DataSource;
+import javax.mail.util.ByteArrayDataSource;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * <p>Wrapper class for ByteArrayDataSource, which contain reference of MimePartDataSource for given attachment.
+ * Both type and name are duplicated stored in this class, in order to delay the load of attachment binary till getInputStream() is called.
+ * </p>
+ *
+ * @since 1.5
+ */
+public class LazyByteArrayDataSource implements DataSource {
+
+ /** InputStream reference for the email attachment binary. */
+ private final InputStream referenceInputStream;
+
+ /** ByteArrayDateSource instance which contain email attachment binary in the form of byte array. */
+ private ByteArrayDataSource ds;
+
+ /** Name of the attachment. */
+ private final String name;
+
+ /** Type of the attachment. */
+ private final String type;
+
+
+ /**
+ * Constructor for this class to read all necessary information for an email attachment.
+ *
+ * @param is the InputStream which represent the attachment binary.
+ * @param type the type of the attachment.
+ * @param name the name of the attachment.
+ */
+ public LazyByteArrayDataSource(InputStream is, String type, String name) {
+ this.referenceInputStream = is;
+ this.type = type;
+ this.name = name;
+ }
+
+ /**
+ * To return an {@code ByteArrayDataSource} instance which represent the email attachment.
+ *
+ * @return An ByteArrayDataSource instance which contain the email attachment.
+ * @throws IOException resolving the email attachment failed
+ */
+ @Override
+ public InputStream getInputStream() throws IOException {
+ if (ds == null) {
+ //Only read attachment data to memory when getInputStream() is called.
+ ds = new ByteArrayDataSource(referenceInputStream, type);
+ ds.setName(name);
+ }
+ return ds.getInputStream();
+ }
+
+ /**
+ * Not supported.
+ *
+ * @return N/A
+ * @since 1.5
+ */
+ @Override
+ public OutputStream getOutputStream() throws IOException {
+ throw new IOException("cannot do this");
+ }
+
+ /**
+ * Get the content type.
+ *
+ * @return A String.
+ * @since 1.5
Review Comment:
You already use `@since 1.5` on the class, that's all that is needed.
##########
src/main/java/org/apache/commons/mail/LazyByteArrayDataSource.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.commons.mail;
+
+import javax.activation.DataSource;
+import javax.mail.util.ByteArrayDataSource;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * <p>Wrapper class for ByteArrayDataSource, which contain reference of MimePartDataSource for given attachment.
+ * Both type and name are duplicated stored in this class, in order to delay the load of attachment binary till getInputStream() is called.
+ * </p>
+ *
+ * @since 1.5
+ */
+public class LazyByteArrayDataSource implements DataSource {
+
+ /** InputStream reference for the email attachment binary. */
+ private final InputStream referenceInputStream;
+
+ /** ByteArrayDateSource instance which contain email attachment binary in the form of byte array. */
+ private ByteArrayDataSource ds;
+
+ /** Name of the attachment. */
+ private final String name;
+
+ /** Type of the attachment. */
+ private final String type;
+
+
+ /**
+ * Constructor for this class to read all necessary information for an email attachment.
+ *
+ * @param is the InputStream which represent the attachment binary.
+ * @param type the type of the attachment.
+ * @param name the name of the attachment.
+ */
+ public LazyByteArrayDataSource(InputStream is, String type, String name) {
+ this.referenceInputStream = is;
+ this.type = type;
+ this.name = name;
+ }
+
+ /**
+ * To return an {@code ByteArrayDataSource} instance which represent the email attachment.
+ *
+ * @return An ByteArrayDataSource instance which contain the email attachment.
+ * @throws IOException resolving the email attachment failed
+ */
+ @Override
+ public InputStream getInputStream() throws IOException {
+ if (ds == null) {
+ //Only read attachment data to memory when getInputStream() is called.
+ ds = new ByteArrayDataSource(referenceInputStream, type);
+ ds.setName(name);
+ }
+ return ds.getInputStream();
+ }
+
+ /**
+ * Not supported.
+ *
+ * @return N/A
+ * @since 1.5
+ */
+ @Override
+ public OutputStream getOutputStream() throws IOException {
+ throw new IOException("cannot do this");
+ }
+
+ /**
+ * Get the content type.
+ *
+ * @return A String.
+ * @since 1.5
+ */
+ @Override
+ public String getContentType() {
+ return type;
+ }
+
+ /**
+ * Get the name.
+ *
+ * @return A String.
+ * @since 1.5
Review Comment:
You already use `@since 1.5` on the class, that's all that is needed.
##########
src/main/java/org/apache/commons/mail/LazyByteArrayDataSource.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.commons.mail;
+
+import javax.activation.DataSource;
+import javax.mail.util.ByteArrayDataSource;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * <p>Wrapper class for ByteArrayDataSource, which contain reference of MimePartDataSource for given attachment.
+ * Both type and name are duplicated stored in this class, in order to delay the load of attachment binary till getInputStream() is called.
+ * </p>
+ *
+ * @since 1.5
+ */
+public class LazyByteArrayDataSource implements DataSource {
+
+ /** InputStream reference for the email attachment binary. */
+ private final InputStream referenceInputStream;
+
+ /** ByteArrayDateSource instance which contain email attachment binary in the form of byte array. */
+ private ByteArrayDataSource ds;
+
+ /** Name of the attachment. */
+ private final String name;
+
+ /** Type of the attachment. */
+ private final String type;
+
+
+ /**
+ * Constructor for this class to read all necessary information for an email attachment.
+ *
+ * @param is the InputStream which represent the attachment binary.
+ * @param type the type of the attachment.
+ * @param name the name of the attachment.
+ */
+ public LazyByteArrayDataSource(InputStream is, String type, String name) {
+ this.referenceInputStream = is;
+ this.type = type;
+ this.name = name;
+ }
+
+ /**
+ * To return an {@code ByteArrayDataSource} instance which represent the email attachment.
Review Comment:
A getter "Gets...", "To..." -> "Gets..."
##########
src/main/java/org/apache/commons/mail/LazyByteArrayDataSource.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.commons.mail;
+
+import javax.activation.DataSource;
+import javax.mail.util.ByteArrayDataSource;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * <p>Wrapper class for ByteArrayDataSource, which contain reference of MimePartDataSource for given attachment.
+ * Both type and name are duplicated stored in this class, in order to delay the load of attachment binary till getInputStream() is called.
+ * </p>
+ *
+ * @since 1.5
+ */
+public class LazyByteArrayDataSource implements DataSource {
+
+ /** InputStream reference for the email attachment binary. */
+ private final InputStream referenceInputStream;
+
+ /** ByteArrayDateSource instance which contain email attachment binary in the form of byte array. */
+ private ByteArrayDataSource ds;
+
+ /** Name of the attachment. */
+ private final String name;
+
+ /** Type of the attachment. */
+ private final String type;
+
+
+ /**
+ * Constructor for this class to read all necessary information for an email attachment.
+ *
+ * @param is the InputStream which represent the attachment binary.
+ * @param type the type of the attachment.
+ * @param name the name of the attachment.
+ */
+ public LazyByteArrayDataSource(InputStream is, String type, String name) {
+ this.referenceInputStream = is;
+ this.type = type;
+ this.name = name;
+ }
+
+ /**
+ * To return an {@code ByteArrayDataSource} instance which represent the email attachment.
+ *
+ * @return An ByteArrayDataSource instance which contain the email attachment.
+ * @throws IOException resolving the email attachment failed
+ */
+ @Override
+ public InputStream getInputStream() throws IOException {
+ if (ds == null) {
+ //Only read attachment data to memory when getInputStream() is called.
+ ds = new ByteArrayDataSource(referenceInputStream, type);
+ ds.setName(name);
+ }
+ return ds.getInputStream();
+ }
+
+ /**
+ * Not supported.
+ *
+ * @return N/A
+ * @since 1.5
+ */
+ @Override
+ public OutputStream getOutputStream() throws IOException {
+ throw new IOException("cannot do this");
+ }
+
+ /**
+ * Get the content type.
Review Comment:
"Get" -> "Gets".
##########
src/main/java/org/apache/commons/mail/LazyByteArrayDataSource.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.commons.mail;
+
+import javax.activation.DataSource;
+import javax.mail.util.ByteArrayDataSource;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * <p>Wrapper class for ByteArrayDataSource, which contain reference of MimePartDataSource for given attachment.
+ * Both type and name are duplicated stored in this class, in order to delay the load of attachment binary till getInputStream() is called.
+ * </p>
+ *
+ * @since 1.5
+ */
+public class LazyByteArrayDataSource implements DataSource {
+
+ /** InputStream reference for the email attachment binary. */
+ private final InputStream referenceInputStream;
+
+ /** ByteArrayDateSource instance which contain email attachment binary in the form of byte array. */
+ private ByteArrayDataSource ds;
+
+ /** Name of the attachment. */
+ private final String name;
+
+ /** Type of the attachment. */
+ private final String type;
+
+
+ /**
+ * Constructor for this class to read all necessary information for an email attachment.
+ *
+ * @param is the InputStream which represent the attachment binary.
+ * @param type the type of the attachment.
+ * @param name the name of the attachment.
+ */
+ public LazyByteArrayDataSource(InputStream is, String type, String name) {
+ this.referenceInputStream = is;
+ this.type = type;
+ this.name = name;
+ }
+
+ /**
+ * To return an {@code ByteArrayDataSource} instance which represent the email attachment.
+ *
+ * @return An ByteArrayDataSource instance which contain the email attachment.
+ * @throws IOException resolving the email attachment failed
+ */
+ @Override
+ public InputStream getInputStream() throws IOException {
+ if (ds == null) {
+ //Only read attachment data to memory when getInputStream() is called.
+ ds = new ByteArrayDataSource(referenceInputStream, type);
+ ds.setName(name);
+ }
+ return ds.getInputStream();
+ }
+
+ /**
+ * Not supported.
+ *
+ * @return N/A
+ * @since 1.5
+ */
+ @Override
+ public OutputStream getOutputStream() throws IOException {
+ throw new IOException("cannot do this");
+ }
+
+ /**
+ * Get the content type.
+ *
+ * @return A String.
+ * @since 1.5
+ */
+ @Override
+ public String getContentType() {
+ return type;
+ }
+
+ /**
+ * Get the name.
Review Comment:
"Gets..."
##########
src/main/java/org/apache/commons/mail/LazyByteArrayDataSource.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.commons.mail;
+
+import javax.activation.DataSource;
+import javax.mail.util.ByteArrayDataSource;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * <p>Wrapper class for ByteArrayDataSource, which contain reference of MimePartDataSource for given attachment.
+ * Both type and name are duplicated stored in this class, in order to delay the load of attachment binary till getInputStream() is called.
+ * </p>
+ *
+ * @since 1.5
+ */
+public class LazyByteArrayDataSource implements DataSource {
+
+ /** InputStream reference for the email attachment binary. */
+ private final InputStream referenceInputStream;
+
+ /** ByteArrayDateSource instance which contain email attachment binary in the form of byte array. */
+ private ByteArrayDataSource ds;
+
+ /** Name of the attachment. */
+ private final String name;
+
+ /** Type of the attachment. */
+ private final String type;
+
+
+ /**
+ * Constructor for this class to read all necessary information for an email attachment.
+ *
+ * @param is the InputStream which represent the attachment binary.
+ * @param type the type of the attachment.
+ * @param name the name of the attachment.
+ */
+ public LazyByteArrayDataSource(InputStream is, String type, String name) {
+ this.referenceInputStream = is;
+ this.type = type;
+ this.name = name;
+ }
+
+ /**
+ * To return an {@code ByteArrayDataSource} instance which represent the email attachment.
+ *
+ * @return An ByteArrayDataSource instance which contain the email attachment.
+ * @throws IOException resolving the email attachment failed
+ */
+ @Override
+ public InputStream getInputStream() throws IOException {
+ if (ds == null) {
+ //Only read attachment data to memory when getInputStream() is called.
+ ds = new ByteArrayDataSource(referenceInputStream, type);
+ ds.setName(name);
+ }
+ return ds.getInputStream();
+ }
+
+ /**
+ * Not supported.
+ *
+ * @return N/A
+ * @since 1.5
+ */
+ @Override
+ public OutputStream getOutputStream() throws IOException {
+ throw new IOException("cannot do this");
Review Comment:
There is an exception for that: `UnsupportedOperationException`.
##########
src/main/java/org/apache/commons/mail/util/MimeMessageParser.java:
##########
@@ -270,14 +267,8 @@ protected DataSource createDataSource(final Multipart parent, final MimePart par
final DataHandler dataHandler = part.getDataHandler();
final DataSource dataSource = dataHandler.getDataSource();
final String contentType = getBaseMimeType(dataSource.getContentType());
- byte[] content;
- try (InputStream inputStream = dataSource.getInputStream())
- {
- content = this.getContent(inputStream);
- }
- final ByteArrayDataSource result = new ByteArrayDataSource(content, contentType);
final String dataSourceName = getDataSourceName(part, dataSource);
- result.setName(dataSourceName);
+ final LazyByteArrayDataSource result = new LazyByteArrayDataSource(dataSource.getInputStream(), contentType, dataSourceName);
Review Comment:
It seems the local variable is no longer needed.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] HiuKwok commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1446295221
This is not yet ready, I'm working on the test-cases
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] garydgregory commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1422451186
Would mocking help here?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] HiuKwok commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1454491921
I believe all comments are addressed, let me know if I missed any, thanks!
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] HiuKwok commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1474811108
@garydgregory all comments are addressed and I have fixed the test cases.
Would that be possible to have another round of review?
Thanks,
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] HiuKwok commented on a diff in pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on code in PR #135:
URL: https://github.com/apache/commons-email/pull/135#discussion_r1099706509
##########
src/main/java/org/apache/commons/mail/util/MimeMessageParser.java:
##########
@@ -16,34 +16,17 @@
*/
package org.apache.commons.mail.util;
-import java.io.BufferedInputStream;
-import java.io.BufferedOutputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.UnsupportedEncodingException;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
import javax.activation.DataHandler;
import javax.activation.DataSource;
import javax.mail.Message;
import javax.mail.MessagingException;
import javax.mail.Multipart;
import javax.mail.Part;
-import javax.mail.internet.ContentType;
-import javax.mail.internet.InternetAddress;
-import javax.mail.internet.MimeBodyPart;
-import javax.mail.internet.MimeMessage;
-import javax.mail.internet.MimePart;
-import javax.mail.internet.MimeUtility;
-import javax.mail.internet.ParseException;
+import javax.mail.internet.*;
Review Comment:
I have fixed this, but still looking for a way to have new unit test.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] HiuKwok commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1419113353
@garydgregory Can you also advise on how unit can be written in this case?
As this is a performance improvement rather than a bug fix, hence this is no change of behavior from an external perspective I can assert.
I'm thinking to create a huge `.eml` file with an attachment in order to trigger an OOM exception as the original approach will load the whole file into the memory, which the revised method only loads a portion of the file into the input stream.
But I don't think that makes sense to push a huge `.eml` under the test resources folder either.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] EMAIL-204: Disable eager attachment loading on MimeMessageParser [commons-email]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1858931551
Hello @HiuKwok,
I am sorry about not getting back to you sooner.
I belive this functionality is currently implemented, albeit differently, in git master. Please verify your use case and close this or update this PR as appropriate.
Thank you for your patience! 😄
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] HiuKwok commented on a diff in pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on code in PR #135:
URL: https://github.com/apache/commons-email/pull/135#discussion_r1125355768
##########
src/main/java/org/apache/commons/mail/LazyByteArrayDataSource.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.commons.mail;
+
+import javax.activation.DataSource;
+import javax.mail.util.ByteArrayDataSource;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * <p>Wrapper class for ByteArrayDataSource, which contain reference of MimePartDataSource for given attachment.
+ * Both type and name are duplicated stored in this class, in order to delay the load of attachment binary till getInputStream() is called.
+ * </p>
+ *
+ * @since 1.5
+ */
+public class LazyByteArrayDataSource implements DataSource {
+
+ /** InputStream reference for the email attachment binary. */
+ private final InputStream referenceInputStream;
+
+ /** ByteArrayDateSource instance which contain email attachment binary in the form of byte array. */
+ private ByteArrayDataSource ds;
+
+ /** Name of the attachment. */
+ private final String name;
+
+ /** Type of the attachment. */
+ private final String type;
+
+
+ /**
+ * Constructor for this class to read all necessary information for an email attachment.
+ *
+ * @param is the InputStream which represent the attachment binary.
+ * @param type the type of the attachment.
+ * @param name the name of the attachment.
+ */
+ public LazyByteArrayDataSource(InputStream is, String type, String name) {
+ this.referenceInputStream = is;
+ this.type = type;
+ this.name = name;
+ }
+
+ /**
+ * To return an {@code ByteArrayDataSource} instance which represent the email attachment.
+ *
+ * @return An ByteArrayDataSource instance which contain the email attachment.
+ * @throws IOException resolving the email attachment failed
+ */
+ @Override
+ public InputStream getInputStream() throws IOException {
+ if (ds == null) {
Review Comment:
Actually, I think this can be thread-safe by having `synchronized` to lock the method.
Updated the code accordingly.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] HiuKwok commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1452228622
@garydgregory I have updated the code change once again with two new test cases to represent the scenarios of lazy loading of Email attachments with appropriate mocking.
Would that be possible for you to review it again?
Appreciate that.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] HiuKwok commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1454788782
@garydgregory I can now reproduce now the issue after running the unit test locally with jdk8.
I will check and update the test case accordingly if need.
Thx for the help!
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] codecov-commenter commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1419039905
# [Codecov](https://codecov.io/gh/apache/commons-email/pull/135?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#135](https://codecov.io/gh/apache/commons-email/pull/135?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (49503b9) into [master](https://codecov.io/gh/apache/commons-email/commit/347d5d9a8984f20a8d8a93bfa69fd246ff838e7c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (347d5d9) will **decrease** coverage by `0.74%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #135 +/- ##
============================================
- Coverage 65.90% 65.17% -0.74%
+ Complexity 305 303 -2
============================================
Files 18 18
Lines 1053 1051 -2
Branches 138 138
============================================
- Hits 694 685 -9
- Misses 280 287 +7
Partials 79 79
```
| [Impacted Files](https://codecov.io/gh/apache/commons-email/pull/135?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...rg/apache/commons/mail/util/MimeMessageParser.java](https://codecov.io/gh/apache/commons-email/pull/135?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvbWFpbC91dGlsL01pbWVNZXNzYWdlUGFyc2VyLmphdmE=) | `78.02% <100.00%> (-8.00%)` | :arrow_down: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] garydgregory commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1419037684
Would you please write unit test that fails without the change to main?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] HiuKwok commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1454796350
It turns out the easymock is having issues mocking the class `DataHandler`,
for now, I have replaced the mock of the class with a constructor instead.
```
final DataHandler dataHandler = new DataHandler(dataSource);
```
I have re-run the test suite locally against JDK8, 11 and 17 and all tests are passing for all three versions :)
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] HiuKwok commented on a diff in pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on code in PR #135:
URL: https://github.com/apache/commons-email/pull/135#discussion_r1125353219
##########
src/main/java/org/apache/commons/mail/LazyByteArrayDataSource.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.commons.mail;
+
+import javax.activation.DataSource;
+import javax.mail.util.ByteArrayDataSource;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * <p>Wrapper class for ByteArrayDataSource, which contain reference of MimePartDataSource for given attachment.
+ * Both type and name are duplicated stored in this class, in order to delay the load of attachment binary till getInputStream() is called.
+ * </p>
+ *
+ * @since 1.5
+ */
+public class LazyByteArrayDataSource implements DataSource {
+
+ /** InputStream reference for the email attachment binary. */
+ private final InputStream referenceInputStream;
+
+ /** ByteArrayDateSource instance which contain email attachment binary in the form of byte array. */
+ private ByteArrayDataSource ds;
+
+ /** Name of the attachment. */
+ private final String name;
+
+ /** Type of the attachment. */
+ private final String type;
+
+
+ /**
+ * Constructor for this class to read all necessary information for an email attachment.
+ *
+ * @param is the InputStream which represent the attachment binary.
+ * @param type the type of the attachment.
+ * @param name the name of the attachment.
+ */
+ public LazyByteArrayDataSource(InputStream is, String type, String name) {
+ this.referenceInputStream = is;
+ this.type = type;
+ this.name = name;
+ }
+
+ /**
+ * To return an {@code ByteArrayDataSource} instance which represent the email attachment.
Review Comment:
how about `Gets an {@code ByteArrayDataSource} instance to represent the email attachment.`?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] HiuKwok commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1454783852
Yes, my local build is green and I tried the exact commands used for the failed builds, which all build successfully (I couldn't reproduce this on my local).
By checking on the error message it seems related to the Easymock, let me investigate, and will update you once again on this thread.
BTW my setup:
OS: MAC OS 12.15.1
Java: Open JDK 17.0.2
MVN: 3.8.5
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] garydgregory commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1454784580
I would try locally with Java 8.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] HiuKwok commented on a diff in pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on code in PR #135:
URL: https://github.com/apache/commons-email/pull/135#discussion_r1125353219
##########
src/main/java/org/apache/commons/mail/LazyByteArrayDataSource.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * 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.commons.mail;
+
+import javax.activation.DataSource;
+import javax.mail.util.ByteArrayDataSource;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * <p>Wrapper class for ByteArrayDataSource, which contain reference of MimePartDataSource for given attachment.
+ * Both type and name are duplicated stored in this class, in order to delay the load of attachment binary till getInputStream() is called.
+ * </p>
+ *
+ * @since 1.5
+ */
+public class LazyByteArrayDataSource implements DataSource {
+
+ /** InputStream reference for the email attachment binary. */
+ private final InputStream referenceInputStream;
+
+ /** ByteArrayDateSource instance which contain email attachment binary in the form of byte array. */
+ private ByteArrayDataSource ds;
+
+ /** Name of the attachment. */
+ private final String name;
+
+ /** Type of the attachment. */
+ private final String type;
+
+
+ /**
+ * Constructor for this class to read all necessary information for an email attachment.
+ *
+ * @param is the InputStream which represent the attachment binary.
+ * @param type the type of the attachment.
+ * @param name the name of the attachment.
+ */
+ public LazyByteArrayDataSource(InputStream is, String type, String name) {
+ this.referenceInputStream = is;
+ this.type = type;
+ this.name = name;
+ }
+
+ /**
+ * To return an {@code ByteArrayDataSource} instance which represent the email attachment.
Review Comment:
how about `Gets an {@code ByteArrayDataSource} instance, to represent the email attachment.`?
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] garydgregory commented on a diff in pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #135:
URL: https://github.com/apache/commons-email/pull/135#discussion_r1097847237
##########
src/main/java/org/apache/commons/mail/util/MimeMessageParser.java:
##########
@@ -16,34 +16,17 @@
*/
package org.apache.commons.mail.util;
-import java.io.BufferedInputStream;
-import java.io.BufferedOutputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.UnsupportedEncodingException;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
import javax.activation.DataHandler;
import javax.activation.DataSource;
import javax.mail.Message;
import javax.mail.MessagingException;
import javax.mail.Multipart;
import javax.mail.Part;
-import javax.mail.internet.ContentType;
-import javax.mail.internet.InternetAddress;
-import javax.mail.internet.MimeBodyPart;
-import javax.mail.internet.MimeMessage;
-import javax.mail.internet.MimePart;
-import javax.mail.internet.MimeUtility;
-import javax.mail.internet.ParseException;
+import javax.mail.internet.*;
Review Comment:
Don't change the imports to use a star.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] HiuKwok commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1422843344
I reckon so, to have a test check on .getInputStream() count and make only it only called when the user is accessing the attachment content.
I will update this MR once again with the test.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] HiuKwok commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1419128531
Actually, I can probably create a test to assert the object size of `mimeMessageParser`, the memory consumption should reduce after the revised code change.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] garydgregory commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1454763771
@HiuKwok
The build fails. Run 'mvn' on the command line before you push. Or, did you not see failures on your set up? OS? Java 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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-email] HiuKwok commented on pull request #135: EMAIL-204: Disable eager attachment loading on MimeMessageParser
Posted by "HiuKwok (via GitHub)" <gi...@apache.org>.
HiuKwok commented on PR #135:
URL: https://github.com/apache/commons-email/pull/135#issuecomment-1454784986
Since it failed on Java 8 test, I will install Java8 locally and rerun the test.
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org