You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2020/04/22 02:31:49 UTC
[james-project] 09/37: JAMES-2997 step #7 Do not rely on properties
to determine if a message has attachment
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 93b7d28b0f82903f4ddc081620e77143f3c30a78
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Jan 17 11:07:30 2020 +0700
JAMES-2997 step #7 Do not rely on properties to determine if a message has attachment
Use attachment metadata directly
---
.../james/mailbox/model/MessageAttachment.java | 6 +++
.../elasticsearch/json/IndexableMessage.java | 13 +-----
.../elasticsearch/json/IndexableMessageTest.java | 53 ++++++----------------
.../lucene/search/LuceneMessageSearchIndex.java | 5 +-
.../james/mailbox/store/StoreMessageManager.java | 6 ---
.../store/mail/model/impl/PropertyBuilder.java | 14 ------
.../mailbox/store/search/MessageSearches.java | 5 +-
.../store/mail/model/impl/PropertyBuilderTest.java | 18 ++------
8 files changed, 31 insertions(+), 89 deletions(-)
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MessageAttachment.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MessageAttachment.java
index 9dda554..fc37b03 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MessageAttachment.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MessageAttachment.java
@@ -19,6 +19,7 @@
package org.apache.james.mailbox.model;
+import java.util.List;
import java.util.Optional;
import com.google.common.base.MoreObjects;
@@ -77,6 +78,11 @@ public class MessageAttachment {
}
}
+ public static boolean hasNonInlinedAttachment(List<MessageAttachment> attachments) {
+ return attachments.stream()
+ .anyMatch(messageAttachment -> !messageAttachment.isInlinedWithCid());
+ }
+
private final Attachment attachment;
private final Optional<String> name;
private final Optional<Cid> cid;
diff --git a/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/json/IndexableMessage.java b/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/json/IndexableMessage.java
index 40d2b1c..1248996 100644
--- a/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/json/IndexableMessage.java
+++ b/mailbox/elasticsearch/src/main/java/org/apache/james/mailbox/elasticsearch/json/IndexableMessage.java
@@ -32,9 +32,8 @@ import org.apache.james.mailbox.ModSeq;
import org.apache.james.mailbox.elasticsearch.IndexAttachments;
import org.apache.james.mailbox.elasticsearch.query.DateResolutionFormater;
import org.apache.james.mailbox.extractor.TextExtractor;
+import org.apache.james.mailbox.model.MessageAttachment;
import org.apache.james.mailbox.store.mail.model.MailboxMessage;
-import org.apache.james.mailbox.store.mail.model.Property;
-import org.apache.james.mailbox.store.mail.model.impl.PropertyBuilder;
import org.apache.james.mailbox.store.search.SearchUtil;
import org.apache.james.mime4j.MimeException;
@@ -98,12 +97,6 @@ public class IndexableMessage {
return this;
}
- private boolean computeHasAttachment(MailboxMessage message) {
- return message.getProperties()
- .stream()
- .anyMatch(property -> property.equals(HAS_ATTACHMENT_PROPERTY));
- }
-
private IndexableMessage instantiateIndexedMessage() throws IOException, MimeException {
String messageId = SearchUtil.getSerializedMessageIdIfSupportedByUnderlyingStorageOrNull(message);
MimePart parsingResult = new MimePartParser(message, textExtractor).parse();
@@ -111,7 +104,7 @@ public class IndexableMessage {
Optional<String> bodyText = parsingResult.locateFirstTextBody();
Optional<String> bodyHtml = parsingResult.locateFirstHtmlBody();
- boolean hasAttachment = computeHasAttachment(message);
+ boolean hasAttachment = MessageAttachment.hasNonInlinedAttachment(message.getAttachments());
List<MimePart> attachments = setFlattenedAttachments(parsingResult, indexAttachments);
HeaderCollection headerCollection = parsingResult.getHeaderCollection();
@@ -192,8 +185,6 @@ public class IndexableMessage {
}
}
- public static final Property HAS_ATTACHMENT_PROPERTY = new Property(PropertyBuilder.JAMES_INTERNALS, PropertyBuilder.HAS_ATTACHMENT, "true");
-
public static Builder builder() {
return new Builder();
}
diff --git a/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/json/IndexableMessageTest.java b/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/json/IndexableMessageTest.java
index 2526f14..694e521 100644
--- a/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/json/IndexableMessageTest.java
+++ b/mailbox/elasticsearch/src/test/java/org/apache/james/mailbox/elasticsearch/json/IndexableMessageTest.java
@@ -37,12 +37,13 @@ import org.apache.james.mailbox.elasticsearch.IndexAttachments;
import org.apache.james.mailbox.extractor.ParsedContent;
import org.apache.james.mailbox.extractor.TextExtractor;
import org.apache.james.mailbox.inmemory.InMemoryMessageId;
+import org.apache.james.mailbox.model.Attachment;
+import org.apache.james.mailbox.model.AttachmentId;
+import org.apache.james.mailbox.model.MessageAttachment;
import org.apache.james.mailbox.model.MessageId;
import org.apache.james.mailbox.model.TestId;
import org.apache.james.mailbox.store.extractor.DefaultTextExtractor;
import org.apache.james.mailbox.store.mail.model.MailboxMessage;
-import org.apache.james.mailbox.store.mail.model.Property;
-import org.apache.james.mailbox.store.mail.model.impl.PropertyBuilder;
import org.apache.james.mailbox.tika.TikaConfiguration;
import org.apache.james.mailbox.tika.TikaExtension;
import org.apache.james.mailbox.tika.TikaHttpClientImpl;
@@ -306,7 +307,7 @@ class IndexableMessageTest {
}
@Test
- void hasAttachmentsShouldReturnTrueWhenPropertyIsPresentAndTrue() throws IOException {
+ void hasAttachmentsShouldReturnTrueWhenNonInlined() throws IOException {
//Given
MailboxMessage mailboxMessage = mock(MailboxMessage.class);
TestId mailboxId = TestId.of(1);
@@ -322,7 +323,15 @@ class IndexableMessageTest {
.thenReturn(new Flags());
when(mailboxMessage.getUid())
.thenReturn(MESSAGE_UID);
- when(mailboxMessage.getProperties()).thenReturn(ImmutableList.of(IndexableMessage.HAS_ATTACHMENT_PROPERTY));
+ when(mailboxMessage.getAttachments())
+ .thenReturn(ImmutableList.of(MessageAttachment.builder()
+ .attachment(Attachment.builder()
+ .attachmentId(AttachmentId.from("1"))
+ .type("text/plain")
+ .size(36)
+ .build())
+ .isInline(false)
+ .build()));
// When
IndexableMessage indexableMessage = IndexableMessage.builder()
@@ -337,7 +346,7 @@ class IndexableMessageTest {
}
@Test
- void hasAttachmentsShouldReturnFalseWhenPropertyIsPresentButFalse() throws IOException {
+ void hasAttachmentsShouldReturnFalseWhenEmptyAttachments() throws IOException {
//Given
MailboxMessage mailboxMessage = mock(MailboxMessage.class);
TestId mailboxId = TestId.of(1);
@@ -353,39 +362,7 @@ class IndexableMessageTest {
.thenReturn(MESSAGE_UID);
when(mailboxMessage.getModSeq())
.thenReturn(ModSeq.first());
- when(mailboxMessage.getProperties())
- .thenReturn(ImmutableList.of(new Property(PropertyBuilder.JAMES_INTERNALS, PropertyBuilder.HAS_ATTACHMENT, "false")));
-
- // When
- IndexableMessage indexableMessage = IndexableMessage.builder()
- .message(mailboxMessage)
- .extractor(new DefaultTextExtractor())
- .zoneId(ZoneId.of("Europe/Paris"))
- .indexAttachments(IndexAttachments.NO)
- .build();
-
- // Then
- assertThat(indexableMessage.getHasAttachment()).isFalse();
- }
-
- @Test
- void hasAttachmentsShouldReturnFalseWhenPropertyIsAbsent() throws IOException {
- //Given
- MailboxMessage mailboxMessage = mock(MailboxMessage.class);
- TestId mailboxId = TestId.of(1);
- when(mailboxMessage.getMailboxId())
- .thenReturn(mailboxId);
- when(mailboxMessage.getModSeq())
- .thenReturn(ModSeq.first());
- when(mailboxMessage.getMessageId())
- .thenReturn(InMemoryMessageId.of(42));
- when(mailboxMessage.getFullContent())
- .thenReturn(ClassLoader.getSystemResourceAsStream("eml/mailWithHeaders.eml"));
- when(mailboxMessage.createFlags())
- .thenReturn(new Flags());
- when(mailboxMessage.getUid())
- .thenReturn(MESSAGE_UID);
- when(mailboxMessage.getProperties())
+ when(mailboxMessage.getAttachments())
.thenReturn(ImmutableList.of());
// When
diff --git a/mailbox/lucene/src/main/java/org/apache/james/mailbox/lucene/search/LuceneMessageSearchIndex.java b/mailbox/lucene/src/main/java/org/apache/james/mailbox/lucene/search/LuceneMessageSearchIndex.java
index 793f4a0..30f1bce 100644
--- a/mailbox/lucene/src/main/java/org/apache/james/mailbox/lucene/search/LuceneMessageSearchIndex.java
+++ b/mailbox/lucene/src/main/java/org/apache/james/mailbox/lucene/search/LuceneMessageSearchIndex.java
@@ -51,6 +51,7 @@ import org.apache.james.mailbox.exception.MailboxException;
import org.apache.james.mailbox.exception.UnsupportedSearchException;
import org.apache.james.mailbox.model.Mailbox;
import org.apache.james.mailbox.model.MailboxId;
+import org.apache.james.mailbox.model.MessageAttachment;
import org.apache.james.mailbox.model.MessageId;
import org.apache.james.mailbox.model.MessageRange;
import org.apache.james.mailbox.model.SearchQuery;
@@ -70,7 +71,6 @@ import org.apache.james.mailbox.model.SearchQuery.UidRange;
import org.apache.james.mailbox.model.UpdatedFlags;
import org.apache.james.mailbox.store.MailboxSessionMapperFactory;
import org.apache.james.mailbox.store.mail.model.MailboxMessage;
-import org.apache.james.mailbox.store.mail.model.impl.PropertyBuilder;
import org.apache.james.mailbox.store.search.ListeningMessageSearchIndex;
import org.apache.james.mailbox.store.search.SearchUtil;
import org.apache.james.mime4j.MimeException;
@@ -732,8 +732,7 @@ public class LuceneMessageSearchIndex extends ListeningMessageSearchIndex {
}
private static boolean hasAttachment(MailboxMessage membership) {
- return membership.getProperties().stream()
- .anyMatch(PropertyBuilder.isHasAttachmentProperty());
+ return MessageAttachment.hasNonInlinedAttachment(membership.getAttachments());
}
private String toSentDateField(DateResolution res) {
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageManager.java
index f5e18c7..1eb89a7 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageManager.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageManager.java
@@ -447,7 +447,6 @@ public class StoreMessageManager implements MessageManager {
final int size = (int) file.length();
final List<MessageAttachment> attachments = extractAttachments(contentIn);
- propertyBuilder.setHasAttachment(hasNonInlinedAttachment(attachments));
final MailboxMessage message = createMessage(internalDate, size, bodyStartOctet, contentIn, flags, propertyBuilder, attachments);
@@ -501,11 +500,6 @@ public class StoreMessageManager implements MessageManager {
return propertyBuilder;
}
- private boolean hasNonInlinedAttachment(List<MessageAttachment> attachments) {
- return attachments.stream()
- .anyMatch(messageAttachment -> !messageAttachment.isInlinedWithCid());
- }
-
private List<MessageAttachment> extractAttachments(SharedFileInputStream contentIn) {
try {
return messageParser.retrieveAttachments(contentIn);
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/PropertyBuilder.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/PropertyBuilder.java
index 90d5192..1f53595 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/PropertyBuilder.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/PropertyBuilder.java
@@ -46,7 +46,6 @@ import java.util.Locale;
import java.util.Map;
import java.util.SortedMap;
import java.util.TreeMap;
-import java.util.function.Predicate;
import org.apache.james.mailbox.store.mail.model.Property;
@@ -56,16 +55,7 @@ import com.github.steveash.guavate.Guavate;
* Builds properties
*/
public class PropertyBuilder {
-
private static final int INITIAL_CAPACITY = 32;
- public static final String JAMES_INTERNALS = "JAMES_INTERNALS";
- public static final String HAS_ATTACHMENT = "HAS_ATTACHMENT";
-
- public static Predicate<Property> isHasAttachmentProperty() {
- return property -> property.getNamespace().equals(PropertyBuilder.JAMES_INTERNALS)
- && property.getLocalName().equals(PropertyBuilder.HAS_ATTACHMENT)
- && property.getValue().equals("true");
- }
private Long textualLineCount;
private final List<Property> properties;
@@ -207,10 +197,6 @@ public class PropertyBuilder {
setProperty(MIME_MIME_TYPE_SPACE, MIME_MEDIA_TYPE_NAME, value);
}
- public void setHasAttachment(boolean value) {
- setProperty(JAMES_INTERNALS, HAS_ATTACHMENT, Boolean.toString(value));
- }
-
/**
* Gets the MIME content subtype.
*
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/search/MessageSearches.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/search/MessageSearches.java
index 03915fb..07d18c6 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/search/MessageSearches.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/search/MessageSearches.java
@@ -55,7 +55,6 @@ import org.apache.james.mailbox.model.SearchQuery.DateResolution;
import org.apache.james.mailbox.model.SearchQuery.UidRange;
import org.apache.james.mailbox.store.ResultUtils;
import org.apache.james.mailbox.store.mail.model.MailboxMessage;
-import org.apache.james.mailbox.store.mail.model.impl.PropertyBuilder;
import org.apache.james.mailbox.store.search.comparator.CombinedComparator;
import org.apache.james.mime4j.MimeException;
import org.apache.james.mime4j.MimeIOException;
@@ -548,9 +547,7 @@ public class MessageSearches implements Iterable<SimpleMessageSearchIndex.Search
private boolean matches(SearchQuery.AttachmentCriterion criterion, MailboxMessage message) throws UnsupportedSearchException {
- boolean mailHasAttachments = message.getProperties()
- .stream()
- .anyMatch(PropertyBuilder.isHasAttachmentProperty());
+ boolean mailHasAttachments = MessageAttachment.hasNonInlinedAttachment(message.getAttachments());
return mailHasAttachments == criterion.getOperator().isSet();
}
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/impl/PropertyBuilderTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/impl/PropertyBuilderTest.java
index 1317441..1e10873 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/impl/PropertyBuilderTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/impl/PropertyBuilderTest.java
@@ -19,32 +19,24 @@
package org.apache.james.mailbox.store.mail.model.impl;
+import static org.apache.james.mailbox.store.mail.model.StandardNames.MIME_CONTENT_MD5_NAME;
+import static org.apache.james.mailbox.store.mail.model.StandardNames.MIME_CONTENT_MD5_SPACE;
import static org.assertj.core.api.Assertions.assertThat;
import org.apache.james.mailbox.store.mail.model.Property;
import org.junit.jupiter.api.Test;
class PropertyBuilderTest {
-
@Test
void emptyPropertyBuilderShouldCreateEmptyProperties() {
assertThat(new PropertyBuilder().toProperties()).isEmpty();
}
@Test
- void setHasAttachmentShouldAddFalseWhenCalledWithFalse() {
+ void setContentMD5ShouldAddMd5Property() {
PropertyBuilder propertyBuilder = new PropertyBuilder();
- propertyBuilder.setHasAttachment(false);
+ propertyBuilder.setContentMD5("123");
assertThat(propertyBuilder.toProperties())
- .containsOnly(new Property(PropertyBuilder.JAMES_INTERNALS, PropertyBuilder.HAS_ATTACHMENT, "false"));
+ .containsOnly(new Property(MIME_CONTENT_MD5_SPACE, MIME_CONTENT_MD5_NAME, "123"));
}
-
- @Test
- void setHasAttachmentShouldAddTrueWhenCalledWithTrue() {
- PropertyBuilder propertyBuilder = new PropertyBuilder();
- propertyBuilder.setHasAttachment(true);
- assertThat(propertyBuilder.toProperties())
- .containsOnly(new Property(PropertyBuilder.JAMES_INTERNALS, PropertyBuilder.HAS_ATTACHMENT, "true"));
- }
-
}
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org