You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by ma...@apache.org on 2023/02/02 22:08:10 UTC

[james-project] branch refactorings-1 created (now b769e07009)

This is an automated email from the ASF dual-hosted git repository.

matthieu pushed a change to branch refactorings-1
in repository https://gitbox.apache.org/repos/asf/james-project.git


      at b769e07009 Ensure InputStream is closed after text extract processing is done to avoid leaks

This branch includes the following new commits:

     new 9799d0762f Remove unused methods
     new b769e07009 Ensure InputStream is closed after text extract processing is done to avoid leaks

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



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


[james-project] 02/02: Ensure InputStream is closed after text extract processing is done to avoid leaks

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

matthieu pushed a commit to branch refactorings-1
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit b769e07009bb726cefe29c2299389526633a014e
Author: Matthieu Baechler <ma...@baechler-craftsmanship.fr>
AuthorDate: Thu Feb 2 21:07:18 2023 +0100

    Ensure InputStream is closed after text extract processing is done to avoid leaks
---
 .../james/mailbox/extractor/TextExtractor.java     | 12 ++++-
 .../mailbox/extractor/TextExtractorContract.java   | 59 ++++++++++++++++++++--
 .../store/extractor/DefaultTextExtractor.java      | 19 ++++---
 .../store/extractor/DefaultTextExtractorTest.java  | 19 ++++++-
 4 files changed, 96 insertions(+), 13 deletions(-)

diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/extractor/TextExtractor.java b/mailbox/api/src/main/java/org/apache/james/mailbox/extractor/TextExtractor.java
index 7628839bd0..7747b8c0f7 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/extractor/TextExtractor.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/extractor/TextExtractor.java
@@ -21,6 +21,7 @@ package org.apache.james.mailbox.extractor;
 
 import java.io.InputStream;
 
+import com.github.fge.lambdas.Throwing;
 import org.apache.james.mailbox.model.ContentType;
 
 import reactor.core.publisher.Mono;
@@ -31,11 +32,18 @@ public interface TextExtractor {
         return true;
     }
 
+    /**
+     * This method will close the InputStream argument.
+     */
     ParsedContent extractContent(InputStream inputStream, ContentType contentType) throws Exception;
 
+    /**
+     * This method will close the InputStream argument.
+     */
     default Mono<ParsedContent> extractContentReactive(InputStream inputStream, ContentType contentType) {
-        return Mono.fromCallable(() -> extractContent(inputStream, contentType))
-            .subscribeOn(Schedulers.boundedElastic());
+        return Mono.using(() -> inputStream,
+                stream -> Mono.fromCallable(() -> extractContent(stream, contentType)).subscribeOn(Schedulers.boundedElastic()),
+                Throwing.consumer(InputStream::close).orDoNothing());
     }
 
 }
diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/extractor/TextExtractorContract.java b/mailbox/api/src/test/java/org/apache/james/mailbox/extractor/TextExtractorContract.java
index 7fbce4131d..2b88072362 100644
--- a/mailbox/api/src/test/java/org/apache/james/mailbox/extractor/TextExtractorContract.java
+++ b/mailbox/api/src/test/java/org/apache/james/mailbox/extractor/TextExtractorContract.java
@@ -1,6 +1,59 @@
 package org.apache.james.mailbox.extractor;
 
