You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "Arsnael (via GitHub)" <gi...@apache.org> on 2023/03/01 10:47:58 UTC
[GitHub] [james-project] Arsnael commented on a diff in pull request #1470: JAMES-3893 Add a WebAdmin API allowing listing user default identity
Arsnael commented on code in PR #1470:
URL: https://github.com/apache/james-project/pull/1470#discussion_r1121493217
##########
server/protocols/webadmin/webadmin-jmap/src/main/java/org/apache/james/webadmin/data/jmap/UserIdentityRoutes.java:
##########
@@ -0,0 +1,112 @@
+/****************************************************************
+ * 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.webadmin.data.jmap;
+
+import static org.apache.james.webadmin.Constants.SEPARATOR;
+
+import java.util.Comparator;
+import java.util.List;
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.identity.IdentityRepository;
+import org.apache.james.util.FunctionalUtils;
+import org.apache.james.webadmin.Routes;
+import org.apache.james.webadmin.data.jmap.dto.UserIdentity;
+import org.apache.james.webadmin.utils.ErrorResponder;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.apache.james.webadmin.utils.ParametersExtractor;
+import org.eclipse.jetty.http.HttpStatus;
+
+import reactor.core.publisher.Flux;
+import spark.HaltException;
+import spark.Request;
+import spark.Response;
+import spark.Service;
+
+public class UserIdentityRoutes implements Routes {
+ public static final String USERS = "/users";
+ public static final String IDENTITIES = "/identities";
+ private static final String USER_NAME = ":userName";
+ private Service service;
+ private final IdentityRepository identityRepository;
+ private final JsonTransformer jsonTransformer;
+
+ @Inject
+ public UserIdentityRoutes(IdentityRepository identityRepository,
+ JsonTransformer jsonTransformer) {
+ this.identityRepository = identityRepository;
+ this.jsonTransformer = jsonTransformer;
+ }
+
+ @Override
+ public String getBasePath() {
+ return USERS;
+ }
+
+ @Override
+ public void define(Service service) {
+ this.service = service;
+ getUserIdentities();
+ }
+
+ public void getUserIdentities() {
+ service.get(USERS + SEPARATOR + USER_NAME + SEPARATOR + IDENTITIES, this::listIdentities, jsonTransformer);
Review Comment:
So if I understand correctly the path here would be:
"/users/:userName//identities" right? I think you should take out the first "/" character in your IDENTITIES constant
##########
server/protocols/webadmin/webadmin-jmap/src/main/java/org/apache/james/webadmin/data/jmap/UserIdentityRoutes.java:
##########
@@ -0,0 +1,112 @@
+/****************************************************************
+ * 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.webadmin.data.jmap;
+
+import static org.apache.james.webadmin.Constants.SEPARATOR;
+
+import java.util.Comparator;
+import java.util.List;
+import java.util.Optional;
+
+import javax.inject.Inject;
+
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.identity.IdentityRepository;
+import org.apache.james.util.FunctionalUtils;
+import org.apache.james.webadmin.Routes;
+import org.apache.james.webadmin.data.jmap.dto.UserIdentity;
+import org.apache.james.webadmin.utils.ErrorResponder;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.apache.james.webadmin.utils.ParametersExtractor;
+import org.eclipse.jetty.http.HttpStatus;
+
+import reactor.core.publisher.Flux;
+import spark.HaltException;
+import spark.Request;
+import spark.Response;
+import spark.Service;
+
+public class UserIdentityRoutes implements Routes {
+ public static final String USERS = "/users";
+ public static final String IDENTITIES = "/identities";
+ private static final String USER_NAME = ":userName";
+ private Service service;
+ private final IdentityRepository identityRepository;
+ private final JsonTransformer jsonTransformer;
+
+ @Inject
+ public UserIdentityRoutes(IdentityRepository identityRepository,
+ JsonTransformer jsonTransformer) {
+ this.identityRepository = identityRepository;
+ this.jsonTransformer = jsonTransformer;
+ }
+
+ @Override
+ public String getBasePath() {
+ return USERS;
+ }
+
+ @Override
+ public void define(Service service) {
+ this.service = service;
+ getUserIdentities();
+ }
+
+ public void getUserIdentities() {
+ service.get(USERS + SEPARATOR + USER_NAME + SEPARATOR + IDENTITIES, this::listIdentities, jsonTransformer);
+ }
+
+ private List<UserIdentity> listIdentities(Request request, Response response) {
+ Username username = extractUsername(request);
+ Optional<Boolean> defaultFilter = ParametersExtractor.extractBoolean(request, "default");
+
+ List<UserIdentity> identities = Flux.from(identityRepository.list(username))
+ .map(UserIdentity::from)
+ .collectList()
+ .block();
+
+ return defaultFilter
+ .filter(FunctionalUtils.identityPredicate())
+ .map(queryDefault -> getDefaultIdentity(identities)
+ .map(List::of)
+ .orElseThrow(() -> throw404("Default identity can not be found")))
+ .orElse(identities);
+ }
+
+ private Optional<UserIdentity> getDefaultIdentity(List<UserIdentity> identities) {
+ return identities.stream()
+ .filter(UserIdentity::getMayDelete)
+ .min(Comparator.comparing(UserIdentity::getSortOrder));
+
Review Comment:
useless extra line
##########
server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc:
##########
@@ -671,6 +671,54 @@ Valid status includes:
- `FAILED`: Error encountered while executing this step. Check the logs.
- `ABORTED`: Won't be executed because of previous step failures.
+=== Retrieving the user identities
+
+....
+curl -XGET http://ip:port/users/{baseUser}/identities
+....
+
+API to get the list identity of a user
Review Comment:
```suggestion
API to get the list of identities of a user
```
##########
server/protocols/webadmin/webadmin-jmap/src/test/java/org/apache/james/webadmin/data/jmap/UserIdentitiesRoutesTest.java:
##########
@@ -0,0 +1,292 @@
+/****************************************************************
+ * 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.webadmin.data.jmap;
+
+import static io.restassured.RestAssured.given;
+import static io.restassured.RestAssured.when;
+import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+
+import java.util.List;
+import java.util.Optional;
+
+import org.apache.james.core.MailAddress;
+import org.apache.james.core.Username;
+import org.apache.james.jmap.api.identity.DefaultIdentitySupplier;
+import org.apache.james.jmap.api.identity.IdentityRepository;
+import org.apache.james.jmap.api.model.EmailAddress;
+import org.apache.james.jmap.api.model.Identity;
+import org.apache.james.jmap.memory.identity.MemoryCustomIdentityDAO;
+import org.apache.james.json.DTOConverter;
+import org.apache.james.mime4j.dom.address.Mailbox;
+import org.apache.james.mime4j.dom.address.MailboxList;
+import org.apache.james.task.Hostname;
+import org.apache.james.task.MemoryTaskManager;
+import org.apache.james.webadmin.WebAdminServer;
+import org.apache.james.webadmin.WebAdminUtils;
+import org.apache.james.webadmin.routes.TasksRoutes;
+import org.apache.james.webadmin.utils.JsonTransformer;
+import org.eclipse.jetty.http.HttpStatus;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+import io.restassured.RestAssured;
+import net.javacrumbs.jsonunit.core.Option;
+import reactor.core.publisher.Mono;
+import reactor.core.scala.publisher.SMono;
+import scala.jdk.javaapi.CollectionConverters;
+
+class UserIdentitiesRoutesTest {
+
+ private static final Username BOB = Username.of("bob@domain.tld");
+ private static final String BASE_PATH = "/users";
+ private static final String GET_IDENTITIES_USERS_PATH = "/%s/identities";
+ private WebAdminServer webAdminServer;
+ private IdentityRepository identityRepository;
+ private DefaultIdentitySupplier identityFactory;
+
+ @BeforeEach
+ void setUp() {
+ MemoryTaskManager taskManager = new MemoryTaskManager(new Hostname("foo"));
+ identityFactory = mock(DefaultIdentitySupplier.class);
+ Mockito.when(identityFactory.userCanSendFrom(any(), any())).thenReturn(SMono.just(true).hasElement());
Review Comment:
Is the mock really necessary here?
##########
server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc:
##########
@@ -671,6 +671,54 @@ Valid status includes:
- `FAILED`: Error encountered while executing this step. Check the logs.
- `ABORTED`: Won't be executed because of previous step failures.
+=== Retrieving the user identities
+
+....
+curl -XGET http://ip:port/users/{baseUser}/identities
+....
+
+API to get the list identity of a user
+
+The response will look like:
+
+```
+[
+ {
+ "name":"identity name 1",
+ "email":"bob@domain.tld",
+ "id":"4c039533-75b9-45db-becc-01fb0e747aa8",
+ "mayDelete":true,
+ "textSignature":"textSignature 1",
+ "htmlSignature":"htmlSignature 1",
+ "sortOrder":1,
+ "bcc":[
+ {
+ "emailerName":"bcc name 1",
+ "mailAddress":"bcc1@domain.org"
+ }
+ ],
+ "replyTo":[
+ {
+ "emailerName":"reply name 1",
+ "mailAddress":"reply1@domain.org"
+ }
+ ]
+ }
+]
+```
+
+Response codes:
+
+* 200: The list was successfully retrieved
+* 400: The user is invalid
+* 404: The user is unknown
+
+The optional `default` query parameter allows getting the default identity of a user.
Review Comment:
I would put that param in the curl example above
##########
server/protocols/webadmin/webadmin-jmap/src/main/java/org/apache/james/webadmin/data/jmap/dto/UserIdentity.java:
##########
@@ -0,0 +1,145 @@
+/****************************************************************
+ * 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.webadmin.data.jmap.dto;
+
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.apache.james.core.MailAddress;
+import org.apache.james.jmap.api.model.Identity;
+
+import scala.jdk.javaapi.CollectionConverters;
+import scala.jdk.javaapi.OptionConverters;
+
+public class UserIdentity {
+ public static UserIdentity from(Identity identity) {
+ return new UserIdentity(
+ identity.name(),
+ identity.email().asString(),
+ identity.id().id().toString(),
+ identity.mayDelete(),
+ identity.textSignature(),
+ identity.htmlSignature(),
+ identity.sortOrder(),
+ getBccFromIdentity(identity),
+ getReplyFromIdentity(identity));
+ }
+
+ public static class EmailAddress {
+ public static EmailAddress from(org.apache.james.jmap.api.model.EmailAddress scala) {
+ return new EmailAddress(scala.nameAsString(), scala.email());
+ }
+
+ private String emailerName;
+ private String mailAddress;
+
+ public EmailAddress(String emailerName, MailAddress mailAddress) {
+ this.emailerName = emailerName;
+ this.mailAddress = mailAddress.asString();
+ }
+
+ public String getEmailerName() {
+ return emailerName;
+ }
+
+ public String getMailAddress() {
+ return mailAddress;
+ }
+ }
+
+ private static List<EmailAddress> getBccFromIdentity(Identity identity) {
+ return OptionConverters.toJava(identity.bcc())
+ .map(CollectionConverters::asJava)
+ .orElseGet(List::of)
+ .stream()
+ .map(EmailAddress::from)
+ .collect(Collectors.toList());
+ }
+
+ private static List<EmailAddress> getReplyFromIdentity(Identity identity) {
+ return OptionConverters.toJava(identity.replyTo())
+ .map(CollectionConverters::asJava)
+ .orElseGet(List::of)
+ .stream()
+ .map(EmailAddress::from)
+ .collect(Collectors.toList());
+ }
+
+ private String name;
+ private String email;
+ private String id;
+ private Boolean mayDelete;
+ private String textSignature;
+ private String htmlSignature;
+ private Integer sortOrder;
+ private List<EmailAddress> bcc;
+ private List<EmailAddress> replyTo;
+
+ public UserIdentity(String name, String email, String id,
Review Comment:
Why you need to redeclare this object? I believe we already have one declared in james-server-data-jmap called `Identity`?
--
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