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 2017/08/08 10:46:45 UTC
[10/10] james-project git commit: JAMES-2048 We should not reject
blank CID already stored
JAMES-2048 We should not reject blank CID already stored
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/eaf503e6
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/eaf503e6
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/eaf503e6
Branch: refs/heads/master
Commit: eaf503e66fba243bf368df474a3276ebb656c216
Parents: 1046e4d
Author: benwa <bt...@linagora.com>
Authored: Thu Aug 3 18:09:20 2017 +0700
Committer: benwa <bt...@linagora.com>
Committed: Tue Aug 8 17:11:05 2017 +0700
----------------------------------------------------------------------
.../org/apache/james/mailbox/model/Cid.java | 118 +++++++--
.../org/apache/james/mailbox/model/CidTest.java | 250 ++++++++++++++++++-
.../cassandra/mail/CassandraMessageDAO.java | 6 +-
.../cassandra/mail/CassandraMessageDAOV2.java | 7 +-
.../store/mail/model/impl/MessageParser.java | 43 ++--
5 files changed, 380 insertions(+), 44 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/james-project/blob/eaf503e6/mailbox/api/src/main/java/org/apache/james/mailbox/model/Cid.java
----------------------------------------------------------------------
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/Cid.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/Cid.java
index 96440e5..612ee32 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/Cid.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/Cid.java
@@ -23,32 +23,120 @@ package org.apache.james.mailbox.model;
import org.apache.commons.lang.StringUtils;
import com.google.common.base.Objects;
+import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
public class Cid {
- public static Cid from(String cidAsString) {
- Preconditions.checkArgument(!StringUtils.isBlank(cidAsString), "'cidAsString' is mandatory");
- return new Cid(normalizedCid(cidAsString));
+ public static final StrictCidValidator DEFAULT_VALIDATOR = new StrictCidValidator();
+
+ public interface CidTransformation {
+ Optional<Cid> apply(CidValidator cidValidator, String value);
}
- private static String normalizedCid(String input) {
- if (isWrappedWithAngleBrackets(input)) {
- return unwrap(input);
+ public static class Identity implements CidTransformation {
+ @Override
+ public Optional<Cid> apply(CidValidator cidValidator, String value) {
+ return toCid(cidValidator, value);
}
- return input;
}
-
- private static String unwrap(String cidAsString) {
- String unwrapCid = cidAsString.substring(1, cidAsString.length() - 1);
- if (StringUtils.isBlank(unwrapCid)) {
- throw new IllegalArgumentException("'cidAsString' is mandatory");
+
+ public static class Unwrap implements CidTransformation {
+ @Override
+ public Optional<Cid> apply(CidValidator cidValidator, String value) {
+ cidValidator.validate(value);
+ if (isWrappedWithAngleBrackets(value)) {
+ return unwrap(value, cidValidator);
+ }
+ return toCid(cidValidator, value);
+ }
+
+
+ private Optional<Cid> unwrap(String cidAsString, CidValidator cidValidator) {
+ String unwrapCid = cidAsString.substring(1, cidAsString.length() - 1);
+ return toCid(cidValidator, unwrapCid);
+ }
+
+ private boolean isWrappedWithAngleBrackets(String cidAsString) {
+ return cidAsString != null
+ && cidAsString.startsWith("<")
+ && cidAsString.endsWith(">");
+ }
+ }
+
+ public interface CidValidator {
+ void validate(String cidValue);
+ }
+
+ public static class StrictCidValidator implements CidValidator {
+ @Override
+ public void validate(String cidValue) {
+ Preconditions.checkArgument(!Strings.isNullOrEmpty(cidValue));
+ Preconditions.checkArgument(!StringUtils.isBlank(cidValue), "'cidAsString' is mandatory");
+ }
+ }
+
+ public static class RelaxedCidValidator implements CidValidator {
+ @Override
+ public void validate(String cidValue) {
+
+ }
+ }
+
+ public static class CidParser {
+ private Optional<CidValidator> validator;
+ private Optional<CidTransformation> transformation;
+
+ private CidParser() {
+ validator = Optional.absent();
+ transformation = Optional.absent();
+ }
+
+ public CidParser relaxed() {
+ validator = Optional.<CidValidator>of(new RelaxedCidValidator());
+ return this;
+ }
+
+ public CidParser strict() {
+ validator = Optional.<CidValidator>of(new StrictCidValidator());
+ return this;
+ }
+
+ public CidParser unwrap() {
+ transformation = Optional.<CidTransformation>of(new Unwrap());
+ return this;
+ }
+
+ public Optional<Cid> parse(String value) {
+ CidValidator cidValidator = validator.or(DEFAULT_VALIDATOR);
+ CidTransformation cidTransformation = transformation.or(new Identity());
+ return cidTransformation.apply(cidValidator, value);
}
- return unwrapCid;
}
- private static boolean isWrappedWithAngleBrackets(String cidAsString) {
- return cidAsString.startsWith("<") && cidAsString.endsWith(">");
+ public static CidParser parser() {
+ return new CidParser();
+ }
+
+ public static Cid from(String value) {
+ return parser()
+ .strict()
+ .unwrap()
+ .parse(value)
+ .get();
+ }
+
+ private static Optional<Cid> toCid(CidValidator cidValidator, String value) {
+ cidValidator.validate(value);
+ return toCid(value);
+ }
+
+ private static Optional<Cid> toCid(String cidAsString) {
+ if (Strings.isNullOrEmpty(cidAsString) || StringUtils.isBlank(cidAsString)) {
+ return Optional.absent();
+ }
+ return Optional.of(new Cid(cidAsString));
}
private final String value;
http://git-wip-us.apache.org/repos/asf/james-project/blob/eaf503e6/mailbox/api/src/test/java/org/apache/james/mailbox/model/CidTest.java
----------------------------------------------------------------------
diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/model/CidTest.java b/mailbox/api/src/test/java/org/apache/james/mailbox/model/CidTest.java
index 20163ad..ef05548 100644
--- a/mailbox/api/src/test/java/org/apache/james/mailbox/model/CidTest.java
+++ b/mailbox/api/src/test/java/org/apache/james/mailbox/model/CidTest.java
@@ -20,24 +20,26 @@
package org.apache.james.mailbox.model;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.guava.api.Assertions.assertThat;
-import org.apache.james.mailbox.model.Cid;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
+import com.google.common.base.Optional;
+
import nl.jqno.equalsverifier.EqualsVerifier;
public class CidTest {
@Rule public ExpectedException expectedException = ExpectedException.none();
-
+
@Test
public void fromShouldThrowWhenNull() {
expectedException.expect(IllegalArgumentException.class);
Cid.from(null);
}
-
+
@Test
public void fromShouldThrowWhenEmpty() {
expectedException.expect(IllegalArgumentException.class);
@@ -67,19 +69,19 @@ public class CidTest {
Cid cid = Cid.from("<123>");
assertThat(cid.getValue()).isEqualTo("123");
}
-
+
@Test
public void fromShouldNotRemoveTagsWhenNone() {
Cid cid = Cid.from("123");
assertThat(cid.getValue()).isEqualTo("123");
}
-
+
@Test
public void fromShouldNotRemoveTagsWhenNotEndTag() {
Cid cid = Cid.from("<123");
assertThat(cid.getValue()).isEqualTo("<123");
}
-
+
@Test
public void fromShouldNotRemoveTagsWhenNotStartTag() {
Cid cid = Cid.from("123>");
@@ -87,6 +89,242 @@ public class CidTest {
}
@Test
+ public void fromRelaxedNoUnwrapShouldReturnAbsentWhenNull() {
+ assertThat(Cid.parser()
+ .relaxed()
+ .parse(null))
+ .isAbsent();
+ }
+
+ @Test
+ public void fromRelaxedNoUnwrapShouldReturnAbsentWhenEmpty() {
+ assertThat(Cid.parser()
+ .relaxed()
+ .parse(""))
+ .isAbsent();
+ }
+
+ @Test
+ public void fromRelaxedNoUnwrapShouldReturnAbsentWhenBlank() {
+ assertThat(Cid.parser()
+ .relaxed()
+ .parse(" "))
+ .isAbsent();
+ }
+
+ @Test
+ public void fromRelaxedNoUnwrapShouldReturnCidWhenEmptyAfterRemoveTags() {
+ Optional<Cid> actual = Cid.parser()
+ .relaxed()
+ .parse("<>");
+ assertThat(actual).isPresent();
+ assertThat(actual.get().getValue()).isEqualTo("<>");
+ }
+
+ @Test
+ public void fromRelaxedNoUnwrapShouldReturnCidWhenBlankAfterRemoveTags() {
+ Optional<Cid> actual = Cid.parser()
+ .relaxed()
+ .parse("< >");
+ assertThat(actual).isPresent();
+ assertThat(actual.get().getValue()).isEqualTo("< >");
+ }
+
+ @Test
+ public void fromRelaxedNoUnwrapShouldNotRemoveTagsWhenExists() {
+ Optional<Cid> actual = Cid.parser()
+ .relaxed()
+ .parse("<123>");
+ assertThat(actual).isPresent();
+ assertThat(actual.get().getValue()).isEqualTo("<123>");
+ }
+
+ @Test
+ public void fromRelaxedNoUnwrapShouldNotRemoveTagsWhenNone() {
+ assertThat(Cid.parser()
+ .relaxed()
+ .parse("123"))
+ .contains(Cid.from("123"));
+ }
+
+ @Test
+ public void fromRelaxedNoUnwrapShouldNotRemoveTagsWhenNotEndTag() {
+ assertThat(Cid.parser()
+ .relaxed()
+ .parse("<123"))
+ .contains(Cid.from("<123"));
+ }
+
+ @Test
+ public void fromRelaxedNoUnwrapShouldNotRemoveTagsWhenNotStartTag() {
+ assertThat(Cid.parser()
+ .relaxed()
+ .parse("123>"))
+ .contains(Cid.from("123>"));
+ }
+
+
+ @Test
+ public void fromRelaxedUnwrapShouldReturnAbsentWhenNull() {
+ assertThat(Cid.parser()
+ .relaxed()
+ .unwrap()
+ .parse(null))
+ .isAbsent();
+ }
+
+ @Test
+ public void fromRelaxedUnwrapShouldReturnAbsentWhenEmpty() {
+ assertThat(Cid.parser()
+ .relaxed()
+ .unwrap()
+ .parse(""))
+ .isAbsent();
+ }
+
+ @Test
+ public void fromRelaxedUnwrapShouldReturnAbsentWhenBlank() {
+ assertThat(Cid.parser()
+ .relaxed()
+ .unwrap()
+ .parse(" "))
+ .isAbsent();
+ }
+
+ @Test
+ public void fromRelaxedUnwrapShouldReturnAbsentWhenEmptyAfterRemoveTags() {
+ assertThat(Cid.parser()
+ .relaxed()
+ .unwrap()
+ .parse("<>"))
+ .isAbsent();
+ }
+
+ @Test
+ public void fromRelaxedUnwrapShouldReturnAbsentWhenBlankAfterRemoveTags() {
+ assertThat(Cid.parser()
+ .relaxed()
+ .unwrap()
+ .parse("< >"))
+ .isAbsent();
+ }
+
+ @Test
+ public void fromRelaxedUnwrapShouldRemoveTagsWhenExists() {
+ Optional<Cid> actual = Cid.parser()
+ .relaxed()
+ .unwrap()
+ .parse("<123>");
+ assertThat(actual).isPresent();
+ assertThat(actual.get().getValue()).isEqualTo("123");
+ }
+
+ @Test
+ public void fromRelaxedUnwrapShouldNotRemoveTagsWhenNone() {
+ assertThat(Cid.parser()
+ .relaxed()
+ .unwrap()
+ .parse("123"))
+ .contains(Cid.from("123"));
+ }
+
+ @Test
+ public void fromRelaxedUnwrapShouldNotRemoveTagsWhenNotEndTag() {
+ assertThat(Cid.parser()
+ .relaxed()
+ .unwrap()
+ .parse("<123"))
+ .contains(Cid.from("<123"));
+ }
+
+ @Test
+ public void fromRelaxedUnwrapShouldNotRemoveTagsWhenNotStartTag() {
+ assertThat(Cid.parser()
+ .relaxed()
+ .unwrap()
+ .parse("123>"))
+ .contains(Cid.from("123>"));
+ }
+
+ @Test
+ public void fromStrictNoUnwrapShouldThrowWhenNull() {
+ expectedException.expect(IllegalArgumentException.class);
+
+ Cid.parser()
+ .strict()
+ .parse(null);
+ }
+
+ @Test
+ public void fromStrictNoUnwrapShouldThrowWhenEmpty() {
+ expectedException.expect(IllegalArgumentException.class);
+
+ Cid.parser()
+ .strict()
+ .parse("");
+ }
+
+ @Test
+ public void fromStrinctNoUnwrapShouldThrowWhenBlank() {
+ expectedException.expect(IllegalArgumentException.class);
+
+ Cid.parser()
+ .strict()
+ .parse(" ");
+ }
+
+ @Test
+ public void fromStrictNoUnwrapShouldNotRemoveTagWhenEmptyAfterRemoveTags() {
+ Optional<Cid> actual = Cid.parser()
+ .strict()
+ .parse("<>");
+ assertThat(actual).isPresent();
+ assertThat(actual.get().getValue()).isEqualTo("<>");
+ }
+
+ @Test
+ public void fromStrictNoUnwrapShouldNotRemoveTagWhenBlankAfterRemoveTags() {
+ Optional<Cid> actual = Cid.parser()
+ .strict()
+ .parse("< >");
+ assertThat(actual).isPresent();
+ assertThat(actual.get().getValue()).isEqualTo("< >");
+ }
+
+ @Test
+ public void fromStrictNoUnwrapShouldNotRemoveTagsWhenExists() {
+ Optional<Cid> actual = Cid.parser()
+ .strict()
+ .parse("<123>");
+ assertThat(actual).isPresent();
+ assertThat(actual.get().getValue()).isEqualTo("<123>");
+ }
+
+ @Test
+ public void fromStrictNoUnwrapShouldNotRemoveTagsWhenNone() {
+ assertThat(Cid.parser()
+ .strict()
+ .parse("123"))
+ .contains(Cid.from("123"));
+ }
+
+ @Test
+ public void fromStrictNoUnwrapShouldNotRemoveTagsWhenNotEndTag() {
+ assertThat(Cid.parser()
+ .strict()
+ .parse("<123"))
+ .contains(Cid.from("<123"));
+ }
+
+ @Test
+ public void fromStrictNoUnwrapShouldNotRemoveTagsWhenNotStartTag() {
+ assertThat(Cid.parser()
+ .strict()
+ .parse("123>"))
+ .contains(Cid.from("123>"));
+ }
+
+ @Test
public void shouldRespectJavaBeanContract() {
EqualsVerifier.forClass(Cid.class).verify();
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/eaf503e6/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAO.java
----------------------------------------------------------------------
diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAO.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAO.java
index 1c73063..35c6a3f 100644
--- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAO.java
+++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAO.java
@@ -74,6 +74,7 @@ import org.apache.james.mailbox.store.mail.model.impl.PropertyBuilder;
import org.apache.james.mailbox.store.mail.model.impl.SimpleProperty;
import org.apache.james.util.CompletableFutureUtil;
import org.apache.james.util.FluentFutureStream;
+import org.apache.james.util.OptionalConverter;
import org.apache.james.util.streams.JamesCollectors;
import com.datastax.driver.core.BoundStatement;
@@ -100,6 +101,7 @@ public class CassandraMessageDAO {
private final PreparedStatement selectByBatch;
private CassandraUtils cassandraUtils;
private final CassandraConfiguration cassandraConfiguration;
+ private final Cid.CidParser cidParser;
@Inject
public CassandraMessageDAO(Session session, CassandraTypesProvider typesProvider, CassandraConfiguration cassandraConfiguration,
@@ -115,6 +117,7 @@ public class CassandraMessageDAO {
this.cassandraConfiguration = cassandraConfiguration;
this.selectByBatch = prepareSelectBatch(session, cassandraConfiguration);
this.cassandraUtils = cassandraUtils;
+ this.cidParser = Cid.parser().relaxed();
}
@VisibleForTesting
@@ -283,7 +286,8 @@ public class CassandraMessageDAO {
return MessageAttachmentRepresentation.builder()
.attachmentId(AttachmentId.from(udtValue.getString(Attachments.ID)))
.name(udtValue.getString(Attachments.NAME))
- .cid(Optional.ofNullable(udtValue.getString(Attachments.CID)).map(Cid::from))
+ .cid(OptionalConverter.fromGuava(
+ cidParser.parse(udtValue.getString(Attachments.CID))))
.isInline(udtValue.getBool(Attachments.IS_INLINE))
.build();
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/eaf503e6/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAOV2.java
----------------------------------------------------------------------
diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAOV2.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAOV2.java
index 6664028..b44bd8c 100644
--- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAOV2.java
+++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraMessageDAOV2.java
@@ -59,6 +59,7 @@ import org.apache.james.backends.cassandra.utils.CassandraAsyncExecutor;
import org.apache.james.mailbox.cassandra.ids.BlobId;
import org.apache.james.mailbox.cassandra.ids.CassandraMessageId;
import org.apache.james.mailbox.cassandra.mail.utils.Limit;
+import org.apache.james.mailbox.cassandra.table.CassandraMessageV1Table;
import org.apache.james.mailbox.cassandra.table.CassandraMessageV2Table.Attachments;
import org.apache.james.mailbox.cassandra.table.CassandraMessageV2Table.Properties;
import org.apache.james.mailbox.exception.MailboxException;
@@ -74,6 +75,7 @@ import org.apache.james.mailbox.store.mail.model.impl.PropertyBuilder;
import org.apache.james.mailbox.store.mail.model.impl.SimpleProperty;
import org.apache.james.util.CompletableFutureUtil;
import org.apache.james.util.FluentFutureStream;
+import org.apache.james.util.OptionalConverter;
import org.apache.james.util.streams.JamesCollectors;
import com.datastax.driver.core.BoundStatement;
@@ -103,6 +105,7 @@ public class CassandraMessageDAOV2 {
private final PreparedStatement selectHeaders;
private final PreparedStatement selectFields;
private final PreparedStatement selectBody;
+ private final Cid.CidParser cidParser;
@Inject
public CassandraMessageDAOV2(Session session, CassandraTypesProvider typesProvider, CassandraBlobsDAO blobsDAO, CassandraConfiguration cassandraConfiguration) {
@@ -116,6 +119,7 @@ public class CassandraMessageDAOV2 {
this.selectHeaders = prepareSelect(session, HEADERS);
this.selectFields = prepareSelect(session, FIELDS);
this.selectBody = prepareSelect(session, BODY);
+ this.cidParser = Cid.parser().relaxed();
}
@VisibleForTesting
@@ -340,7 +344,8 @@ public class CassandraMessageDAOV2 {
return MessageAttachmentRepresentation.builder()
.attachmentId(AttachmentId.from(udtValue.getString(Attachments.ID)))
.name(udtValue.getString(Attachments.NAME))
- .cid(Optional.ofNullable(udtValue.getString(Attachments.CID)).map(Cid::from))
+ .cid(OptionalConverter.fromGuava(
+ cidParser.parse(udtValue.getString(CassandraMessageV1Table.Attachments.CID))))
.isInline(udtValue.getBool(Attachments.IS_INLINE))
.build();
}
http://git-wip-us.apache.org/repos/asf/james-project/blob/eaf503e6/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/MessageParser.java
----------------------------------------------------------------------
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/MessageParser.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/MessageParser.java
index 3e7e42e..62bf74e 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/MessageParser.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/MessageParser.java
@@ -69,25 +69,14 @@ public class MessageParser {
ContentDispositionField.DISPOSITION_TYPE_ATTACHMENT.toLowerCase(Locale.US),
ContentDispositionField.DISPOSITION_TYPE_INLINE.toLowerCase(Locale.US));
private static final Logger LOGGER = LoggerFactory.getLogger(MessageParser.class);
- private static final Function<String, Optional<Cid>> STRING_TO_CID_FUNCTION = new Function<String, Optional<Cid>>() {
- @Override
- public Optional<Cid> apply(String cid) {
- try {
- return Optional.of(Cid.from(cid));
- } catch (IllegalArgumentException e) {
- return Optional.absent();
- }
- }
- };
-
- private static final Function<ContentIdField, Optional<Cid>> CONTENT_ID_FIELD_TO_OPTIONAL_CID_FUNCTION = new Function<ContentIdField, Optional<Cid>>() {
- @Override
- public Optional<Cid> apply(ContentIdField field) {
- return Optional.fromNullable(field.getId())
- .transform(STRING_TO_CID_FUNCTION)
- .or(Optional.<Cid>absent());
- }
- };
+
+ private final Cid.CidParser cidParser;
+
+ public MessageParser() {
+ cidParser = Cid.parser()
+ .relaxed()
+ .unwrap();
+ }
public List<MessageAttachment> retrieveAttachments(InputStream fullContent) throws MimeException, IOException {
DefaultMessageBuilder defaultMessageBuilder = new DefaultMessageBuilder();
@@ -197,8 +186,20 @@ public class MessageParser {
}
private Optional<Cid> cid(Optional<ContentIdField> contentIdField) {
- return contentIdField.transform(CONTENT_ID_FIELD_TO_OPTIONAL_CID_FUNCTION)
- .or(Optional.<Cid>absent());
+ if (!contentIdField.isPresent()) {
+ return Optional.absent();
+ }
+ return contentIdField.transform(toCid())
+ .get();
+ }
+
+ private Function<ContentIdField, Optional<Cid>> toCid() {
+ return new Function<ContentIdField, Optional<Cid>>() {
+ @Override
+ public Optional<Cid> apply(ContentIdField input) {
+ return cidParser.parse(input.getId());
+ }
+ };
}
private boolean isMultipart(Entity entity) {
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org