-import static org.junit.jupiter.api.Assertions.*;
-class TextExtractorContract {
-  
+import org.apache.james.mailbox.model.ContentType;
+import org.junit.jupiter.api.Test;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+import static org.assertj.core.api.Assertions.catchException;
+import static org.mockito.Mockito.*;
+
+public interface TextExtractorContract {
+
+    TextExtractor testee();
+    ContentType supportedContentType();
+
+    byte[] supportedContent();
+
+    @Test
+    default void extractContentShouldCloseInputStreamOnSuccess() throws Exception {
+        InputStream stream = spy(new ByteArrayInputStream(supportedContent()));
+
+        testee().extractContent(stream, supportedContentType());
+
+        verify(stream).close();
+    }
+
+    @Test
+    default void extractContentShouldCloseInputStreamOnException() throws Exception {
+        InputStream stream = mock(InputStream.class);
+
+        when(stream.read(any(), anyInt(), anyInt())).thenThrow(new IOException(""));
+
+        catchException(() -> testee().extractContent(stream, supportedContentType()));
+
+        verify(stream).close();
+    }
+
+    @Test
+    default void extractContentReactiveShouldCloseInputStreamOnSuccess() throws Exception {
+        InputStream stream = spy(new ByteArrayInputStream(supportedContent()));
+
+        testee().extractContentReactive(stream, supportedContentType()).block();
+
+        verify(stream).close();
+    }
+
+    @Test
+    default void extractContentReactiveShouldCloseInputStreamOnException() throws Exception {
+        InputStream stream = mock(InputStream.class);
+
+        when(stream.read(any(), anyInt(), anyInt())).thenThrow(new IOException(""));
+
+        catchException(() -> testee().extractContentReactive(stream, supportedContentType()).block());
+
+        verify(stream).close();
+    }
 }
\ No newline at end of file
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractor.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractor.java
index b2af165023..133eab9653 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractor.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractor.java
@@ -25,6 +25,7 @@ import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.Optional;
 
+import com.github.fge.lambdas.Throwing;
 import org.apache.commons.io.IOUtils;
 import org.apache.james.mailbox.extractor.ParsedContent;
 import org.apache.james.mailbox.extractor.TextExtractor;
@@ -46,11 +47,13 @@ public class DefaultTextExtractor implements TextExtractor {
 
     @Override
     public ParsedContent extractContent(InputStream inputStream, ContentType contentType) throws Exception {
-        if (applicable(contentType)) {
-            Charset charset = contentType.charset().orElse(StandardCharsets.UTF_8);
-            return new ParsedContent(Optional.ofNullable(IOUtils.toString(inputStream, charset)), new HashMap<>());
-        } else {
-            return new ParsedContent(Optional.empty(), new HashMap<>());
+        try (var input = inputStream) {
+            if (applicable(contentType)) {
+                Charset charset = contentType.charset().orElse(StandardCharsets.UTF_8);
+                return new ParsedContent(Optional.ofNullable(IOUtils.toString(input, charset)), new HashMap<>());
+            } else {
+                return new ParsedContent(Optional.empty(), new HashMap<>());
+            }
         }
     }
 
@@ -58,8 +61,10 @@ public class DefaultTextExtractor implements TextExtractor {
     public Mono<ParsedContent> extractContentReactive(InputStream inputStream, ContentType contentType) {
         if (applicable(contentType)) {
             Charset charset = contentType.charset().orElse(StandardCharsets.UTF_8);
-            return Mono.fromCallable(() -> new ParsedContent(Optional.ofNullable(IOUtils.toString(inputStream, charset)), new HashMap<>()))
-                .subscribeOn(Schedulers.boundedElastic());
+            return Mono.using(() -> inputStream,
+                    stream -> Mono.fromCallable(() -> new ParsedContent(Optional.ofNullable(IOUtils.toString(stream, charset)), new HashMap<>()))
+                            .subscribeOn(Schedulers.boundedElastic()),
+                    Throwing.consumer(InputStream::close).orDoNothing());
         } else {
             return Mono.just(new ParsedContent(Optional.empty(), new HashMap<>()));
         }
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractorTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractorTest.java
index aaac5b987d..954cb86283 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractorTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/extractor/DefaultTextExtractorTest.java
@@ -22,15 +22,32 @@ package org.apache.james.mailbox.store.extractor;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
 
 import org.apache.james.mailbox.extractor.TextExtractor;
+import org.apache.james.mailbox.extractor.TextExtractorContract;
 import org.apache.james.mailbox.model.ContentType;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
-class DefaultTextExtractorTest {
+class DefaultTextExtractorTest implements TextExtractorContract {
     TextExtractor textExtractor;
 
+    @Override
+    public TextExtractor testee() {
+        return textExtractor;
+    }
+
+    @Override
+    public ContentType supportedContentType() {
+        return ContentType.of("text/plain");
+    }
+
+    @Override
+    public byte[] supportedContent() {
+        return "foo".getBytes(StandardCharsets.UTF_8);
+    }
+
     @BeforeEach
     void setUp() {
         textExtractor = new DefaultTextExtractor();


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


[james-project] 01/02: Remove unused methods

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

matthieu pushed a commit to branch refactorings-1
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit 9799d0762fc625bb9757fd10483c2f6e6eb608ab
Author: Matthieu Baechler <ma...@baechler-craftsmanship.fr>
AuthorDate: Thu Feb 2 21:05:25 2023 +0100

    Remove unused methods
---
 .../apache/james/mailbox/extractor/TextExtractorContract.java |  6 ++++++
 .../org/apache/james/mailbox/tika/CachingTextExtractor.java   | 11 -----------
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/extractor/TextExtractorContract.java b/mailbox/api/src/test/java/org/apache/james/mailbox/extractor/TextExtractorContract.java
new file mode 100644
index 0000000000..7fbce4131d
--- /dev/null
+++ b/mailbox/api/src/test/java/org/apache/james/mailbox/extractor/TextExtractorContract.java
@@ -0,0 +1,6 @@
+package org.apache.james.mailbox.extractor;
+
+import static org.junit.jupiter.api.Assertions.*;
+class TextExtractorContract {
+  
+}
\ No newline at end of file
diff --git a/mailbox/tika/src/main/java/org/apache/james/mailbox/tika/CachingTextExtractor.java b/mailbox/tika/src/main/java/org/apache/james/mailbox/tika/CachingTextExtractor.java
index 562fa60cc8..e41de811d1 100644
--- a/mailbox/tika/src/main/java/org/apache/james/mailbox/tika/CachingTextExtractor.java
+++ b/mailbox/tika/src/main/java/org/apache/james/mailbox/tika/CachingTextExtractor.java
@@ -132,15 +132,4 @@ public class CachingTextExtractor implements TextExtractor {
             .doOnNext(next -> weightMetric.add(computeWeight(next)));
     }
 
-    private Exception unwrap(Exception e) {
-        return Optional.ofNullable(e.getCause())
-            .filter(throwable -> throwable instanceof Exception)
-            .map(throwable -> (Exception) throwable)
-            .orElse(e);
-    }
-
-    @VisibleForTesting
-    long size() {
-        return cache.synchronous().estimatedSize();
-    }
 }


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