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/03 12:32:14 UTC
[james-project] branch refactorings-2 created (now 956db1063b)
This is an automated email from the ASF dual-hosted git repository.
matthieu pushed a change to branch refactorings-2
in repository https://gitbox.apache.org/repos/asf/james-project.git
at 956db1063b Refactor text extraction in a more monadic style and avoid building unnecessary `InputStream`s
This branch includes the following new commits:
new b41f9a97ad Avoid String comparison when you can match domain types
new b4f55a5157 Reword a comment about DefaultTextExtractor
new 7cc6df02e8 Provide factory methods to simplify ParsedContent instantiations
new cc65d9e7cc Ensure `TextExtractor`s `applicable` method handle null correctly
new 956db1063b Refactor text extraction in a more monadic style and avoid building unnecessary `InputStream`s
The 5 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] 03/05: Provide factory methods to simplify ParsedContent instantiations
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
matthieu pushed a commit to branch refactorings-2
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 7cc6df02e8a589d9c64bcdb4e9f6eb38da14be01
Author: Matthieu Baechler <ma...@baechler-craftsmanship.fr>
AuthorDate: Fri Feb 3 09:12:49 2023 +0100
Provide factory methods to simplify ParsedContent instantiations
---
.../james/mailbox/extractor/ParsedContent.java | 21 ++++++++++++++++-----
.../james/mailbox/opensearch/json/MimePart.java | 5 +----
.../opensearch/json/IndexableMessageTest.java | 4 ++--
.../mailbox/store/search/PDFTextExtractor.java | 7 ++-----
.../store/extractor/DefaultTextExtractor.java | 10 ++++------
.../mailbox/store/extractor/JsoupTextExtractor.java | 10 ++--------
.../james/mailbox/tika/TikaTextExtractor.java | 2 +-
.../mailbox/tika/CachingTextExtractorTest.java | 4 ++--
8 files changed, 30 insertions(+), 33 deletions(-)
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/extractor/ParsedContent.java b/mailbox/api/src/main/java/org/apache/james/mailbox/extractor/ParsedContent.java
index 4be66463f3..080bc42d0a 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/extractor/ParsedContent.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/extractor/ParsedContent.java
@@ -27,11 +27,26 @@ import java.util.Optional;
import com.google.common.collect.ImmutableMap;
public class ParsedContent {
+ public static ParsedContent empty() {
+ return new ParsedContent(Optional.empty(), ImmutableMap.of());
+ }
+
+ public static ParsedContent of(String textualContent) {
+ return new ParsedContent(Optional.of(textualContent), ImmutableMap.of());
+ }
+
+ public static ParsedContent of(Optional<String> textualContent) {
+ return new ParsedContent(textualContent, ImmutableMap.of());
+ }
+
+ public static ParsedContent of(Optional<String> textualContent, Map<String, List<String>> metadata) {
+ return new ParsedContent(textualContent, metadata);
+ }
private final Optional<String> textualContent;
private final Map<String, List<String>> metadata;
- public ParsedContent(Optional<String> textualContent, Map<String, List<String>> metadata) {
+ private ParsedContent(Optional<String> textualContent, Map<String, List<String>> metadata) {
this.textualContent = textualContent;
this.metadata = metadata;
}
@@ -44,10 +59,6 @@ public class ParsedContent {
return metadata;
}
- public static ParsedContent empty() {
- return new ParsedContent(Optional.empty(), ImmutableMap.of());
- }
-
@Override
public final boolean equals(Object o) {
if (o instanceof ParsedContent) {
diff --git a/mailbox/opensearch/src/main/java/org/apache/james/mailbox/opensearch/json/MimePart.java b/mailbox/opensearch/src/main/java/org/apache/james/mailbox/opensearch/json/MimePart.java
index 8adf2d8d45..bce4c80cbe 100644
--- a/mailbox/opensearch/src/main/java/org/apache/james/mailbox/opensearch/json/MimePart.java
+++ b/mailbox/opensearch/src/main/java/org/apache/james/mailbox/opensearch/json/MimePart.java
@@ -42,7 +42,6 @@ import org.slf4j.LoggerFactory;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.github.fge.lambdas.Throwing;
-import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import reactor.core.publisher.Flux;
@@ -205,9 +204,7 @@ public class MimePart {
new ByteArrayInputStream(bodyContent.get()),
contentType.orElse(null));
}
- return Mono.fromCallable(() -> new ParsedContent(
- Optional.ofNullable(IOUtils.toString(new ByteArrayInputStream(bodyContent.get()), charset.orElse(StandardCharsets.UTF_8))),
- ImmutableMap.of()));
+ return Mono.fromCallable(() -> ParsedContent.of(IOUtils.toString(new ByteArrayInputStream(bodyContent.get()), charset.orElse(StandardCharsets.UTF_8))));
}
private boolean shouldPerformTextExtraction() {
diff --git a/mailbox/opensearch/src/test/java/org/apache/james/mailbox/opensearch/json/IndexableMessageTest.java b/mailbox/opensearch/src/test/java/org/apache/james/mailbox/opensearch/json/IndexableMessageTest.java
index 97e48b19f9..b477c69f49 100644
--- a/mailbox/opensearch/src/test/java/org/apache/james/mailbox/opensearch/json/IndexableMessageTest.java
+++ b/mailbox/opensearch/src/test/java/org/apache/james/mailbox/opensearch/json/IndexableMessageTest.java
@@ -236,9 +236,9 @@ class IndexableMessageTest {
TextExtractor textExtractor = mock(TextExtractor.class);
when(textExtractor.applicable(any())).thenReturn(true);
when(textExtractor.extractContentReactive(any(), any()))
- .thenReturn(Mono.just(new ParsedContent(Optional.of("first attachment content"), ImmutableMap.of())))
+ .thenReturn(Mono.just(ParsedContent.of("first attachment content")))
.thenReturn(Mono.error(new RuntimeException("second cannot be parsed")))
- .thenReturn(Mono.just(new ParsedContent(Optional.of("third attachment content"), ImmutableMap.of())));
+ .thenReturn(Mono.just(ParsedContent.of("third attachment content")));
// When
IndexableMessage indexableMessage = IndexableMessage.builder()
diff --git a/mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/PDFTextExtractor.java b/mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/PDFTextExtractor.java
index 8f82685165..0149246809 100644
--- a/mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/PDFTextExtractor.java
+++ b/mailbox/scanning-search/src/test/java/org/apache/james/mailbox/store/search/PDFTextExtractor.java
@@ -46,7 +46,7 @@ public class PDFTextExtractor implements TextExtractor {
if (isPDF(contentType)) {
return extractTextFromPDF(inputStream);
}
- return new ParsedContent(Optional.ofNullable(IOUtils.toString(inputStream, StandardCharsets.UTF_8)), ImmutableMap.of());
+ return ParsedContent.of(IOUtils.toString(inputStream, StandardCharsets.UTF_8));
}
private boolean isPDF(ContentType contentType) {
@@ -54,9 +54,6 @@ public class PDFTextExtractor implements TextExtractor {
}
private ParsedContent extractTextFromPDF(InputStream inputStream) throws IOException {
- return new ParsedContent(
- Optional.ofNullable(new PDFTextStripper().getText(
- PDDocument.load(inputStream))),
- ImmutableMap.of());
+ return ParsedContent.of(new PDFTextStripper().getText(PDDocument.load(inputStream)));
}
}
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 2bf97902d7..b710879541 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
@@ -22,8 +22,6 @@ package org.apache.james.mailbox.store.extractor;
import java.io.InputStream;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
-import java.util.HashMap;
-import java.util.Optional;
import org.apache.commons.io.IOUtils;
import org.apache.james.mailbox.extractor.ParsedContent;
@@ -51,9 +49,9 @@ public class DefaultTextExtractor implements TextExtractor {
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<>());
+ return ParsedContent.of(IOUtils.toString(input, charset));
} else {
- return new ParsedContent(Optional.empty(), new HashMap<>());
+ return ParsedContent.empty();
}
}
}
@@ -63,11 +61,11 @@ public class DefaultTextExtractor implements TextExtractor {
if (applicable(contentType)) {
Charset charset = contentType.charset().orElse(StandardCharsets.UTF_8);
return Mono.using(() -> inputStream,
- stream -> Mono.fromCallable(() -> new ParsedContent(Optional.ofNullable(IOUtils.toString(stream, charset)), new HashMap<>()))
+ stream -> Mono.fromCallable(() -> ParsedContent.of(IOUtils.toString(stream, charset)))
.subscribeOn(Schedulers.boundedElastic()),
Throwing.consumer(InputStream::close).orDoNothing());
} else {
- return Mono.just(new ParsedContent(Optional.empty(), new HashMap<>()));
+ return Mono.just(ParsedContent.empty());
}
}
}
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/extractor/JsoupTextExtractor.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/extractor/JsoupTextExtractor.java
index 01281e04cb..282d4499c6 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/extractor/JsoupTextExtractor.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/extractor/JsoupTextExtractor.java
@@ -23,9 +23,6 @@ import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
import org.apache.commons.io.IOUtils;
import org.apache.james.mailbox.extractor.ParsedContent;
@@ -35,15 +32,12 @@ import org.apache.james.mailbox.model.ContentType.MimeType;
import org.jsoup.Jsoup;
import org.jsoup.nodes.Document;
-import com.google.common.collect.ImmutableMap;
-
import reactor.core.publisher.Mono;
import reactor.core.scheduler.Schedulers;
public class JsoupTextExtractor implements TextExtractor {
private static final String TITLE_HTML_TAG = "title";
private static final String NO_BASE_URI = "";
- private static final Map<String, List<String>> EMPTY_METADATA = ImmutableMap.of();
private static final MimeType TEXT_HTML = MimeType.of("text/html");
private static final MimeType TEXT_PLAIN = MimeType.of("text/plain");
@@ -88,12 +82,12 @@ public class JsoupTextExtractor implements TextExtractor {
}
private ParsedContent parsePlainTextContent(InputStream inputStream, Charset charset) throws IOException {
- return new ParsedContent(Optional.ofNullable(IOUtils.toString(inputStream, charset)), EMPTY_METADATA);
+ return ParsedContent.of(IOUtils.toString(inputStream, charset));
}
private ParsedContent parseHtmlContent(InputStream inputStream, Charset charset) throws IOException {
Document doc = Jsoup.parse(inputStream, charset.name(), NO_BASE_URI);
doc.select(TITLE_HTML_TAG).remove();
- return new ParsedContent(Optional.ofNullable(doc.text()), EMPTY_METADATA);
+ return ParsedContent.of(doc.text());
}
}
diff --git a/mailbox/tika/src/main/java/org/apache/james/mailbox/tika/TikaTextExtractor.java b/mailbox/tika/src/main/java/org/apache/james/mailbox/tika/TikaTextExtractor.java
index 8c639096b7..0c1e833cf9 100644
--- a/mailbox/tika/src/main/java/org/apache/james/mailbox/tika/TikaTextExtractor.java
+++ b/mailbox/tika/src/main/java/org/apache/james/mailbox/tika/TikaTextExtractor.java
@@ -96,7 +96,7 @@ public class TikaTextExtractor implements TextExtractor {
public Mono<ParsedContent> performContentExtraction(InputStream inputStream, ContentType contentType) {
Mono<ContentAndMetadata> contentAndMetadata = convert(tikaHttpClient.recursiveMetaDataAsJson(inputStream, contentType));
return contentAndMetadata
- .map(result -> new ParsedContent(result.getContent(), result.getMetadata()));
+ .map(result -> ParsedContent.of(result.getContent(), result.getMetadata()));
}
private Mono<ContentAndMetadata> convert(Mono<InputStream> maybeInputStream) {
diff --git a/mailbox/tika/src/test/java/org/apache/james/mailbox/tika/CachingTextExtractorTest.java b/mailbox/tika/src/test/java/org/apache/james/mailbox/tika/CachingTextExtractorTest.java
index e402edf027..494a80e896 100644
--- a/mailbox/tika/src/test/java/org/apache/james/mailbox/tika/CachingTextExtractorTest.java
+++ b/mailbox/tika/src/test/java/org/apache/james/mailbox/tika/CachingTextExtractorTest.java
@@ -56,9 +56,9 @@ import reactor.core.publisher.Mono;
class CachingTextExtractorTest {
- private static final ParsedContent RESULT = new ParsedContent(Optional.of("content"), ImmutableMap.of());
+ private static final ParsedContent RESULT = ParsedContent.of("content");
public static final String BIG_STRING = Strings.repeat("0123456789", 103 * 1024);
- private static final ParsedContent _2MiB_RESULT = new ParsedContent(Optional.of(BIG_STRING), ImmutableMap.of());
+ private static final ParsedContent _2MiB_RESULT = ParsedContent.of(BIG_STRING);
private static final Function<Integer, InputStream> STREAM_GENERATOR =
i -> new ByteArrayInputStream(String.format("content%d", i).getBytes(StandardCharsets.UTF_8));
private static final Supplier<InputStream> INPUT_STREAM = () -> STREAM_GENERATOR.apply(1);
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[james-project] 01/05: Avoid String comparison when you can match domain types
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
matthieu pushed a commit to branch refactorings-2
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit b41f9a97adf152fed33d7729ef5b70ccd9e5bec9
Author: Matthieu Baechler <ma...@baechler-craftsmanship.fr>
AuthorDate: Fri Feb 3 08:53:20 2023 +0100
Avoid String comparison when you can match domain types
---
.../api/src/main/java/org/apache/james/mailbox/model/ContentType.java | 3 +++
.../org/apache/james/mailbox/store/extractor/DefaultTextExtractor.java | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/ContentType.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/ContentType.java
index fa7bf33543..63b25424e9 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/ContentType.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/ContentType.java
@@ -99,6 +99,9 @@ public class ContentType {
}
public static class MediaType {
+
+ public static MediaType TEXT = MediaType.of("text");
+
public static MediaType of(String value) {
Preconditions.checkState(!Strings.isNullOrEmpty(value), "'media type' is mandatory");
return new MediaType(value);
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 180f263a43..f42ced3a20 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
@@ -43,7 +43,7 @@ import reactor.core.scheduler.Schedulers;
public class DefaultTextExtractor implements TextExtractor {
@Override
public boolean applicable(ContentType contentType) {
- return contentType != null && contentType.asString().startsWith("text/");
+ return contentType != null && contentType.mediaType().equals(ContentType.MediaType.TEXT);
}
@Override
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[james-project] 04/05: Ensure `TextExtractor`s `applicable` method handle null correctly
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
matthieu pushed a commit to branch refactorings-2
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit cc65d9e7ccd5a3607a3e4b8afe8ce9904ff7e905
Author: Matthieu Baechler <ma...@baechler-craftsmanship.fr>
AuthorDate: Fri Feb 3 09:13:21 2023 +0100
Ensure `TextExtractor`s `applicable` method handle null correctly
---
.../org/apache/james/mailbox/extractor/TextExtractorContract.java | 6 ++++++
1 file changed, 6 insertions(+)
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 2820744e14..01ad138416 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
@@ -25,6 +25,7 @@ import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.catchException;
import static org.mockito.Mockito.*;
@@ -74,4 +75,9 @@ public interface TextExtractorContract {
verify(stream).close();
}
+
+ @Test
+ default void applicableShouldReturnFalseOnNull() {
+ assertThat(testee().applicable(null)).isFalse();
+ }
}
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[james-project] 05/05: Refactor text extraction in a more monadic style and avoid building unnecessary `InputStream`s
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
matthieu pushed a commit to branch refactorings-2
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 956db1063b3113eba08b533ba6fd92be26a3fda3
Author: Matthieu Baechler <ma...@baechler-craftsmanship.fr>
AuthorDate: Fri Feb 3 09:19:00 2023 +0100
Refactor text extraction in a more monadic style and avoid building unnecessary `InputStream`s
---
.../james/mailbox/opensearch/json/MimePart.java | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/mailbox/opensearch/src/main/java/org/apache/james/mailbox/opensearch/json/MimePart.java b/mailbox/opensearch/src/main/java/org/apache/james/mailbox/opensearch/json/MimePart.java
index bce4c80cbe..be9c1a3fdc 100644
--- a/mailbox/opensearch/src/main/java/org/apache/james/mailbox/opensearch/json/MimePart.java
+++ b/mailbox/opensearch/src/main/java/org/apache/james/mailbox/opensearch/json/MimePart.java
@@ -196,15 +196,15 @@ public class MimePart {
}
private Mono<ParsedContent> extractText(TextExtractor textExtractor) {
- if (bodyContent.isEmpty()) {
- return Mono.empty();
- }
- if (shouldPerformTextExtraction()) {
- return textExtractor.extractContentReactive(
- new ByteArrayInputStream(bodyContent.get()),
- contentType.orElse(null));
- }
- return Mono.fromCallable(() -> ParsedContent.of(IOUtils.toString(new ByteArrayInputStream(bodyContent.get()), charset.orElse(StandardCharsets.UTF_8))));
+ return Mono.justOrEmpty(bodyContent)
+ .flatMap(content -> {
+ if (shouldPerformTextExtraction()) {
+ return textExtractor.extractContentReactive(
+ new ByteArrayInputStream(content),
+ contentType.orElse(null));
+ }
+ return Mono.fromCallable(() -> ParsedContent.of(new String(content, charset.orElse(StandardCharsets.UTF_8))));
+ });
}
private boolean shouldPerformTextExtraction() {
@@ -220,7 +220,7 @@ public class MimePart {
}
}
-
+
public static Builder builder(Predicate<ContentType> shouldCaryOverContent) {
return new Builder(shouldCaryOverContent);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[james-project] 02/05: Reword a comment about DefaultTextExtractor
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
matthieu pushed a commit to branch refactorings-2
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit b4f55a5157e8781e922646a7b70bedfa5ddc02a7
Author: Matthieu Baechler <ma...@baechler-craftsmanship.fr>
AuthorDate: Fri Feb 3 08:54:12 2023 +0100
Reword a comment about DefaultTextExtractor
---
.../org/apache/james/mailbox/store/extractor/DefaultTextExtractor.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 f42ced3a20..2bf97902d7 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
@@ -38,7 +38,7 @@ import reactor.core.scheduler.Schedulers;
/**
* A default text extractor that is directly based on the input file provided.
*
- * Costs less calculations that TikaTextExtractor, but result is not that good.
+ * Costs fewer calculations than TikaTextExtractor, but result is not that good.
*/
public class DefaultTextExtractor implements TextExtractor {
@Override
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org