You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by rc...@apache.org on 2020/12/28 07:43:43 UTC
[james-project] 11/16: JAMES-3469 Add null checks within
MailboxChange POJO builder
This is an automated email from the ASF dual-hosted git repository.
rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 75293e5d8986097da19478a5dc4ffe1a52686b72
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Dec 21 14:43:51 2020 +0700
JAMES-3469 Add null checks within MailboxChange POJO builder
Enforces accountId and state to always be here, implementations can safely skip these checks
---
.../james/jmap/api/change/MailboxChange.java | 40 ++++++++++++
.../change/MemoryMailboxChangeRepository.java | 3 -
.../change/MailboxChangeRepositoryContract.java | 21 -------
.../james/jmap/api/change/MailboxChangeTest.java | 72 ++++++++++++++++++++++
4 files changed, 112 insertions(+), 24 deletions(-)
diff --git a/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/change/MailboxChange.java b/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/change/MailboxChange.java
index f242cc0..f8af13e 100644
--- a/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/change/MailboxChange.java
+++ b/server/data/data-jmap/src/main/java/org/apache/james/jmap/api/change/MailboxChange.java
@@ -22,6 +22,7 @@ package org.apache.james.jmap.api.change;
import java.time.Clock;
import java.time.ZonedDateTime;
import java.util.List;
+import java.util.Objects;
import java.util.Optional;
import java.util.stream.Stream;
@@ -46,6 +47,8 @@ import org.apache.james.mailbox.model.MailboxId;
import org.apache.james.mailbox.model.MailboxPath;
import com.github.steveash.guavate.Guavate;
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
public class MailboxChange {
@@ -74,6 +77,10 @@ public class MailboxChange {
private Optional<List<MailboxId>> destroyed;
private Builder(AccountId accountId, State state, ZonedDateTime date) {
+ Preconditions.checkNotNull(accountId, "'accountId' cannot be null");
+ Preconditions.checkNotNull(state, "'state' cannot be null");
+ Preconditions.checkNotNull(date, "'date' cannot be null");
+
this.accountId = accountId;
this.state = state;
this.date = date;
@@ -339,4 +346,37 @@ public class MailboxChange {
public List<MailboxId> getDestroyed() {
return destroyed;
}
+
+ @Override
+ public final boolean equals(Object o) {
+ if (o instanceof MailboxChange) {
+ MailboxChange that = (MailboxChange) o;
+ return Objects.equals(accountId, that.accountId)
+ && Objects.equals(state, that.state)
+ && Objects.equals(date, that.date)
+ && Objects.equals(delegated, that.delegated)
+ && Objects.equals(created, that.created)
+ && Objects.equals(updated, that.updated)
+ && Objects.equals(destroyed, that.destroyed);
+ }
+ return false;
+ }
+
+ @Override
+ public final int hashCode() {
+ return Objects.hash(accountId, state, date, delegated, created, updated, destroyed);
+ }
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("accountId", accountId)
+ .add("state", state)
+ .add("date", date)
+ .add("isDelegated", delegated)
+ .add("created", created)
+ .add("updated", updated)
+ .add("destroyed", destroyed)
+ .toString();
+ }
}
diff --git a/server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/change/MemoryMailboxChangeRepository.java b/server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/change/MemoryMailboxChangeRepository.java
index b50b497..6dc9105 100644
--- a/server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/change/MemoryMailboxChangeRepository.java
+++ b/server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/change/MemoryMailboxChangeRepository.java
@@ -51,9 +51,6 @@ public class MemoryMailboxChangeRepository implements MailboxChangeRepository {
@Override
public Mono<Void> save(MailboxChange change) {
- Preconditions.checkNotNull(change.getAccountId());
- Preconditions.checkNotNull(change.getState());
-
return Mono.just(mailboxChangeMap.put(change.getAccountId(), change)).then();
}
diff --git a/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/change/MailboxChangeRepositoryContract.java b/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/change/MailboxChangeRepositoryContract.java
index 81a9266..5be5163 100644
--- a/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/change/MailboxChangeRepositoryContract.java
+++ b/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/change/MailboxChangeRepositoryContract.java
@@ -147,27 +147,6 @@ public interface MailboxChangeRepositoryContract {
}
@Test
- default void saveChangeShouldFailWhenNoAccountId() {
- MailboxChangeRepository repository = mailboxChangeRepository();
- State.Factory stateFactory = stateFactory();
-
- MailboxChange change = MailboxChange.builder().accountId(null).state(stateFactory.generate()).date(DATE).created(ImmutableList.of(TestId.of(1))).build();
-
- assertThatThrownBy(() -> repository.save(change).block())
- .isInstanceOf(NullPointerException.class);
- }
-
- @Test
- default void saveChangeShouldFailWhenNoState() {
- MailboxChangeRepository repository = mailboxChangeRepository();
-
- MailboxChange change = MailboxChange.builder().accountId(ACCOUNT_ID).state(null).date(DATE).created(ImmutableList.of(TestId.of(1))).build();
-
- assertThatThrownBy(() -> repository.save(change).block())
- .isInstanceOf(NullPointerException.class);
- }
-
- @Test
default void getChangesShouldSuccess() {
MailboxChangeRepository repository = mailboxChangeRepository();
State.Factory stateFactory = stateFactory();
diff --git a/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/change/MailboxChangeTest.java b/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/change/MailboxChangeTest.java
new file mode 100644
index 0000000..06b0ed9
--- /dev/null
+++ b/server/data/data-jmap/src/test/java/org/apache/james/jmap/api/change/MailboxChangeTest.java
@@ -0,0 +1,72 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.jmap.api.change;
+
+import static org.apache.james.mailbox.fixture.MailboxFixture.BOB;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.time.ZonedDateTime;
+import java.util.UUID;
+
+import org.apache.james.jmap.api.model.AccountId;
+import org.junit.jupiter.api.Test;
+
+import nl.jqno.equalsverifier.EqualsVerifier;
+
+class MailboxChangeTest {
+ AccountId ACCOUNT_ID = AccountId.fromUsername(BOB);
+ ZonedDateTime DATE = ZonedDateTime.now();
+
+ @Test
+ void shouldMatchBeanContract() {
+ EqualsVerifier.forClass(MailboxChange.class)
+ .verify();
+ }
+
+ @Test
+ void shouldThrowOnNullAccountId() {
+ assertThatThrownBy(() ->
+ MailboxChange.builder()
+ .accountId(null)
+ .state(State.of(UUID.randomUUID()))
+ .date(DATE.minusHours(2)))
+ .isInstanceOf(NullPointerException.class);
+ }
+
+ @Test
+ void shouldThrowOnNullState() {
+ assertThatThrownBy(() ->
+ MailboxChange.builder()
+ .accountId(ACCOUNT_ID)
+ .state(null)
+ .date(DATE.minusHours(2)))
+ .isInstanceOf(NullPointerException.class);;
+ }
+
+ @Test
+ void shouldThrowOnNullDate() {
+ assertThatThrownBy(() ->
+ MailboxChange.builder()
+ .accountId(ACCOUNT_ID)
+ .state(State.of(UUID.randomUUID()))
+ .date(null))
+ .isInstanceOf(NullPointerException.class);;
+ }
+}
\ 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