chibenwa commented on a change in pull request #707:
URL: https://github.com/apache/james-project/pull/707#discussion_r736597282



##########
File path: 
server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/model/UuidState.scala
##########
@@ -0,0 +1,60 @@
+/******************************************************************
+ * 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.model
+
+import java.util.UUID
+
+import eu.timepit.refined.api.Refined
+import eu.timepit.refined.refineV
+import eu.timepit.refined.string.Uuid
+import org.apache.james.jmap.api.change.{State => JavaState, EmailChanges, 
MailboxChanges}
+
+import scala.util.Try
+
+object UuidState {

Review comment:
       THis is not enough as an explanation. I see no references to `UuidState` 
in the TypeName class above.

##########
File path: 
server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/pushsubscription/MemoryPushSubscriptionRepository.java
##########
@@ -0,0 +1,172 @@
+/******************************************************************
+ * 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.memory.pushsubscription;
+
+import java.time.Clock;
+import java.time.ZonedDateTime;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.model.DeviceClientIdInvalidException;
+import org.apache.james.jmap.api.model.ExpireTimeInvalidException;
+import org.apache.james.jmap.api.model.PushSubscription;
+import org.apache.james.jmap.api.model.PushSubscriptionCreationRequest;
+import org.apache.james.jmap.api.model.PushSubscriptionExpiredTime;
+import org.apache.james.jmap.api.model.PushSubscriptionId;
+import org.apache.james.jmap.api.model.PushSubscriptionNotFoundException;
+import org.apache.james.jmap.api.model.TypeName;
+import org.apache.james.jmap.api.model.VerificationCode;
+import org.apache.james.jmap.api.pushsubscription.PushSubscriptionRepository;
+import org.reactivestreams.Publisher;
+
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.Table;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+import scala.jdk.javaapi.CollectionConverters;
+import scala.jdk.javaapi.OptionConverters;
+
+public class MemoryPushSubscriptionRepository implements 
PushSubscriptionRepository {
+    private final Table<Username, PushSubscriptionId, PushSubscription> table;
+    private final Clock clock;
+
+    @Inject
+    public MemoryPushSubscriptionRepository(Clock clock) {
+        this.clock = clock;
+        table = HashBasedTable.create();
+    }
+
+    @Override
+    public Publisher<PushSubscriptionId> save(Username username, 
PushSubscriptionCreationRequest request) {
+        return Mono.just(request)
+            .handle((req, sink) -> {
+                if (req.expires().isDefined()) {
+                    ZonedDateTime inputTime = req.expires().get().value();
+                    if (!expireTimePreconditions(inputTime, clock)) {
+                        sink.error(new ExpireTimeInvalidException(inputTime, 
"expires must be greater than now"));
+                    }
+                }
+                if (!isUniqueDeviceClientId(username, req.deviceClientId())) {
+                    sink.error(new 
DeviceClientIdInvalidException(req.deviceClientId(), "deviceClientId must be 
unique"));
+                }
+            })
+            .thenReturn(PushSubscriptionId.generate())
+            .doOnNext(id -> table.put(username, id,
+                PushSubscription.from(request,
+                    id,
+                    
evaluateExpiresTime(OptionConverters.toJava(request.expires().map(PushSubscriptionExpiredTime::value)),
 clock),
+                    VerificationCode.generate())));
+    }
+
+    @Override
+    public Publisher<Void> updateExpireTime(Username username, 
PushSubscriptionId id, ZonedDateTime newExpire) {
+        return Mono.just(newExpire)
+            .handle((inputTime, sink) -> {
+                if (!expireTimePreconditions(newExpire, clock)) {
+                    sink.error(new ExpireTimeInvalidException(inputTime, 
"expires must be greater than now"));
+                }
+            })
+            .then(Mono.justOrEmpty(table.get(username, id))
+                .doOnNext(pushSubscription -> {
+                    PushSubscription newPushSubscription = 
PushSubscription.apply(
+                        pushSubscription.id(),
+                        pushSubscription.deviceClientId(),
+                        pushSubscription.url(),
+                        pushSubscription.keys(),
+                        pushSubscription.verificationCode(),
+                        pushSubscription.validated(),
+                        
PushSubscriptionExpiredTime.apply(evaluateExpiresTime(Optional.of(newExpire), 
clock)),
+                        pushSubscription.types());

Review comment:
       pushSubscription.withExpiredTime(...)

##########
File path: 
server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/pushsubscription/MemoryPushSubscriptionRepository.java
##########
@@ -0,0 +1,172 @@
+/******************************************************************
+ * 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.memory.pushsubscription;
+
+import java.time.Clock;
+import java.time.ZonedDateTime;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.model.DeviceClientIdInvalidException;
+import org.apache.james.jmap.api.model.ExpireTimeInvalidException;
+import org.apache.james.jmap.api.model.PushSubscription;
+import org.apache.james.jmap.api.model.PushSubscriptionCreationRequest;
+import org.apache.james.jmap.api.model.PushSubscriptionExpiredTime;
+import org.apache.james.jmap.api.model.PushSubscriptionId;
+import org.apache.james.jmap.api.model.PushSubscriptionNotFoundException;
+import org.apache.james.jmap.api.model.TypeName;
+import org.apache.james.jmap.api.model.VerificationCode;
+import org.apache.james.jmap.api.pushsubscription.PushSubscriptionRepository;
+import org.reactivestreams.Publisher;
+
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.Table;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+import scala.jdk.javaapi.CollectionConverters;
+import scala.jdk.javaapi.OptionConverters;
+
+public class MemoryPushSubscriptionRepository implements 
PushSubscriptionRepository {
+    private final Table<Username, PushSubscriptionId, PushSubscription> table;
+    private final Clock clock;
+
+    @Inject
+    public MemoryPushSubscriptionRepository(Clock clock) {
+        this.clock = clock;
+        table = HashBasedTable.create();
+    }
+
+    @Override
+    public Publisher<PushSubscriptionId> save(Username username, 
PushSubscriptionCreationRequest request) {
+        return Mono.just(request)
+            .handle((req, sink) -> {
+                if (req.expires().isDefined()) {
+                    ZonedDateTime inputTime = req.expires().get().value();
+                    if (!expireTimePreconditions(inputTime, clock)) {
+                        sink.error(new ExpireTimeInvalidException(inputTime, 
"expires must be greater than now"));
+                    }
+                }
+                if (!isUniqueDeviceClientId(username, req.deviceClientId())) {
+                    sink.error(new 
DeviceClientIdInvalidException(req.deviceClientId(), "deviceClientId must be 
unique"));
+                }
+            })
+            .thenReturn(PushSubscriptionId.generate())
+            .doOnNext(id -> table.put(username, id,
+                PushSubscription.from(request,
+                    id,
+                    
evaluateExpiresTime(OptionConverters.toJava(request.expires().map(PushSubscriptionExpiredTime::value)),
 clock),
+                    VerificationCode.generate())));
+    }
+
+    @Override
+    public Publisher<Void> updateExpireTime(Username username, 
PushSubscriptionId id, ZonedDateTime newExpire) {
+        return Mono.just(newExpire)
+            .handle((inputTime, sink) -> {
+                if (!expireTimePreconditions(newExpire, clock)) {
+                    sink.error(new ExpireTimeInvalidException(inputTime, 
"expires must be greater than now"));
+                }
+            })
+            .then(Mono.justOrEmpty(table.get(username, id))
+                .doOnNext(pushSubscription -> {
+                    PushSubscription newPushSubscription = 
PushSubscription.apply(
+                        pushSubscription.id(),
+                        pushSubscription.deviceClientId(),
+                        pushSubscription.url(),
+                        pushSubscription.keys(),
+                        pushSubscription.verificationCode(),
+                        pushSubscription.validated(),
+                        
PushSubscriptionExpiredTime.apply(evaluateExpiresTime(Optional.of(newExpire), 
clock)),
+                        pushSubscription.types());
+                    table.put(username, id, newPushSubscription);
+                })
+                .switchIfEmpty(Mono.error(() -> new 
PushSubscriptionNotFoundException(id)))
+                .then());
+    }
+
+    @Override
+    public Publisher<Void> updateTypes(Username username, PushSubscriptionId 
id, Set<TypeName> types) {
+        return Mono.justOrEmpty(table.get(username, id))
+            .doOnNext(pushSubscription -> {
+                PushSubscription newPushSubscription = PushSubscription.apply(
+                    pushSubscription.id(),
+                    pushSubscription.deviceClientId(),
+                    pushSubscription.url(),
+                    pushSubscription.keys(),
+                    pushSubscription.verificationCode(),
+                    pushSubscription.validated(),
+                    pushSubscription.expires(),
+                    CollectionConverters.asScala(types).toSeq());
+                table.put(username, id, newPushSubscription);
+            })
+            .switchIfEmpty(Mono.error(() -> new 
PushSubscriptionNotFoundException(id)))
+            .then();
+    }
+
+    @Override
+    public Publisher<Void> revoke(Username username, PushSubscriptionId id) {
+        return Mono.fromCallable(() -> table.remove(username, id)).then();
+    }
+
+    @Override
+    public Publisher<PushSubscription> get(Username username, 
Set<PushSubscriptionId> ids) {
+        return Flux.fromStream(table.row(username).entrySet().stream())
+            .filter(entry -> ids.contains(entry.getKey()))
+            .map(Map.Entry::getValue)
+            .filter(subscription -> isNotOutdatedSubscription(subscription, 
clock));
+    }
+
+    @Override
+    public Publisher<PushSubscription> list(Username username) {
+        return Flux.fromStream(table.row(username).entrySet().stream())
+            .map(Map.Entry::getValue)
+            .filter(subscription -> isNotOutdatedSubscription(subscription, 
clock));
+    }
+
+    @Override
+    public Publisher<Void> validateVerificationCode(Username username, 
PushSubscriptionId id) {
+        return Mono.justOrEmpty(table.get(username, id))
+            .doOnNext(pushSubscription -> {
+                if (!pushSubscription.validated()) {
+                    PushSubscription newPushSubscription = 
PushSubscription.apply(
+                        pushSubscription.id(),
+                        pushSubscription.deviceClientId(),
+                        pushSubscription.url(),
+                        pushSubscription.keys(),
+                        pushSubscription.verificationCode(),
+                        true,
+                        pushSubscription.expires(),
+                        pushSubscription.types());
+                    table.put(username, id, newPushSubscription);
+                }
+            })
+            .switchIfEmpty(Mono.error(() -> new 
PushSubscriptionNotFoundException(id)))
+            .then();
+    }
+
+    private boolean isUniqueDeviceClientId(Username username, String 
deviceClientId) {
+        return table.row(username).values().stream()
+            .noneMatch(e -> e.deviceClientId().equals(deviceClientId));

Review comment:
       What is `e`?

##########
File path: 
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/WebSocketRoutes.scala
##########
@@ -32,9 +32,9 @@ import org.apache.james.events.{EventBus, Registration}
 import org.apache.james.jmap.HttpConstants.JSON_CONTENT_TYPE
 import org.apache.james.jmap.JMAPUrls.JMAP_WS
 import org.apache.james.jmap.api.change.{EmailChangeRepository, 
MailboxChangeRepository}
-import org.apache.james.jmap.api.model.{EmailTypeName, MailboxTypeName, 
UuidState, AccountId => JavaAccountId}
-import org.apache.james.jmap.change.{AccountIdRegistrationKey, 
StateChangeListener, _}
-import org.apache.james.jmap.core.{OutboundMessage, ProblemDetails, RequestId, 
WebSocketError, WebSocketPushDisable, WebSocketPushEnable, WebSocketRequest, 
WebSocketResponse, _}
+import org.apache.james.jmap.api.model.{UuidState, AccountId => JavaAccountId}
+import org.apache.james.jmap.change._
+import org.apache.james.jmap.core._

Review comment:
       > Why don't we either explicitly import or just do wildcard import? 
(question)
   
   Answer: because we want everything to be explicit. With wildcard you never 
really know where the stuff you use come from. There can be naming clashes.
   
   We only use `_` for implicits with mixins, eg OptionsConverter...

##########
File path: 
server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/pushsubscription/MemoryPushSubscriptionRepository.java
##########
@@ -0,0 +1,163 @@
+/******************************************************************
+ * 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.memory.pushsubscription;
+
+import java.time.Clock;
+import java.time.ZonedDateTime;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.UUID;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.model.DeviceClientIdInvalidException;
+import org.apache.james.jmap.api.model.PushSubscription;
+import org.apache.james.jmap.api.model.PushSubscriptionCreationRequest;
+import org.apache.james.jmap.api.model.PushSubscriptionExpiredTime;
+import org.apache.james.jmap.api.model.PushSubscriptionId;
+import org.apache.james.jmap.api.model.PushSubscriptionNotFoundException;
+import org.apache.james.jmap.api.model.TypeName;
+import org.apache.james.jmap.api.pushsubscription.PushSubscriptionRepository;
+import 
org.apache.james.jmap.api.pushsubscription.PushSubscriptionVerificationCodeFactory;
+import org.reactivestreams.Publisher;
+
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.Table;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+import scala.jdk.javaapi.CollectionConverters;
+import scala.jdk.javaapi.OptionConverters;
+
+public class MemoryPushSubscriptionRepository implements 
PushSubscriptionRepository {
+    private final Table<Username, PushSubscriptionId, PushSubscription> table;
+    private final Clock clock;
+    private final PushSubscriptionVerificationCodeFactory 
verificationCodeFactory;
+
+    @Inject
+    public MemoryPushSubscriptionRepository(Clock clock, 
PushSubscriptionVerificationCodeFactory factory) {
+        this.clock = clock;
+        this.verificationCodeFactory = factory;
+        table = HashBasedTable.create();
+    }
+
+    @Override
+    public Publisher<PushSubscriptionId> save(Username username, 
PushSubscriptionCreationRequest request) {
+        if (request.expires().isDefined()) {
+            expireTimePreconditions(request.expires().get().value(), clock);
+        }
+        if (!isUniqueDeviceClientId(username, request.deviceClientId())) {
+            throw new DeviceClientIdInvalidException(request.deviceClientId(), 
"deviceClientId must be unique");
+        }
+        return Mono.fromCallable(() -> 
PushSubscriptionId.apply(UUID.randomUUID()))
+            .doOnNext(id -> table.put(username, id,
+                PushSubscription.from(request,
+                    id,
+                    
evaluateExpiresTime(OptionConverters.toJava(request.expires().map(PushSubscriptionExpiredTime::value)),
 clock),
+                    String.valueOf(verificationCodeFactory.generate()))));
+    }
+
+    @Override
+    public Publisher<Void> updateExpireTime(Username username, 
PushSubscriptionId id, ZonedDateTime newExpire) {
+        expireTimePreconditions(newExpire, clock);
+        return Mono.justOrEmpty(table.get(username, id))
+            .doOnNext(pushSubscription -> {
+                PushSubscription newPushSubscription = PushSubscription.apply(
+                    pushSubscription.id(),
+                    pushSubscription.deviceClientId(),
+                    pushSubscription.url(),
+                    pushSubscription.keys(),
+                    pushSubscription.verificationCode(),
+                    pushSubscription.validated(),
+                    
PushSubscriptionExpiredTime.apply(evaluateExpiresTime(Optional.of(newExpire), 
clock)),
+                    pushSubscription.types());
+                table.put(username, id, newPushSubscription);
+            })
+            .switchIfEmpty(Mono.error(() -> new 
PushSubscriptionNotFoundException(id)))
+            .then();
+    }
+
+    @Override
+    public Publisher<Void> updateTypes(Username username, PushSubscriptionId 
id, Set<TypeName> types) {
+        return Mono.justOrEmpty(table.get(username, id))
+            .doOnNext(pushSubscription -> {
+                PushSubscription newPushSubscription = PushSubscription.apply(
+                    pushSubscription.id(),
+                    pushSubscription.deviceClientId(),
+                    pushSubscription.url(),
+                    pushSubscription.keys(),
+                    pushSubscription.verificationCode(),
+                    pushSubscription.validated(),
+                    pushSubscription.expires(),
+                    CollectionConverters.asScala(types).toSeq());
+                table.put(username, id, newPushSubscription);
+            })
+            .switchIfEmpty(Mono.error(() -> new 
PushSubscriptionNotFoundException(id)))
+            .then();
+    }
+
+    @Override
+    public Publisher<Void> revoke(Username username, PushSubscriptionId id) {
+        return Mono.fromCallable(() -> table.remove(username, id)).then();
+    }
+
+    @Override
+    public Publisher<PushSubscription> get(Username username, 
Set<PushSubscriptionId> ids) {
+        return Flux.fromStream(table.row(username).entrySet().stream())
+            .filter(entry -> ids.contains(entry.getKey()))
+            .map(Map.Entry::getValue)
+            .filter(subscription -> isNotOutdatedSubscription(subscription, 
clock));
+    }
+
+    @Override
+    public Publisher<PushSubscription> list(Username username) {
+        return Flux.fromStream(table.row(username).entrySet().stream())
+            .map(Map.Entry::getValue)
+            .filter(subscription -> isNotOutdatedSubscription(subscription, 
clock));
+    }
+
+    @Override
+    public Publisher<Void> validateVerificationCode(Username username, 
PushSubscriptionId id) {
+        return Mono.justOrEmpty(table.get(username, id))
+            .doOnNext(pushSubscription -> {
+                if (!pushSubscription.validated()) {
+                    PushSubscription newPushSubscription = 
PushSubscription.apply(
+                        pushSubscription.id(),
+                        pushSubscription.deviceClientId(),
+                        pushSubscription.url(),
+                        pushSubscription.keys(),
+                        pushSubscription.verificationCode(),
+                        true,
+                        pushSubscription.expires(),
+                        pushSubscription.types());

Review comment:
       Not solved

##########
File path: 
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/json/EmailSetSerializer.scala
##########
@@ -25,10 +25,11 @@ import eu.timepit.refined.collection.NonEmpty
 import eu.timepit.refined.refineV
 import eu.timepit.refined.types.string.NonEmptyString
 import javax.inject.Inject
+import org.apache.james.jmap.api.model.UuidState
 import org.apache.james.jmap.core.Id.IdConstraint
-import org.apache.james.jmap.core.{Id, SetError, UTCDate, UuidState}
+import org.apache.james.jmap.core.{Id, SetError, UTCDate}
 import org.apache.james.jmap.mail.KeywordsFactory.STRICT_KEYWORDS_FACTORY
-import org.apache.james.jmap.mail.{AddressesHeaderValue, AsAddresses, AsDate, 
AsGroupedAddresses, AsMessageIds, AsRaw, AsText, AsURLs, Attachment, BlobId, 
Charset, ClientBody, ClientCid, ClientEmailBodyValue, ClientPartId, 
DateHeaderValue, DestroyIds, Disposition, EmailAddress, EmailAddressGroup, 
EmailCreationId, EmailCreationRequest, EmailCreationResponse, EmailHeader, 
EmailHeaderName, EmailHeaderValue, EmailImport, EmailImportRequest, 
EmailImportResponse, EmailSetRequest, EmailSetResponse, EmailSetUpdate, 
EmailerName, GroupName, GroupedAddressesHeaderValue, HeaderMessageId, 
HeaderURL, IsEncodingProblem, IsTruncated, Keyword, Keywords, Language, 
Languages, Location, MailboxIds, MessageIdsHeaderValue, Name, ParseOption, 
RawHeaderValue, SpecificHeaderRequest, Subject, TextHeaderValue, ThreadId, 
Type, URLsHeaderValue, UnparsedMessageId}
+import org.apache.james.jmap.mail._

Review comment:
       Oups

##########
File path: 
server/data/data-jmap/src/main/java/org/apache/james/jmap/api/pushsubscription/PushSubscriptionRepository.java
##########
@@ -0,0 +1,63 @@
+/******************************************************************
+ * 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.pushsubscription;
+
+import java.time.Clock;
+import java.time.ZonedDateTime;
+import java.util.Optional;
+import java.util.Set;
+
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.model.PushSubscription;
+import org.apache.james.jmap.api.model.PushSubscriptionCreationRequest;
+import org.apache.james.jmap.api.model.PushSubscriptionId;
+import org.apache.james.jmap.api.model.TypeName;
+import org.reactivestreams.Publisher;
+
+public interface PushSubscriptionRepository {
+    Publisher<PushSubscriptionId> save(Username username, 
PushSubscriptionCreationRequest pushSubscriptionCreationRequest);
+
+    Publisher<Void> updateExpireTime(Username username, PushSubscriptionId id, 
ZonedDateTime newExpire);
+
+    Publisher<Void> updateTypes(Username username, PushSubscriptionId id, 
Set<TypeName> types);
+
+    Publisher<Void> revoke(Username username, PushSubscriptionId id);
+
+    Publisher<PushSubscription> get(Username username, Set<PushSubscriptionId> 
ids);
+
+    Publisher<PushSubscription> list(Username username);
+
+    Publisher<Void> validateVerificationCode(Username username, 
PushSubscriptionId id);
+
+    default boolean expireTimePreconditions(ZonedDateTime dateTime, Clock 
clock) {
+        return PushSubscription.checkValidTime(dateTime, clock);
+    }
+
+    default ZonedDateTime evaluateExpiresTime(Optional<ZonedDateTime> 
inputTime, Clock clock) {
+        ZonedDateTime now = ZonedDateTime.now(clock);
+        ZonedDateTime maxExpiresTime = 
now.plusDays(PushSubscription.EXPIRES_TIME_MAX_DAY());
+        return inputTime.filter(input -> input.isBefore(maxExpiresTime))
+            .orElse(maxExpiresTime);
+    }
+
+    default boolean isNotOutdatedSubscription(PushSubscription subscription, 
Clock clock) {
+        return PushSubscription.checkValidTime(subscription.expires(), clock);
+    }

Review comment:
       I am not a fan of leaking implentation details in the interface.
   
   Here I prefer keeping those within the memory implentation. We will see down 
the road if the Cassandra implementation needs it too. If so helper classes are 
likely more appropriate... Duplication of so little logic seems also OK to me.

##########
File path: 
server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/routes/WebSocketRoutes.scala
##########
@@ -32,9 +32,9 @@ import org.apache.james.events.{EventBus, Registration}
 import org.apache.james.jmap.HttpConstants.JSON_CONTENT_TYPE
 import org.apache.james.jmap.JMAPUrls.JMAP_WS
 import org.apache.james.jmap.api.change.{EmailChangeRepository, 
MailboxChangeRepository}
-import org.apache.james.jmap.api.model.{EmailTypeName, MailboxTypeName, 
UuidState, AccountId => JavaAccountId}
-import org.apache.james.jmap.change.{AccountIdRegistrationKey, 
StateChangeListener, _}
-import org.apache.james.jmap.core.{OutboundMessage, ProblemDetails, RequestId, 
WebSocketError, WebSocketPushDisable, WebSocketPushEnable, WebSocketRequest, 
WebSocketResponse, _}
+import org.apache.james.jmap.api.model.{UuidState, AccountId => JavaAccountId}
+import org.apache.james.jmap.change._
+import org.apache.james.jmap.core._

Review comment:
       Please avoid changing these imports and reconfigure your IDE to avoid 
doing so.

##########
File path: 
server/data/data-jmap/src/main/java/org/apache/james/jmap/memory/pushsubscription/MemoryPushSubscriptionRepository.java
##########
@@ -0,0 +1,163 @@
+/******************************************************************
+ * 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.memory.pushsubscription;
+
+import java.time.Clock;
+import java.time.ZonedDateTime;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.UUID;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.model.DeviceClientIdInvalidException;
+import org.apache.james.jmap.api.model.PushSubscription;
+import org.apache.james.jmap.api.model.PushSubscriptionCreationRequest;
+import org.apache.james.jmap.api.model.PushSubscriptionExpiredTime;
+import org.apache.james.jmap.api.model.PushSubscriptionId;
+import org.apache.james.jmap.api.model.PushSubscriptionNotFoundException;
+import org.apache.james.jmap.api.model.TypeName;
+import org.apache.james.jmap.api.pushsubscription.PushSubscriptionRepository;
+import 
org.apache.james.jmap.api.pushsubscription.PushSubscriptionVerificationCodeFactory;
+import org.reactivestreams.Publisher;
+
+import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.Table;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+import scala.jdk.javaapi.CollectionConverters;
+import scala.jdk.javaapi.OptionConverters;
+
+public class MemoryPushSubscriptionRepository implements 
PushSubscriptionRepository {
+    private final Table<Username, PushSubscriptionId, PushSubscription> table;
+    private final Clock clock;
+    private final PushSubscriptionVerificationCodeFactory 
verificationCodeFactory;
+
+    @Inject
+    public MemoryPushSubscriptionRepository(Clock clock, 
PushSubscriptionVerificationCodeFactory factory) {
+        this.clock = clock;
+        this.verificationCodeFactory = factory;
+        table = HashBasedTable.create();
+    }
+
+    @Override
+    public Publisher<PushSubscriptionId> save(Username username, 
PushSubscriptionCreationRequest request) {
+        if (request.expires().isDefined()) {
+            expireTimePreconditions(request.expires().get().value(), clock);
+        }
+        if (!isUniqueDeviceClientId(username, request.deviceClientId())) {
+            throw new DeviceClientIdInvalidException(request.deviceClientId(), 
"deviceClientId must be unique");
+        }
+        return Mono.fromCallable(() -> 
PushSubscriptionId.apply(UUID.randomUUID()))
+            .doOnNext(id -> table.put(username, id,
+                PushSubscription.from(request,
+                    id,
+                    
evaluateExpiresTime(OptionConverters.toJava(request.expires().map(PushSubscriptionExpiredTime::value)),
 clock),
+                    String.valueOf(verificationCodeFactory.generate()))));
+    }
+
+    @Override
+    public Publisher<Void> updateExpireTime(Username username, 
PushSubscriptionId id, ZonedDateTime newExpire) {
+        expireTimePreconditions(newExpire, clock);
+        return Mono.justOrEmpty(table.get(username, id))
+            .doOnNext(pushSubscription -> {
+                PushSubscription newPushSubscription = PushSubscription.apply(
+                    pushSubscription.id(),
+                    pushSubscription.deviceClientId(),
+                    pushSubscription.url(),
+                    pushSubscription.keys(),
+                    pushSubscription.verificationCode(),
+                    pushSubscription.validated(),
+                    
PushSubscriptionExpiredTime.apply(evaluateExpiresTime(Optional.of(newExpire), 
clock)),
+                    pushSubscription.types());
+                table.put(username, id, newPushSubscription);
+            })
+            .switchIfEmpty(Mono.error(() -> new 
PushSubscriptionNotFoundException(id)))
+            .then();
+    }
+
+    @Override
+    public Publisher<Void> updateTypes(Username username, PushSubscriptionId 
id, Set<TypeName> types) {
+        return Mono.justOrEmpty(table.get(username, id))
+            .doOnNext(pushSubscription -> {
+                PushSubscription newPushSubscription = PushSubscription.apply(
+                    pushSubscription.id(),
+                    pushSubscription.deviceClientId(),
+                    pushSubscription.url(),
+                    pushSubscription.keys(),
+                    pushSubscription.verificationCode(),
+                    pushSubscription.validated(),
+                    pushSubscription.expires(),
+                    CollectionConverters.asScala(types).toSeq());
+                table.put(username, id, newPushSubscription);

Review comment:
       (not solved)

##########
File path: 
server/data/data-jmap/src/test/scala/org/apache/james/jmap/api/pushsubscription/PushSubscriptionRepositoryContract.scala
##########
@@ -0,0 +1,195 @@
+/** ****************************************************************
+ * 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.pushsubscription
+
+import java.net.URL
+import java.time.{Clock, Instant, ZoneId, ZonedDateTime}
+
+import org.apache.james.core.Username
+import org.apache.james.jmap.api.model.{DeviceClientId, 
DeviceClientIdInvalidException, ExpireTimeInvalidException, 
PushSubscriptionCreationRequest, PushSubscriptionExpiredTime, 
PushSubscriptionServerURL, TypeName, UuidState}
+import 
org.apache.james.jmap.api.pushsubscription.PushSubscriptionRepositoryContract.{ALICE,
 BIGGER_EXPIRE_THAN_MAX, INVALID_EXPIRE, MAX_EXPIRE}
+import org.assertj.core.api.Assertions.{assertThat, assertThatThrownBy}
+import org.junit.jupiter.api.Test
+import reactor.core.scala.publisher.{SFlux, SMono}
+
+import scala.jdk.CollectionConverters._
+
+case object CustomTypeName1 extends TypeName {
+  override val asString: String = "custom1"
+
+  override def parse(string: String): Option[TypeName] = string match {
+    case CustomTypeName1.asString => Some(CustomTypeName1)
+    case _ => None
+  }
+
+  override def parseState(string: String): Either[IllegalArgumentException, 
UuidState] = UuidState.parse(string)
+}
+
+case object CustomTypeName2 extends TypeName {
+  override val asString: String = "custom2"
+
+  override def parse(string: String): Option[TypeName] = string match {
+    case CustomTypeName2.asString => Some(CustomTypeName2)
+    case _ => None
+  }
+
+  override def parseState(string: String): Either[IllegalArgumentException, 
UuidState] = UuidState.parse(string)
+}
+
+object PushSubscriptionRepositoryContract {
+  val NOW: Instant = Instant.parse("2021-10-25T07:05:39.160Z")
+  val CLOCK: Clock = Clock.fixed(NOW, ZoneId.of("UTC"))
+  val INVALID_EXPIRE: ZonedDateTime = 
ZonedDateTime.parse("2020-10-25T07:05:39.160Z[UTC]")
+  val MAX_EXPIRE: ZonedDateTime = 
ZonedDateTime.parse("2021-10-25T07:05:39.160Z[UTC]").plusDays(7)
+  val BIGGER_EXPIRE_THAN_MAX: ZonedDateTime = 
ZonedDateTime.parse("2021-10-25T07:05:39.160Z[UTC]").plusDays(8)
+  val ALICE: Username = Username.of("alice")
+  val BOB: Username = Username.of("bob")
+}
+
+trait PushSubscriptionRepositoryContract {
+  def testee: PushSubscriptionRepository
+
+  @Test
+  def validSubscriptionShouldBeSavedSuccessfully(): Unit = {
+    val validRequest = PushSubscriptionCreationRequest(
+      deviceClientId = DeviceClientId.apply("1"),
+      url = PushSubscriptionServerURL(new URL("https://example.com/push";)),
+      keys = Option.empty,
+      expires = Option.empty,
+      types = Seq(CustomTypeName1)
+    )

Review comment:
       ```suggestion
         types = Seq(CustomTypeName1))
   ```
   
   I really do not like brackets alone on their lines...
   
   Can we fix it in all this class as well?




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to