You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2021/11/10 07:54:17 UTC
[GitHub] [james-project] chibenwa opened a new pull request #739: JAMES-3534 API, Memory & contract test for identity storage
chibenwa opened a new pull request #739:
URL: https://github.com/apache/james-project/pull/739
-> Include a convenient API for updates, allowing updating several fields at once...
-> Include a DAO layer dedicated to storage, and a repository layer ready to abstract default identities which will later be "patched".
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on a change in pull request #739: JAMES-3534 API, Memory & contract test for identity storage
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on a change in pull request #739:
URL: https://github.com/apache/james-project/pull/739#discussion_r747196100
##########
File path: server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MDNSendMethodContract.scala
##########
@@ -302,7 +303,7 @@ trait MDNSendMethodContract {
`given`
.body(request)
.when
- .post
+ .post.prettyPeek()
Review comment:
do not forget to delete :')
##########
File path: server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MDNSendMethodContract.scala
##########
@@ -1214,7 +1215,7 @@ trait MDNSendMethodContract {
`given`(buildBOBRequestSpecification(server))
.body(request)
.when
- .post
+ .post.prettyPeek()
Review comment:
idem
##########
File path: server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MDNSendMethodContract.scala
##########
@@ -1035,7 +1036,7 @@ trait MDNSendMethodContract {
`given`(buildBOBRequestSpecification(server))
.body(request)
.when
- .post
+ .post.prettyPeek()
Review comment:
idem
##########
File path: server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MDNSendMethodContract.scala
##########
@@ -1374,7 +1375,7 @@ trait MDNSendMethodContract {
`given`
.body(request)
.when
- .post
+ .post.prettyPeek()
Review comment:
idem
##########
File path: server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MDNSendMethodContract.scala
##########
@@ -1888,7 +1889,7 @@ trait MDNSendMethodContract {
`given`
.body(request)
.when
- .post
+ .post.prettyPeek()
Review comment:
idem
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on a change in pull request #739: JAMES-3534 API, Memory & contract test for identity storage
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on a change in pull request #739:
URL: https://github.com/apache/james-project/pull/739#discussion_r747967351
##########
File path: server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/model/Identity.scala
##########
@@ -0,0 +1,45 @@
+/****************************************************************
+ * 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 org.apache.james.core.MailAddress
+
+case class IdentityName(name: String) extends AnyVal
+case class TextSignature(name: String) extends AnyVal
+case class HtmlSignature(name: String) extends AnyVal
+case class MayDeleteIdentity(value: Boolean) extends AnyVal
+
+object IdentityId {
+ def generate: IdentityId = IdentityId(UUID.randomUUID())
+}
+case class IdentityId(id: UUID)
+
+case class Identity(id: IdentityId,
+ name: IdentityName,
Review comment:
name: Option[IdentityName]?
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa merged pull request #739: JAMES-3534 API, Memory & contract test for identity storage
Posted by GitBox <gi...@apache.org>.
chibenwa merged pull request #739:
URL: https://github.com/apache/james-project/pull/739
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #739: JAMES-3534 API, Memory & contract test for identity storage
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #739:
URL: https://github.com/apache/james-project/pull/739#discussion_r748026655
##########
File path: server/data/data-jmap/src/main/scala/org/apache/james/jmap/memory/identity/MemoryCustomIdentityDAO.scala
##########
@@ -0,0 +1,49 @@
+/****************************************************************
+ * 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.identity
+
+import com.google.common.collect.{HashBasedTable, Table}
+import org.apache.james.core.Username
+import org.apache.james.jmap.api.identity.{CustomIdentityDAO, IdentityCreationRequest, IdentityNotFoundException, IdentityUpdate}
+import org.apache.james.jmap.api.model.{Identity, IdentityId}
+import org.reactivestreams.Publisher
+import reactor.core.scala.publisher.{SFlux, SMono}
+
+import scala.jdk.CollectionConverters._
+
+class MemoryCustomIdentityDAO extends CustomIdentityDAO {
+ private val table: Table[Username, IdentityId, Identity] = HashBasedTable.create
+
+ override def save(user: Username, creationRequest: IdentityCreationRequest): Publisher[Identity] =
+ SMono.fromCallable(() => IdentityId.generate)
+ .map(creationRequest.asIdentity)
+ .doOnNext(identity => table.put(user, identity.id, identity))
Review comment:
Sure :+1: missed this, good catch.
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on a change in pull request #739: JAMES-3534 API, Memory & contract test for identity storage
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on a change in pull request #739:
URL: https://github.com/apache/james-project/pull/739#discussion_r747195115
##########
File path: server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/Identity.scala
##########
@@ -20,26 +20,22 @@
package org.apache.james.jmap.mail
import java.nio.charset.StandardCharsets
+import java.util.UUID
import com.google.common.collect.ImmutableList
-import com.google.common.hash.Hashing
-import eu.timepit.refined.refineV
import javax.inject.Inject
import org.apache.james.core.MailAddress
-import org.apache.james.jmap.core.Id.Id
import org.apache.james.mailbox.MailboxSession
import org.apache.james.rrt.api.CanSendFrom
import scala.jdk.CollectionConverters._
+import scala.util.Try
case class IdentityName(name: String) extends AnyVal
case class TextSignature(name: String) extends AnyVal
case class HtmlSignature(name: String) extends AnyVal
case class MayDeleteIdentity(value: Boolean) extends AnyVal
-case class IdentityId(id: Id)
-case class IdentityIds(ids: List[IdentityId]) {
- def contains(identityId: IdentityId): Boolean = ids.contains(identityId)
-}
+case class IdentityId(id: UUID)
Review comment:
https://github.com/linagora/james-project/issues/4427#issuecomment-963823454
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on a change in pull request #739: JAMES-3534 API, Memory & contract test for identity storage
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on a change in pull request #739:
URL: https://github.com/apache/james-project/pull/739#discussion_r747967115
##########
File path: server/data/data-jmap/src/main/scala/org/apache/james/jmap/memory/identity/MemoryCustomIdentityDAO.scala
##########
@@ -0,0 +1,49 @@
+/****************************************************************
+ * 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.identity
+
+import com.google.common.collect.{HashBasedTable, Table}
+import org.apache.james.core.Username
+import org.apache.james.jmap.api.identity.{CustomIdentityDAO, IdentityCreationRequest, IdentityNotFoundException, IdentityUpdate}
+import org.apache.james.jmap.api.model.{Identity, IdentityId}
+import org.reactivestreams.Publisher
+import reactor.core.scala.publisher.{SFlux, SMono}
+
+import scala.jdk.CollectionConverters._
+
+class MemoryCustomIdentityDAO extends CustomIdentityDAO {
+ private val table: Table[Username, IdentityId, Identity] = HashBasedTable.create
+
+ override def save(user: Username, creationRequest: IdentityCreationRequest): Publisher[Identity] =
+ SMono.fromCallable(() => IdentityId.generate)
+ .map(creationRequest.asIdentity)
+ .doOnNext(identity => table.put(user, identity.id, identity))
Review comment:
We should set default value for name, textSignature and htmlSignature in case they are empty (see https://jmap.io/spec-mail.html#identities)
##########
File path: server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/model/Identity.scala
##########
@@ -0,0 +1,45 @@
+/****************************************************************
+ * 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 org.apache.james.core.MailAddress
+
+case class IdentityName(name: String) extends AnyVal
+case class TextSignature(name: String) extends AnyVal
+case class HtmlSignature(name: String) extends AnyVal
+case class MayDeleteIdentity(value: Boolean) extends AnyVal
+
+object IdentityId {
+ def generate: IdentityId = IdentityId(UUID.randomUUID())
+}
+case class IdentityId(id: UUID)
+
+case class Identity(id: IdentityId,
+ name: IdentityName,
Review comment:
name: Option[IdentityName]
##########
File path: server/data/data-jmap/src/main/scala/org/apache/james/jmap/memory/identity/MemoryCustomIdentityDAO.scala
##########
@@ -0,0 +1,49 @@
+/****************************************************************
+ * 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.identity
+
+import com.google.common.collect.{HashBasedTable, Table}
+import org.apache.james.core.Username
+import org.apache.james.jmap.api.identity.{CustomIdentityDAO, IdentityCreationRequest, IdentityNotFoundException, IdentityUpdate}
+import org.apache.james.jmap.api.model.{Identity, IdentityId}
+import org.reactivestreams.Publisher
+import reactor.core.scala.publisher.{SFlux, SMono}
+
+import scala.jdk.CollectionConverters._
+
+class MemoryCustomIdentityDAO extends CustomIdentityDAO {
+ private val table: Table[Username, IdentityId, Identity] = HashBasedTable.create
+
+ override def save(user: Username, creationRequest: IdentityCreationRequest): Publisher[Identity] =
+ SMono.fromCallable(() => IdentityId.generate)
+ .map(creationRequest.asIdentity)
+ .doOnNext(identity => table.put(user, identity.id, identity))
Review comment:
Also, adding tests for default values when empty input?
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #739: JAMES-3534 API, Memory & contract test for identity storage
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #739:
URL: https://github.com/apache/james-project/pull/739#discussion_r747191951
##########
File path: server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MDNSendMethod.scala
##########
@@ -76,7 +76,9 @@ class MDNSendMethod @Inject()(serializer: MDNSerializer,
invocation: InvocationWithContext,
mailboxSession: MailboxSession,
request: MDNSendRequest): SFlux[InvocationWithContext] =
- identityResolver.resolveIdentityId(request.identityId, mailboxSession)
+ request.identityId.validate
+ .fold(e => SMono.error(new IllegalArgumentException("The IdentityId cannot be found", e)),
Review comment:
Here we match the ID format but not an UUID so we can confidently say the identity do not exist (which is a convention in JMAP)
Note that I wrote this not to break tests ;-)
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on a change in pull request #739: JAMES-3534 API, Memory & contract test for identity storage
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on a change in pull request #739:
URL: https://github.com/apache/james-project/pull/739#discussion_r747972228
##########
File path: server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/model/Identity.scala
##########
@@ -0,0 +1,45 @@
+/****************************************************************
+ * 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 org.apache.james.core.MailAddress
+
+case class IdentityName(name: String) extends AnyVal
+case class TextSignature(name: String) extends AnyVal
+case class HtmlSignature(name: String) extends AnyVal
+case class MayDeleteIdentity(value: Boolean) extends AnyVal
+
+object IdentityId {
+ def generate: IdentityId = IdentityId(UUID.randomUUID())
+}
+case class IdentityId(id: UUID)
+
+case class Identity(id: IdentityId,
+ name: IdentityName,
Review comment:
Or we refine Identity like this
```scala
case class Identity(id: IdentityId,
name: IdentityName,
email: MailAddress,
replyTo: Option[List[EmailAddress]],
bcc: Option[List[EmailAddress]],
textSignature: TextSignature,
htmlSignature: HtmlSignature,
mayDelete: MayDeleteIdentity)
```
and set default value at IdentityCreationRequest::asIdentity ?
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on a change in pull request #739: JAMES-3534 API, Memory & contract test for identity storage
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on a change in pull request #739:
URL: https://github.com/apache/james-project/pull/739#discussion_r747973033
##########
File path: server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/model/Identity.scala
##########
@@ -0,0 +1,45 @@
+/****************************************************************
+ * 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 org.apache.james.core.MailAddress
+
+case class IdentityName(name: String) extends AnyVal
+case class TextSignature(name: String) extends AnyVal
+case class HtmlSignature(name: String) extends AnyVal
+case class MayDeleteIdentity(value: Boolean) extends AnyVal
+
+object IdentityId {
+ def generate: IdentityId = IdentityId(UUID.randomUUID())
+}
+case class IdentityId(id: UUID)
+
+case class Identity(id: IdentityId,
+ name: IdentityName,
Review comment:
I see name, textSignature and htmlSignature always have value. So Optional here seems not necessary?
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #739: JAMES-3534 API, Memory & contract test for identity storage
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #739:
URL: https://github.com/apache/james-project/pull/739#discussion_r748031189
##########
File path: server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/model/Identity.scala
##########
@@ -0,0 +1,45 @@
+/****************************************************************
+ * 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 org.apache.james.core.MailAddress
+
+case class IdentityName(name: String) extends AnyVal
+case class TextSignature(name: String) extends AnyVal
+case class HtmlSignature(name: String) extends AnyVal
+case class MayDeleteIdentity(value: Boolean) extends AnyVal
+
+object IdentityId {
+ def generate: IdentityId = IdentityId(UUID.randomUUID())
+}
+case class IdentityId(id: UUID)
+
+case class Identity(id: IdentityId,
+ name: IdentityName,
Review comment:
> name: Option[IdentityName]?
This cannot be nulll
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #739: JAMES-3534 API, Memory & contract test for identity storage
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #739:
URL: https://github.com/apache/james-project/pull/739#discussion_r747192123
##########
File path: server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/identity/IdentityDAO.scala
##########
@@ -0,0 +1,113 @@
+package org.apache.james.jmap.api.identity
+
+import java.nio.charset.StandardCharsets
+import java.util.UUID
+
+import com.google.common.collect.ImmutableList
+import javax.inject.Inject
+import org.apache.james.core.{MailAddress, Username}
+import org.apache.james.jmap.api.model.{EmailAddress, HtmlSignature, Identity, IdentityId, IdentityName, MayDeleteIdentity, TextSignature}
+import org.apache.james.rrt.api.CanSendFrom
+import org.reactivestreams.Publisher
+import reactor.core.scala.publisher.{SFlux, SMono}
+import reactor.core.scheduler.Schedulers
+
+import scala.jdk.CollectionConverters._
+import scala.util.Try
+
+case class IdentityCreationRequest(name: IdentityName,
+ email: MailAddress,
+ replyTo: Option[List[EmailAddress]],
+ bcc: Option[List[EmailAddress]],
+ textSignature: Option[TextSignature],
+ htmlSignature: Option[HtmlSignature]) {
+ def asIdentity(id: IdentityId): Identity = Identity(id, name, email, replyTo, bcc, textSignature, htmlSignature, mayDelete = MayDeleteIdentity(true))
+}
+
+trait IdentityUpdate {
+ def update(identity: Identity): Identity
+}
+case class IdentityNameUpdate(name: IdentityName) extends IdentityUpdate {
+ override def update(identity: Identity): Identity = identity.copy(name = name)
+}
+case class IdentityReplyToUpdate(replyTo: Option[List[EmailAddress]]) extends IdentityUpdate {
+ override def update(identity: Identity): Identity = identity.copy(replyTo = replyTo)
+}
+case class IdentityBccUpdate(bcc: Option[List[EmailAddress]]) extends IdentityUpdate {
+ override def update(identity: Identity): Identity = identity.copy(bcc = bcc)
+}
+case class IdentityTextSignatureUpdate(textSignature: Option[TextSignature]) extends IdentityUpdate {
+ override def update(identity: Identity): Identity = identity.copy(textSignature = textSignature)
+}
+case class IdentityHtmlSignatureUpdate(htmlSignature: Option[HtmlSignature]) extends IdentityUpdate {
+ override def update(identity: Identity): Identity = identity.copy(htmlSignature = htmlSignature)
+}
+
+case class IdentityUpdateRequest(name: IdentityNameUpdate,
+ replyTo: IdentityReplyToUpdate,
+ bcc: IdentityBccUpdate,
+ textSignature: IdentityTextSignatureUpdate,
+ htmlSignature: IdentityHtmlSignatureUpdate) {
+ def update(identity: Identity): Identity =
+ List(name, replyTo, bcc, textSignature, htmlSignature)
+ .foldLeft(identity)((acc, update) => update.update(acc))
+}
+
+trait CustomIdentityDAO {
+ def save(user: Username, creationRequest: IdentityCreationRequest): Publisher[Identity]
+
+ def list(user: Username): Publisher[Identity]
+
+ def update(user: Username, identityId: IdentityId, identityUpdate: IdentityUpdate): Publisher[Unit]
+
+ def delete(username: Username, ids: Seq[IdentityId]): Publisher[Unit]
+}
+
+class DefaultIdentitySupplier @Inject()(canSendFrom: CanSendFrom) {
+ def listIdentities(username: Username): List[Identity] =
+ canSendFrom.allValidFromAddressesForUser(username)
+ .collect(ImmutableList.toImmutableList()).asScala.toList
+ .flatMap(address =>
+ from(address).map(id =>
+ Identity(
+ id = id,
+ name = IdentityName(address.asString()),
+ email = address,
+ replyTo = None,
+ bcc = None,
+ textSignature = None,
+ htmlSignature = None,
+ mayDelete = MayDeleteIdentity(false))))
+
+ private def from(address: MailAddress): Option[IdentityId] =
+ Try(UUID.nameUUIDFromBytes(address.asString().getBytes(StandardCharsets.UTF_8)))
+ .toEither
+ .toOption
+ .map(IdentityId)
+}
+
+// This class is intended to merge default (server-set0 identities with (user defined) custom identities
+// Using the custom identities we can stores deltas of the default (server-set) identities allowing to modify them.
+class IdentityRepository @Inject()(customIdentityDao: CustomIdentityDAO, identityFactory: DefaultIdentitySupplier) {
+ def save(user: Username, creationRequest: IdentityCreationRequest): Publisher[Identity] = customIdentityDao.save(user, creationRequest)
+
+ def list(user: Username): Publisher[Identity] = SFlux.merge(Seq(
+ customIdentityDao.list(user),
+ SMono.fromCallable(() => identityFactory.listIdentities(user))
+ .subscribeOn(Schedulers.elastic())
+ .flatMapMany(SFlux.fromIterable)))
+
+ def update(user: Username, identityId: IdentityId, identityUpdate: IdentityUpdate): Publisher[Unit] = customIdentityDao.update(user, identityId, identityUpdate)
+
+ def delete(username: Username, ids: Seq[IdentityId]): Publisher[Unit] = customIdentityDao.delete(username, ids)
Review comment:
I let later tickets do this ;-)
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on a change in pull request #739: JAMES-3534 API, Memory & contract test for identity storage
Posted by GitBox <gi...@apache.org>.
vttranlina commented on a change in pull request #739:
URL: https://github.com/apache/james-project/pull/739#discussion_r747184096
##########
File path: server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/Identity.scala
##########
@@ -20,26 +20,22 @@
package org.apache.james.jmap.mail
import java.nio.charset.StandardCharsets
+import java.util.UUID
import com.google.common.collect.ImmutableList
-import com.google.common.hash.Hashing
-import eu.timepit.refined.refineV
import javax.inject.Inject
import org.apache.james.core.MailAddress
-import org.apache.james.jmap.core.Id.Id
import org.apache.james.mailbox.MailboxSession
import org.apache.james.rrt.api.CanSendFrom
import scala.jdk.CollectionConverters._
+import scala.util.Try
case class IdentityName(name: String) extends AnyVal
case class TextSignature(name: String) extends AnyVal
case class HtmlSignature(name: String) extends AnyVal
case class MayDeleteIdentity(value: Boolean) extends AnyVal
-case class IdentityId(id: Id)
-case class IdentityIds(ids: List[IdentityId]) {
- def contains(identityId: IdentityId): Boolean = ids.contains(identityId)
-}
+case class IdentityId(id: UUID)
Review comment:
Why do we change `Id` to `UUID`?
##########
File path: server/data/data-jmap/src/main/scala/org/apache/james/jmap/api/identity/IdentityDAO.scala
##########
@@ -0,0 +1,113 @@
+package org.apache.james.jmap.api.identity
+
+import java.nio.charset.StandardCharsets
+import java.util.UUID
+
+import com.google.common.collect.ImmutableList
+import javax.inject.Inject
+import org.apache.james.core.{MailAddress, Username}
+import org.apache.james.jmap.api.model.{EmailAddress, HtmlSignature, Identity, IdentityId, IdentityName, MayDeleteIdentity, TextSignature}
+import org.apache.james.rrt.api.CanSendFrom
+import org.reactivestreams.Publisher
+import reactor.core.scala.publisher.{SFlux, SMono}
+import reactor.core.scheduler.Schedulers
+
+import scala.jdk.CollectionConverters._
+import scala.util.Try
+
+case class IdentityCreationRequest(name: IdentityName,
+ email: MailAddress,
+ replyTo: Option[List[EmailAddress]],
+ bcc: Option[List[EmailAddress]],
+ textSignature: Option[TextSignature],
+ htmlSignature: Option[HtmlSignature]) {
+ def asIdentity(id: IdentityId): Identity = Identity(id, name, email, replyTo, bcc, textSignature, htmlSignature, mayDelete = MayDeleteIdentity(true))
+}
+
+trait IdentityUpdate {
+ def update(identity: Identity): Identity
+}
+case class IdentityNameUpdate(name: IdentityName) extends IdentityUpdate {
+ override def update(identity: Identity): Identity = identity.copy(name = name)
+}
+case class IdentityReplyToUpdate(replyTo: Option[List[EmailAddress]]) extends IdentityUpdate {
+ override def update(identity: Identity): Identity = identity.copy(replyTo = replyTo)
+}
+case class IdentityBccUpdate(bcc: Option[List[EmailAddress]]) extends IdentityUpdate {
+ override def update(identity: Identity): Identity = identity.copy(bcc = bcc)
+}
+case class IdentityTextSignatureUpdate(textSignature: Option[TextSignature]) extends IdentityUpdate {
+ override def update(identity: Identity): Identity = identity.copy(textSignature = textSignature)
+}
+case class IdentityHtmlSignatureUpdate(htmlSignature: Option[HtmlSignature]) extends IdentityUpdate {
+ override def update(identity: Identity): Identity = identity.copy(htmlSignature = htmlSignature)
+}
+
+case class IdentityUpdateRequest(name: IdentityNameUpdate,
+ replyTo: IdentityReplyToUpdate,
+ bcc: IdentityBccUpdate,
+ textSignature: IdentityTextSignatureUpdate,
+ htmlSignature: IdentityHtmlSignatureUpdate) {
+ def update(identity: Identity): Identity =
+ List(name, replyTo, bcc, textSignature, htmlSignature)
+ .foldLeft(identity)((acc, update) => update.update(acc))
+}
+
+trait CustomIdentityDAO {
+ def save(user: Username, creationRequest: IdentityCreationRequest): Publisher[Identity]
+
+ def list(user: Username): Publisher[Identity]
+
+ def update(user: Username, identityId: IdentityId, identityUpdate: IdentityUpdate): Publisher[Unit]
+
+ def delete(username: Username, ids: Seq[IdentityId]): Publisher[Unit]
+}
+
+class DefaultIdentitySupplier @Inject()(canSendFrom: CanSendFrom) {
+ def listIdentities(username: Username): List[Identity] =
+ canSendFrom.allValidFromAddressesForUser(username)
+ .collect(ImmutableList.toImmutableList()).asScala.toList
+ .flatMap(address =>
+ from(address).map(id =>
+ Identity(
+ id = id,
+ name = IdentityName(address.asString()),
+ email = address,
+ replyTo = None,
+ bcc = None,
+ textSignature = None,
+ htmlSignature = None,
+ mayDelete = MayDeleteIdentity(false))))
+
+ private def from(address: MailAddress): Option[IdentityId] =
+ Try(UUID.nameUUIDFromBytes(address.asString().getBytes(StandardCharsets.UTF_8)))
+ .toEither
+ .toOption
+ .map(IdentityId)
+}
+
+// This class is intended to merge default (server-set0 identities with (user defined) custom identities
+// Using the custom identities we can stores deltas of the default (server-set) identities allowing to modify them.
+class IdentityRepository @Inject()(customIdentityDao: CustomIdentityDAO, identityFactory: DefaultIdentitySupplier) {
+ def save(user: Username, creationRequest: IdentityCreationRequest): Publisher[Identity] = customIdentityDao.save(user, creationRequest)
+
+ def list(user: Username): Publisher[Identity] = SFlux.merge(Seq(
+ customIdentityDao.list(user),
+ SMono.fromCallable(() => identityFactory.listIdentities(user))
+ .subscribeOn(Schedulers.elastic())
+ .flatMapMany(SFlux.fromIterable)))
+
+ def update(user: Username, identityId: IdentityId, identityUpdate: IdentityUpdate): Publisher[Unit] = customIdentityDao.update(user, identityId, identityUpdate)
+
+ def delete(username: Username, ids: Seq[IdentityId]): Publisher[Unit] = customIdentityDao.delete(username, ids)
Review comment:
Should we check Id is exists in `identityFactory`, if yes => throw an exception?
From https://jmap.io/spec-mail.html#identityset
```
Servers may wish to set this to false for the user’s username or other default address. Attempts to destroy an Identity with mayDelete: false will be rejected with a standard forbidden SetError.
```
##########
File path: server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MDNSendMethod.scala
##########
@@ -76,7 +76,9 @@ class MDNSendMethod @Inject()(serializer: MDNSerializer,
invocation: InvocationWithContext,
mailboxSession: MailboxSession,
request: MDNSendRequest): SFlux[InvocationWithContext] =
- identityResolver.resolveIdentityId(request.identityId, mailboxSession)
+ request.identityId.validate
+ .fold(e => SMono.error(new IllegalArgumentException("The IdentityId cannot be found", e)),
Review comment:
"The IdentityId is invalid"?
##########
File path: server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/IdentityGetMethod.scala
##########
@@ -31,12 +32,12 @@ import org.apache.james.jmap.routes.SessionSupplier
import org.apache.james.mailbox.MailboxSession
import org.apache.james.metrics.api.MetricFactory
import play.api.libs.json.{JsError, JsObject, JsSuccess}
-import reactor.core.scala.publisher.SMono
+import reactor.core.scala.publisher.{SFlux, SMono}
import reactor.core.scheduler.Schedulers
-class IdentityGetMethod @Inject() (identityFactory: IdentityFactory,
- val metricFactory: MetricFactory,
- val sessionSupplier: SessionSupplier) extends MethodRequiringAccountId[IdentityGetRequest] {
+class IdentityGetMethod @Inject() (identityFactory: IdentityRepository,
Review comment:
`identityFactory` -> `identityRepository`, will more clear
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on a change in pull request #739: JAMES-3534 API, Memory & contract test for identity storage
Posted by GitBox <gi...@apache.org>.
quantranhong1999 commented on a change in pull request #739:
URL: https://github.com/apache/james-project/pull/739#discussion_r747264687
##########
File path: server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/IdentityGetMethod.scala
##########
@@ -64,14 +65,17 @@ class IdentityGetMethod @Inject() (identityFactory: IdentityFactory,
private def getIdentities(request: IdentityGetRequest,
mailboxSession: MailboxSession): SMono[IdentityGetResponse] =
- SMono.fromCallable(() => identityFactory.listIdentities(mailboxSession))
+ SFlux(identityRepository.list(mailboxSession.getUser))
+ .collectSeq()
+ .map(_.toList)
.map(request.computeResponse)
}
-case class IdentityResolver @Inject()(identityFactory: IdentityFactory) {
-
+case class IdentityResolver @Inject()(identityFactory: IdentityRepository) {
Review comment:
identityFactory -> identityRepository ?
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on a change in pull request #739: JAMES-3534 API, Memory & contract test for identity storage
Posted by GitBox <gi...@apache.org>.
Arsnael commented on a change in pull request #739:
URL: https://github.com/apache/james-project/pull/739#discussion_r747381151
##########
File path: server/data/data-jmap/src/test/scala/org/apache/james/jmap/api/identity/CustomIdentityDAOContract.scala
##########
@@ -0,0 +1,183 @@
+/****************************************************************
+ * 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.identity
+
+import org.apache.james.core.{MailAddress, Username}
+import org.apache.james.jmap.api.model.{EmailAddress, EmailerName, HtmlSignature, Identity, IdentityId, IdentityName, MayDeleteIdentity, TextSignature}
+import org.assertj.core.api.Assertions.{assertThat, assertThatThrownBy}
+import org.junit.jupiter.api.Test
+import reactor.core.scala.publisher.{SFlux, SMono}
+
+trait CustomIdentityDAOContract {
+ private val bob = Username.of("bob@localhost")
+
+ def testee(): CustomIdentityDAO
+
+ @Test
+ def listShouldReturnEmptyWhenNone(): Unit = {
+ assertThat(SFlux(testee().list(bob)).asJava().collectList().block())
+ .isEmpty()
+ }
+
+ @Test
+ def listShouldReturnSavedIdentities(): Unit = {
Review comment:
Here we list one identity. Should we have a test to list multiple identities?
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #739: JAMES-3534 API, Memory & contract test for identity storage
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #739:
URL: https://github.com/apache/james-project/pull/739#discussion_r747192304
##########
File path: server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/Identity.scala
##########
@@ -20,26 +20,22 @@
package org.apache.james.jmap.mail
import java.nio.charset.StandardCharsets
+import java.util.UUID
import com.google.common.collect.ImmutableList
-import com.google.common.hash.Hashing
-import eu.timepit.refined.refineV
import javax.inject.Inject
import org.apache.james.core.MailAddress
-import org.apache.james.jmap.core.Id.Id
import org.apache.james.mailbox.MailboxSession
import org.apache.james.rrt.api.CanSendFrom
import scala.jdk.CollectionConverters._
+import scala.util.Try
case class IdentityName(name: String) extends AnyVal
case class TextSignature(name: String) extends AnyVal
case class HtmlSignature(name: String) extends AnyVal
case class MayDeleteIdentity(value: Boolean) extends AnyVal
-case class IdentityId(id: Id)
-case class IdentityIds(ids: List[IdentityId]) {
- def contains(identityId: IdentityId): Boolean = ids.contains(identityId)
-}
+case class IdentityId(id: UUID)
Review comment:
UUID are easier to manage, easier to store than the generic format.
--
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-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org