This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 4b35308eae4ac772f824e2a762564d44f393e5d1 Author: Benoit Tellier <[email protected]> AuthorDate: Thu Oct 8 08:58:36 2020 +0700 JAMES-3416 ElasticSearch address indexing fixes [email protected] was matching with [email protected] --- CHANGELOG.md | 1 + .../james/mailbox/elasticsearch/json/EMailer.java | 7 +- .../elasticsearch/json/HeaderCollection.java | 3 +- .../ElasticSearchIntegrationTest.java | 33 ++++++++++ .../mailbox/elasticsearch/json/EMailersTest.java | 10 +-- .../elasticsearch/json/HeaderCollectionTest.java | 53 +++++++-------- mailbox/store/src/test/resources/eml/htmlMail.json | 2 +- .../store/src/test/resources/eml/nonTextual.json | 2 +- .../src/test/resources/eml/pgpSignedMail.json | 2 +- .../src/test/resources/eml/recursiveMail.json | 2 +- .../eml/recursiveMailWithoutAttachments.json | 2 +- mailbox/store/src/test/resources/eml/spamMail.json | 2 +- .../contract/EmailQueryMethodContract.scala | 77 ++-------------------- 13 files changed, 78 insertions(+), 118 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72e2314..7f87ace 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ Use BlobStore cache instead. ### Fixed - JAMES-3305 Avoid crashes upon deserialization issues when consuming RabbitMQ messages, leverage dead-letter feature - JAMES-3212 JMAP Handle subcrible/unsubcrible child's folder when update mailbox +- JAMES-3416 Fix ElasticSearch email address search ### Removed - HybridBlobStore. This will be removed after 3.6.0 release. Introduced to fasten small blob access, its usage could be diff --git a/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/json/EMailer.java b/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/json/EMailer.java index e0a8d86..1860a69 100644 --- a/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/json/EMailer.java +++ b/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/json/EMailer.java @@ -20,6 +20,7 @@ package org.apache.james.mailbox.elasticsearch.json; import java.util.Objects; +import java.util.Optional; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Joiner; @@ -27,16 +28,16 @@ import com.google.common.base.MoreObjects; public class EMailer implements SerializableMessage { - private final String name; + private final Optional<String> name; private final String address; - public EMailer(String name, String address) { + public EMailer(Optional<String> name, String address) { this.name = name; this.address = address; } @JsonProperty(JsonMessageConstants.EMailer.NAME) - public String getName() { + public Optional<String> getName() { return name; } diff --git a/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/json/HeaderCollection.java b/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/json/HeaderCollection.java index ffadfa0..0f4ca17 100644 --- a/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/json/HeaderCollection.java +++ b/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/json/HeaderCollection.java @@ -27,7 +27,6 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Stream; -import org.apache.james.mailbox.store.search.SearchUtil; import org.apache.james.mailbox.store.search.comparator.SentDateComparator; import org.apache.james.mime4j.dom.address.Address; import org.apache.james.mime4j.dom.address.Group; @@ -155,7 +154,7 @@ public class HeaderCollection { .parseAddressList(rawHeaderValue) .stream() .flatMap(this::convertAddressToMailboxStream) - .map((mailbox) -> new EMailer(SearchUtil.getDisplayAddress(mailbox), mailbox.getAddress())) + .map((mailbox) -> new EMailer(Optional.ofNullable(mailbox.getName()), mailbox.getAddress())) .forEach(addressSet::add); } diff --git a/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/ElasticSearchIntegrationTest.java b/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/ElasticSearchIntegrationTest.java index ed1e22a..043a3dc 100644 --- a/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/ElasticSearchIntegrationTest.java +++ b/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/ElasticSearchIntegrationTest.java @@ -49,6 +49,7 @@ import org.apache.james.mailbox.tika.TikaHttpClientImpl; import org.apache.james.mailbox.tika.TikaTextExtractor; import org.apache.james.metrics.tests.RecordingMetricFactory; import org.apache.james.mime4j.dom.Message; +import org.apache.james.mime4j.stream.RawField; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -259,4 +260,36 @@ class ElasticSearchIntegrationTest extends AbstractMessageSearchIndexTest { assertThat(messageManager.search(SearchQuery.of(SearchQuery.all()), session)) .contains(customStringHeaderMessageId.getUid()); } + + @Test + void addressMatchesShouldBeExact() throws Exception { + // results should not include the domain part nor the local part but the full email address + MailboxPath mailboxPath = MailboxPath.forUser(USERNAME, INBOX); + MailboxSession session = MailboxSessionUtil.create(USERNAME); + MessageManager messageManager = storeMailboxManager.getMailbox(mailboxPath, session); + + Message.Builder messageBuilder = Message.Builder + .of() + .setSubject("test") + .setBody("testmail", StandardCharsets.UTF_8); + + ComposedMessageId messageId1 = messageManager.appendMessage( + MessageManager.AppendCommand.builder().build( + messageBuilder + .addField(new RawField("To", "[email protected]")) + .build()), + session).getId(); + + ComposedMessageId messageId2 = messageManager.appendMessage( + MessageManager.AppendCommand.builder().build( + messageBuilder + .addField(new RawField("To", "[email protected]")) + .build()), + session).getId(); + + elasticSearch.awaitForElasticSearch(); + + assertThat(messageManager.search(SearchQuery.of(SearchQuery.address(SearchQuery.AddressType.To, "[email protected]")), session)) + .containsOnly(messageId2.getUid()); + } } \ No newline at end of file diff --git a/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/json/EMailersTest.java b/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/json/EMailersTest.java index 885e3bc..31e957e 100644 --- a/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/json/EMailersTest.java +++ b/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/json/EMailersTest.java @@ -22,6 +22,8 @@ package org.apache.james.mailbox.elasticsearch.json; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import java.util.Optional; + import org.junit.jupiter.api.Test; import com.google.common.base.Joiner; @@ -45,7 +47,7 @@ class EMailersTest { @Test void serializeShouldNotJoinWhenOneElement() { - EMailer emailer = new EMailer("name", "address"); + EMailer emailer = new EMailer(Optional.of("name"), "address"); EMailers eMailers = EMailers.from(ImmutableSet.of(emailer)); assertThat(eMailers.serialize()).isEqualTo(emailer.serialize()); @@ -53,9 +55,9 @@ class EMailersTest { @Test void serializeShouldJoinWhenMultipleElements() { - EMailer emailer = new EMailer("name", "address"); - EMailer emailer2 = new EMailer("name2", "address2"); - EMailer emailer3 = new EMailer("name3", "address3"); + EMailer emailer = new EMailer(Optional.of("name"), "address"); + EMailer emailer2 = new EMailer(Optional.of("name2"), "address2"); + EMailer emailer3 = new EMailer(Optional.of("name3"), "address3"); String expected = Joiner.on(" ").join(emailer.serialize(), emailer2.serialize(), emailer3.serialize()); diff --git a/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/json/HeaderCollectionTest.java b/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/json/HeaderCollectionTest.java index 250e00a..f7aec4e 100644 --- a/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/json/HeaderCollectionTest.java +++ b/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/json/HeaderCollectionTest.java @@ -23,6 +23,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.time.format.DateTimeFormatter; +import java.util.Optional; import java.util.stream.Stream; import org.junit.jupiter.api.Test; @@ -55,7 +56,7 @@ class HeaderCollectionTest { .build(); assertThat(headerCollection.getToAddressSet()) - .containsOnly(new EMailer("[email protected]", "[email protected]")); + .containsOnly(new EMailer(Optional.empty(), "[email protected]")); } @Test @@ -66,8 +67,8 @@ class HeaderCollectionTest { assertThat(headerCollection.getToAddressSet()) .containsOnly( - new EMailer("[email protected]", "[email protected]"), - new EMailer("[email protected]", "[email protected]")); + new EMailer(Optional.empty(), "[email protected]"), + new EMailer(Optional.empty(), "[email protected]")); } @Test @@ -79,8 +80,8 @@ class HeaderCollectionTest { assertThat(headerCollection.getToAddressSet()) .containsOnly( - new EMailer("[email protected]", "[email protected]"), - new EMailer("[email protected]", "[email protected]")); + new EMailer(Optional.empty(), "[email protected]"), + new EMailer(Optional.empty(), "[email protected]")); } @Test @@ -90,7 +91,7 @@ class HeaderCollectionTest { .build(); assertThat(headerCollection.getToAddressSet()) - .containsOnly(new EMailer("Christophe Hamerling", "[email protected]")); + .containsOnly(new EMailer(Optional.of("Christophe Hamerling"), "[email protected]")); } @ParameterizedTest @@ -102,7 +103,7 @@ class HeaderCollectionTest { assertThat(headerCollection.getFromAddressSet()) .extracting(EMailer::getName) - .contains(nameOfFromAddress); + .contains(Optional.ofNullable(nameOfFromAddress)); } @Test @@ -133,8 +134,8 @@ class HeaderCollectionTest { .build(); assertThat(headerCollection.getFromAddressSet()) - .containsOnly(new EMailer("Christophe Hamerling", "[email protected]"), - new EMailer("Graham CROSMARIE", "[email protected]")); + .containsOnly(new EMailer(Optional.of("Christophe Hamerling"), "[email protected]"), + new EMailer(Optional.of("Graham CROSMARIE"), "[email protected]")); } @Test @@ -145,8 +146,8 @@ class HeaderCollectionTest { .build(); assertThat(headerCollection.getFromAddressSet()) - .containsOnly(new EMailer("Christophe Hamerling", "[email protected]"), - new EMailer("Graham CROSMARIE", "[email protected]")); + .containsOnly(new EMailer(Optional.of("Christophe Hamerling"), "[email protected]"), + new EMailer(Optional.of("Graham CROSMARIE"), "[email protected]")); } @Test @@ -169,8 +170,8 @@ class HeaderCollectionTest { .build(); assertThat(headerCollection.getToAddressSet()) - .containsOnly(new EMailer("Christophe Hamerling", "[email protected]"), - new EMailer("[email protected]", "[email protected]")); + .containsOnly(new EMailer(Optional.of("Christophe Hamerling"), "[email protected]"), + new EMailer(Optional.empty(), "[email protected]")); } @Test @@ -180,7 +181,7 @@ class HeaderCollectionTest { .build(); assertThat(headerCollection.getCcAddressSet()) - .containsOnly(new EMailer("Christophe Hamerling", "[email protected]")); + .containsOnly(new EMailer(Optional.of("Christophe Hamerling"), "[email protected]")); } @Test @@ -190,7 +191,7 @@ class HeaderCollectionTest { .build(); assertThat(headerCollection.getReplyToAddressSet()) - .containsOnly(new EMailer("Christophe Hamerling", "[email protected]")); + .containsOnly(new EMailer(Optional.of("Christophe Hamerling"), "[email protected]")); } @Test @@ -200,17 +201,7 @@ class HeaderCollectionTest { .build(); assertThat(headerCollection.getBccAddressSet()) - .containsOnly(new EMailer("Christophe Hamerling", "[email protected]")); - } - - @Test - void headerContaingNoAddressShouldBeConsideredBothAsNameAndAddress() { - HeaderCollection headerCollection = HeaderCollection.builder() - .add(new FieldImpl("Bcc", "Not an address")) - .build(); - - assertThat(headerCollection.getBccAddressSet()) - .containsOnly(new EMailer("Not an address", "Not an address")); + .containsOnly(new EMailer(Optional.of("Christophe Hamerling"), "[email protected]")); } @Test @@ -220,7 +211,7 @@ class HeaderCollectionTest { .build(); assertThat(headerCollection.getBccAddressSet()) - .containsOnly(new EMailer("Mickey", "[email protected]")); + .containsOnly(new EMailer(Optional.of("Mickey"), "[email protected]")); } @Test @@ -230,8 +221,8 @@ class HeaderCollectionTest { .build(); assertThat(headerCollection.getBccAddressSet()) - .containsOnly(new EMailer("Mickey", "[email protected]"), - new EMailer("Miny", "[email protected]")); + .containsOnly(new EMailer(Optional.of("Mickey"), "[email protected]"), + new EMailer(Optional.of("Miny"), "[email protected]")); } @Test @@ -241,8 +232,8 @@ class HeaderCollectionTest { .build(); assertThat(headerCollection.getBccAddressSet()) - .containsOnly(new EMailer("Mickey", "[email protected]"), - new EMailer("Miny", "[email protected]")); + .containsOnly(new EMailer(Optional.of("Mickey"), "[email protected]"), + new EMailer(Optional.of("Miny"), "[email protected]")); } @Test diff --git a/mailbox/store/src/test/resources/eml/htmlMail.json b/mailbox/store/src/test/resources/eml/htmlMail.json index 326664b..45445d2 100644 --- a/mailbox/store/src/test/resources/eml/htmlMail.json +++ b/mailbox/store/src/test/resources/eml/htmlMail.json @@ -105,7 +105,7 @@ ], "to":[ { - "name":"[email protected]", + "name":null, "address":"[email protected]" } ], diff --git a/mailbox/store/src/test/resources/eml/nonTextual.json b/mailbox/store/src/test/resources/eml/nonTextual.json index 48d5d15..121434e 100644 --- a/mailbox/store/src/test/resources/eml/nonTextual.json +++ b/mailbox/store/src/test/resources/eml/nonTextual.json @@ -68,7 +68,7 @@ "size":25, "subject":["Test message"], "subtype":"text", - "to":[{"name":"[email protected]","address":"[email protected]"}], + "to":[{"name":null,"address":"[email protected]"}], "uid":25, "userFlags":[], "mimeMessageID":"<[email protected]>", diff --git a/mailbox/store/src/test/resources/eml/pgpSignedMail.json b/mailbox/store/src/test/resources/eml/pgpSignedMail.json index f5673d5..32538b7 100644 --- a/mailbox/store/src/test/resources/eml/pgpSignedMail.json +++ b/mailbox/store/src/test/resources/eml/pgpSignedMail.json @@ -150,7 +150,7 @@ ], "to": [ { - "name": "[email protected]", + "name": null, "address": "[email protected]" } ], diff --git a/mailbox/store/src/test/resources/eml/recursiveMail.json b/mailbox/store/src/test/resources/eml/recursiveMail.json index ba6edc4..e59b266 100644 --- a/mailbox/store/src/test/resources/eml/recursiveMail.json +++ b/mailbox/store/src/test/resources/eml/recursiveMail.json @@ -66,7 +66,7 @@ ], "to": [ { - "name": "[email protected]", + "name": null, "address": "[email protected]" } ], diff --git a/mailbox/store/src/test/resources/eml/recursiveMailWithoutAttachments.json b/mailbox/store/src/test/resources/eml/recursiveMailWithoutAttachments.json index b0f7c5d..7c0daa2 100644 --- a/mailbox/store/src/test/resources/eml/recursiveMailWithoutAttachments.json +++ b/mailbox/store/src/test/resources/eml/recursiveMailWithoutAttachments.json @@ -66,7 +66,7 @@ ], "to": [ { - "name": "[email protected]", + "name": null, "address": "[email protected]" } ], diff --git a/mailbox/store/src/test/resources/eml/spamMail.json b/mailbox/store/src/test/resources/eml/spamMail.json index 626881a..7216c1f 100644 --- a/mailbox/store/src/test/resources/eml/spamMail.json +++ b/mailbox/store/src/test/resources/eml/spamMail.json @@ -105,7 +105,7 @@ ], "to": [ { - "name": "[email protected]", + "name": null, "address": "[email protected]" } ], diff --git a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailQueryMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailQueryMethodContract.scala index 33c53b3..de4683f 100644 --- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailQueryMethodContract.scala +++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/EmailQueryMethodContract.scala @@ -4829,7 +4829,7 @@ trait EmailQueryMethodContract { .of .setSubject("a mail") .setFrom("[email protected]") - .setFrom("[email protected]") + .setFrom("[email protected]") .setBody("lorem ipsum", StandardCharsets.UTF_8) .build)) .getMessageId @@ -4839,7 +4839,7 @@ trait EmailQueryMethodContract { .of .setSubject("another mail") .setTo("[email protected]") - .setTo("[email protected]") + .setTo("[email protected]") .setBody("lorem ipsum", StandardCharsets.UTF_8) .build)) .getMessageId @@ -4848,7 +4848,7 @@ trait EmailQueryMethodContract { .appendMessage(BOB.asString, inbox(BOB), AppendCommand.from(Message.Builder .of .setSubject("another mail") - .addField(new RawField("Cc", "<[email protected]>, <[email protected]>")) + .addField(new RawField("Cc", "<[email protected]>, <[email protected]>")) .setBody("lorem ipsum", StandardCharsets.UTF_8) .build)) .getMessageId @@ -4857,15 +4857,7 @@ trait EmailQueryMethodContract { .appendMessage(BOB.asString, inbox(BOB), AppendCommand.from(Message.Builder .of .setSubject("another mail") - .addField(new RawField("Bcc", "<[email protected]>, <[email protected]>")) - .setBody("lorem ipsum", StandardCharsets.UTF_8) - .build)) - .getMessageId - - val messageId5: MessageId = mailboxProbe - .appendMessage(BOB.asString, inbox(BOB), AppendCommand.from(Message.Builder - .of - .setSubject("Subject with test word") + .addField(new RawField("Bcc", "<[email protected]>, <[email protected]>")) .setBody("lorem ipsum", StandardCharsets.UTF_8) .build)) .getMessageId @@ -4888,7 +4880,7 @@ trait EmailQueryMethodContract { | { | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6", | "filter": { - | "text": "test" + | "text": "[email protected]" | } | }, | "c1"]] @@ -4912,7 +4904,6 @@ trait EmailQueryMethodContract { .inPath("$.methodResponses[0][1].ids") .isEqualTo( s"""[ - | "${messageId5.serialize}", | "${messageId4.serialize}", | "${messageId3.serialize}", | "${messageId2.serialize}", @@ -4979,64 +4970,6 @@ trait EmailQueryMethodContract { } @Test - def textShouldMatchFromField(server: GuiceJamesServer): Unit = { - server.getProbe(classOf[MailboxProbeImpl]).createMailbox(inbox(BOB)) - val messageId1: MessageId = server.getProbe(classOf[MailboxProbeImpl]) - .appendMessage(BOB.asString, inbox(BOB), AppendCommand.from(Message.Builder - .of - .addField(new RawField("From", "[email protected]")) - .setSubject("a mail") - .setBody("This is a test body", StandardCharsets.UTF_8) - .build)) - .getMessageId - - server.getProbe(classOf[MailboxProbeImpl]) - .appendMessage(BOB.asString, inbox(BOB), AppendCommand.from(Message.Builder - .of - .setSubject("should not be found mail") - .setBody("lorem ipsum", StandardCharsets.UTF_8) - .build)) - .getMessageId - - val request = - s"""{ - | "using": [ - | "urn:ietf:params:jmap:core", - | "urn:ietf:params:jmap:mail"], - | "methodCalls": [[ - | "Email/query", - | { - | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6", - | "filter": { - | "text": "[email protected]" - | } - | }, - | "c1"]] - |}""".stripMargin - - awaitAtMostTenSeconds.untilAsserted { () => - val response = `given` - .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER) - .body(request) - .when - .post - .`then` - .statusCode(SC_OK) - .contentType(JSON) - .extract - .body - .asString - - assertThatJson(response) - .inPath("$.methodResponses[0][1].ids") - .isEqualTo( - s"""[ - | "${messageId1.serialize}" - |]""".stripMargin) - } - } - - @Test def emailQueryShouldSupportTextFilterForHtmlBody(server: GuiceJamesServer): Unit = { server.getProbe(classOf[MailboxProbeImpl]).createMailbox(inbox(BOB)) val messageId1: MessageId = server.getProbe(classOf[MailboxProbeImpl]) --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
