j-hellenberg commented on code in PR #2588:
URL: https://github.com/apache/james-project/pull/2588#discussion_r1906933688
##########
mailbox/api/src/test/java/org/apache/james/mailbox/model/MailboxPathTest.java:
##########
@@ -30,427 +30,498 @@
import
org.apache.james.mailbox.exception.HasEmptyMailboxNameInHierarchyException;
import org.apache.james.mailbox.exception.MailboxNameException;
import org.apache.james.mailbox.exception.TooLongMailboxNameException;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
+import com.google.common.base.CharMatcher;
import com.google.common.base.Strings;
import nl.jqno.equalsverifier.EqualsVerifier;
class MailboxPathTest {
- private static final Username USER = Username.of("user");
- private static final Username BUGGY_USER = Username.of("buggy:bob");
-
- @Test
- void shouldMatchBeanContract() {
- EqualsVerifier.forClass(MailboxPath.class)
- .withNonnullFields("namespace")
- .verify();
- }
-
- static Stream<Arguments> parseShouldYieldCorrectResults() {
- return Stream.of(
- Arguments.of(MailboxPath.forUser(USER, "test")),
- Arguments.of(MailboxPath.forUser(USER, "a:b")),
- Arguments.of(MailboxPath.forUser(USER, "a;b")),
- Arguments.of(MailboxPath.forUser(USER, "a;;b")),
- Arguments.of(MailboxPath.forUser(USER, "a:b:c:")),
- Arguments.of(MailboxPath.forUser(USER, "a:b:c:")),
- Arguments.of(MailboxPath.forUser(USER, ":")),
- Arguments.of(MailboxPath.forUser(USER, ";")),
- Arguments.of(MailboxPath.forUser(USER, "")),
- Arguments.of(MailboxPath.inbox(USER)),
- Arguments.of(MailboxPath.inbox(Username.of("a;b"))),
- Arguments.of(MailboxPath.inbox(Username.of(";"))),
- Arguments.of(MailboxPath.inbox(Username.of(":"))),
- Arguments.of(MailboxPath.inbox(Username.of("a;;a"))),
- Arguments.of(MailboxPath.inbox(Username.of("a:::;:::;:;;;a"))),
- Arguments.of(MailboxPath.inbox(Username.of("a::a"))),
- Arguments.of(MailboxPath.inbox(Username.of("a/a"))),
- Arguments.of(MailboxPath.inbox(Username.of("a//a"))),
- Arguments.of(MailboxPath.inbox(Username.of("a/:a"))),
- Arguments.of(MailboxPath.inbox(Username.of("a/;a"))),
-
Arguments.of(MailboxPath.inbox(Username.of("a/:::;;/:://:;//:/a"))),
- Arguments.of(MailboxPath.inbox(BUGGY_USER)),
- Arguments.of(new MailboxPath("#whatever", USER, "whatever")),
- Arguments.of(new MailboxPath(null, USER, "whatever")));
- }
-
- @ParameterizedTest
- @MethodSource
- void parseShouldYieldCorrectResults(MailboxPath mailboxPath) {
- assertThat(MailboxPath.parseEscaped(mailboxPath.asEscapedString()))
- .contains(mailboxPath);
- }
-
- @Test
- void asStringShouldFormatUser() {
- assertThat(MailboxPath.forUser(USER,
"inbox.folder.subfolder").asString())
- .isEqualTo("#private:user:inbox.folder.subfolder");
- }
-
- @Test
- void getNameShouldReturnSubfolder() {
- assertThat(MailboxPath.forUser(USER,
"inbox.folder.subfolder").getName('.'))
- .isEqualTo("subfolder");
- }
-
- @Test
- void getNameShouldNoopWhenNoDelimiter() {
- assertThat(MailboxPath.forUser(USER, "name").getName('.'))
- .isEqualTo("name");
- }
-
- @Test
- void getNameShouldNoopWhenEmpty() {
- assertThat(MailboxPath.forUser(USER, "").getName('.'))
- .isEqualTo("");
- }
-
- @Test
- void getNameShouldNoopWhenBlank() {
- assertThat(MailboxPath.forUser(USER, " ").getName('.'))
- .isEqualTo(" ");
- }
-
- @Test
- void getHierarchyLevelsShouldBeOrdered() {
- assertThat(MailboxPath.forUser(USER, "inbox.folder.subfolder")
- .getHierarchyLevels('.'))
- .containsExactly(
- MailboxPath.forUser(USER, "inbox"),
- MailboxPath.forUser(USER, "inbox.folder"),
- MailboxPath.forUser(USER, "inbox.folder.subfolder"));
- }
-
- @Test
- void childShouldConcatenateChildNameWithParentForlder() {
- assertThat(MailboxPath.forUser(USER, "folder")
- .child("toto", '.'))
- .isEqualTo(MailboxPath.forUser(USER, "folder.toto"));
- }
-
- @Test
- void childShouldThrowWhenNull() {
- MailboxPath path = MailboxPath.forUser(USER, "folder");
- assertThatThrownBy(() -> path.child(null, '.'))
- .isInstanceOf(IllegalArgumentException.class);
- }
-
- @Test
- void childShouldThrowWhenEmpty() {
- MailboxPath path = MailboxPath.forUser(USER, "folder");
- assertThatThrownBy(() -> path.child("", '.'))
- .isInstanceOf(IllegalArgumentException.class);
- }
-
- @Test
- void shouldThrowWhenLineBreak() {
- assertThatThrownBy(() -> MailboxPath.forUser(USER, "a\r\n [ALERT]
that's bad").assertAcceptable('.'))
- .isInstanceOf(MailboxNameException.class);
- }
-
- @Test
- void childShouldThrowWhenBlank() {
- MailboxPath path = MailboxPath.forUser(USER, "folder");
- assertThatThrownBy(() -> path.child(" ", '.'))
- .isInstanceOf(IllegalArgumentException.class);
- }
-
- @Test
- void childShouldThrowWhenContainsDelimiter() {
- MailboxPath path = MailboxPath.forUser(USER, "folder");
- assertThatThrownBy(() -> path.child("a.b", '.'))
- .isInstanceOf(IllegalArgumentException.class);
- }
-
- @Test
- void getHierarchyLevelsShouldReturnPathWhenOneLevel() {
- assertThat(MailboxPath.forUser(USER, "inbox")
- .getHierarchyLevels('.'))
- .containsExactly(
- MailboxPath.forUser(USER, "inbox"));
- }
-
- @Test
- void getHierarchyLevelsShouldReturnPathWhenEmptyName() {
- assertThat(MailboxPath.forUser(USER, "")
- .getHierarchyLevels('.'))
- .containsExactly(
- MailboxPath.forUser(USER, ""));
- }
-
- @Test
- void getHierarchyLevelsShouldReturnPathWhenBlankName() {
- assertThat(MailboxPath.forUser(USER, " ")
- .getHierarchyLevels('.'))
- .containsExactly(
- MailboxPath.forUser(USER, " "));
- }
-
- @Test
- void getHierarchyLevelsShouldReturnPathWhenNullName() {
- assertThat(MailboxPath.forUser(USER, null)
- .getHierarchyLevels('.'))
- .containsExactly(
- MailboxPath.forUser(USER, null));
- }
-
- @Test
- void sanitizeShouldNotThrowOnNullMailboxName() {
- assertThat(MailboxPath.forUser(USER, null)
- .sanitize('.'))
- .isEqualTo(
- MailboxPath.forUser(USER, null));
- }
-
- @Test
- void sanitizeShouldReturnEmptyWhenEmpty() {
- assertThat(MailboxPath.forUser(USER, "")
- .sanitize('.'))
- .isEqualTo(
- MailboxPath.forUser(USER, ""));
- }
-
- @Test
- void sanitizeShouldRemoveMaximumOneTrailingDelimiterWhenAlone() {
- assertThat(MailboxPath.forUser(USER, ".")
- .sanitize('.'))
- .isEqualTo(
- MailboxPath.forUser(USER, ""));
- }
-
- @Test
- void sanitizeShouldPreserveHeadingDelimiter() {
- assertThat(MailboxPath.forUser(USER, ".a")
- .sanitize('.'))
- .isEqualTo(
- MailboxPath.forUser(USER, ".a"));
- }
-
- @Test
- void sanitizeShouldRemoveTrailingDelimiter() {
- assertThat(MailboxPath.forUser(USER, "a.")
- .sanitize('.'))
- .isEqualTo(
- MailboxPath.forUser(USER, "a"));
- }
-
- @Test
- void sanitizeShouldRemoveMaximumOneTrailingDelimiter() {
- assertThat(MailboxPath.forUser(USER, "a..")
- .sanitize('.'))
- .isEqualTo(
- MailboxPath.forUser(USER, "a."));
- }
-
- @Test
- void sanitizeShouldPreserveRedundantDelimiters() {
- assertThat(MailboxPath.forUser(USER, "a..a")
- .sanitize('.'))
- .isEqualTo(
- MailboxPath.forUser(USER, "a..a"));
- }
-
- @Test
- void hasEmptyNameInHierarchyShouldBeFalseIfSingleLevelPath() {
- assertThat(MailboxPath.forUser(USER, "a")
- .hasEmptyNameInHierarchy('.'))
- .isFalse();
- }
-
- @Test
- void hasEmptyNameInHierarchyShouldBeFalseIfNestedLevelWithNonEmptyNames() {
- assertThat(MailboxPath.forUser(USER, "a.b.c")
- .hasEmptyNameInHierarchy('.'))
- .isFalse();
- }
-
- @Test
- void hasEmptyNameInHierarchyShouldBeTrueIfEmptyPath() {
- assertThat(MailboxPath.forUser(USER, "")
- .hasEmptyNameInHierarchy('.'))
- .isTrue();
- }
-
- @Test
- void hasEmptyNameInHierarchyShouldBeTrueIfBlankPath() {
- assertThat(MailboxPath.forUser(USER, " ")
- .hasEmptyNameInHierarchy('.'))
- .isTrue();
- }
-
- @Test
- void hasEmptyNameInHierarchyShouldBeTrueIfPathWithTwoEmptyNames() {
- assertThat(MailboxPath.forUser(USER, ".")
- .hasEmptyNameInHierarchy('.'))
- .isTrue();
- }
-
- @Test
- void
hasEmptyNameInHierarchyShouldBeTrueIfPathWithAnEmptyNameBetweenTwoNames() {
- assertThat(MailboxPath.forUser(USER, "a..b")
- .hasEmptyNameInHierarchy('.'))
- .isTrue();
- }
-
- @Test
- void
hasEmptyNameInHierarchyShouldBeTrueIfPathWithABlankNameBetweenTwoNames() {
- assertThat(MailboxPath.forUser(USER, "a. .b")
- .hasEmptyNameInHierarchy('.'))
- .isTrue();
- }
-
- @Test
- void hasEmptyNameInHierarchyShouldBeTrueIfPathWithHeadingEmptyNames() {
- assertThat(MailboxPath.forUser(USER, "..a")
- .hasEmptyNameInHierarchy('.'))
- .isTrue();
- }
-
- @Test
- void hasEmptyNameInHierarchyShouldBeTrueIfPathWithHeadingBlankName() {
- assertThat(MailboxPath.forUser(USER, " .a")
- .hasEmptyNameInHierarchy('.'))
- .isTrue();
- }
-
- @Test
- void hasEmptyNameInHierarchyShouldBeTrueIfPathWithATrailingEmptyName() {
- assertThat(MailboxPath.forUser(USER, "a.")
- .hasEmptyNameInHierarchy('.'))
- .isTrue();
- }
-
- @Test
- void hasEmptyNameInHierarchyShouldBeTrueIfPathWithATrailingBlankName() {
- assertThat(MailboxPath.forUser(USER, "a. ")
- .hasEmptyNameInHierarchy('.'))
- .isTrue();
- }
-
- @Test
- void hasEmptyNameInHierarchyShouldBeTrueIfPathWithTrailingEmptyNames() {
- assertThat(MailboxPath.forUser(USER, "a..")
- .hasEmptyNameInHierarchy('.'))
- .isTrue();
- }
-
- @Test
- void hasEmptyNameInHierarchyShouldBeTrueIfPathWithTrailingBlankNames() {
- assertThat(MailboxPath.forUser(USER, "a. . ")
- .hasEmptyNameInHierarchy('.'))
- .isTrue();
- }
-
- @Test
- void assertAcceptableShouldThrowOnDoubleSeparator() {
- assertThatThrownBy(() -> MailboxPath.forUser(USER, "a..b")
- .assertAcceptable('.'))
- .isInstanceOf(HasEmptyMailboxNameInHierarchyException.class);
- }
-
- @Test
- void assertAcceptableShouldThrowWhenStartsWithSharp() {
- assertThatThrownBy(() -> MailboxPath.forUser(USER, "#ab")
- .assertAcceptable('.'))
- .isInstanceOf(MailboxNameException.class);
- }
-
- @Test
- void assertAcceptableShouldNotThrowWhenSharpInTheMiddle() {
- assertThatCode(() -> MailboxPath.forUser(USER, "mailbox #17")
- .assertAcceptable('.'))
- .doesNotThrowAnyException();
- }
-
- @Test
- void assertAcceptableShouldThrowOnPercent() {
- assertThatThrownBy(() -> MailboxPath.forUser(USER, "a%b")
- .assertAcceptable('.'))
- .isInstanceOf(MailboxNameException.class);
- }
-
- @Test
- void assertAcceptableShouldNotThrowOnPercentWhenRelaxMode() {
- System.setProperty("james.relaxed.mailbox.name.validation", "true");
-
- assertThatCode(() -> MailboxPath.forUser(USER, "a%b"))
- .doesNotThrowAnyException();
- }
Review Comment:
The old version of the relax-mode tests essentially asked "If I do nothing,
does it throw an exception?" :grin: Obviously, that sort of mistake is easy to
overlook as the resulting test trivially passes, but I still found it a bit
funny
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
