From 56f451b30facfa263b5cad375268983918a137c0 Mon Sep 17 00:00:00 2001 From: Jeffrey Griffin Date: Sun, 3 Feb 2019 21:09:06 -0800 Subject: [PATCH] add directory feedback "reason" --- .../controllers/DirectoryController.java | 26 ++++++--- .../entities/DirectoryFeedbackRequest.java | 42 +++++++++++++++ .../controllers/DirectoryControllerTest.java | 54 +++++++++++++++++-- 3 files changed, 112 insertions(+), 10 deletions(-) create mode 100644 src/main/java/org/whispersystems/textsecuregcm/entities/DirectoryFeedbackRequest.java diff --git a/src/main/java/org/whispersystems/textsecuregcm/controllers/DirectoryController.java b/src/main/java/org/whispersystems/textsecuregcm/controllers/DirectoryController.java index 28f061f16..d8a0d0d6e 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/controllers/DirectoryController.java +++ b/src/main/java/org/whispersystems/textsecuregcm/controllers/DirectoryController.java @@ -27,6 +27,7 @@ import org.whispersystems.textsecuregcm.auth.DirectoryCredentialsGenerator; import org.whispersystems.textsecuregcm.entities.ClientContact; import org.whispersystems.textsecuregcm.entities.ClientContactTokens; import org.whispersystems.textsecuregcm.entities.ClientContacts; +import org.whispersystems.textsecuregcm.entities.DirectoryFeedbackRequest; import org.whispersystems.textsecuregcm.limits.RateLimiters; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.Device; @@ -71,17 +72,17 @@ public class DirectoryController { private final Map iosFeedbackMeters = new HashMap() {{ for (String status : FEEDBACK_STATUSES) { - put(status, metricRegistry.meter(name(DirectoryController.class, "feedback", "ios", status))); + put(status, metricRegistry.meter(name(DirectoryController.class, "feedback-v2", "ios", status))); } }}; private final Map androidFeedbackMeters = new HashMap() {{ for (String status : FEEDBACK_STATUSES) { - put(status, metricRegistry.meter(name(DirectoryController.class, "feedback", "android", status))); + put(status, metricRegistry.meter(name(DirectoryController.class, "feedback-v2", "android", status))); } }}; private final Map unknownFeedbackMeters = new HashMap() {{ for (String status : FEEDBACK_STATUSES) { - put(status, metricRegistry.meter(name(DirectoryController.class, "feedback", "unknown", status))); + put(status, metricRegistry.meter(name(DirectoryController.class, "feedback-v2", "unknown", status))); } }}; @@ -107,11 +108,15 @@ public class DirectoryController { } @PUT - @Path("/feedback/{status}") - public Response setFeedback(@Auth Account account, - @PathParam("status") String status) + @Path("/feedback-v2/{status}") + @Consumes(MediaType.APPLICATION_JSON) + @Produces(MediaType.APPLICATION_JSON) + public Response setFeedback(@Auth Account account, + @PathParam("status") String status, + @Valid DirectoryFeedbackRequest request) { Map platformFeedbackMeters = unknownFeedbackMeters; + String platformName = "unknown"; Optional masterDevice = account.getMasterDevice(); if (masterDevice.isPresent()) { @@ -125,6 +130,15 @@ public class DirectoryController { Optional meter = Optional.ofNullable(platformFeedbackMeters.get(status)); if (meter.isPresent()) { meter.get().mark(); + + if (!"ok".equals(status) && + request != null && + request.getReason().isPresent() && + request.getReason().get().length() != 0) + { + logger.info("directory feedback platform=" + platformName + " status=" + status + ": " + request.getReason().get()); + } + return Response.ok().build(); } else { return Response.status(Status.NOT_FOUND).build(); diff --git a/src/main/java/org/whispersystems/textsecuregcm/entities/DirectoryFeedbackRequest.java b/src/main/java/org/whispersystems/textsecuregcm/entities/DirectoryFeedbackRequest.java new file mode 100644 index 000000000..cafb4a779 --- /dev/null +++ b/src/main/java/org/whispersystems/textsecuregcm/entities/DirectoryFeedbackRequest.java @@ -0,0 +1,42 @@ +/* + * Copyright (C) 2019 Open WhisperSystems + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package org.whispersystems.textsecuregcm.entities; + +import com.fasterxml.jackson.annotation.JsonProperty; + +import javax.validation.constraints.Size; +import java.util.Optional; + +public class DirectoryFeedbackRequest { + + @Size(max = 1024) + @JsonProperty + private Optional reason; + + public DirectoryFeedbackRequest() { + } + + public DirectoryFeedbackRequest(Optional reason) { + this.reason = reason; + } + + public Optional getReason() { + return reason; + } + +} diff --git a/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DirectoryControllerTest.java b/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DirectoryControllerTest.java index 13e86c72f..8de8029fd 100644 --- a/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DirectoryControllerTest.java +++ b/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/DirectoryControllerTest.java @@ -10,6 +10,7 @@ import org.whispersystems.textsecuregcm.auth.DirectoryCredentials; import org.whispersystems.textsecuregcm.auth.DirectoryCredentialsGenerator; import org.whispersystems.textsecuregcm.controllers.DirectoryController; import org.whispersystems.textsecuregcm.entities.ClientContactTokens; +import org.whispersystems.textsecuregcm.entities.DirectoryFeedbackRequest; import org.whispersystems.textsecuregcm.limits.RateLimiter; import org.whispersystems.textsecuregcm.limits.RateLimiters; import org.whispersystems.textsecuregcm.storage.Account; @@ -24,6 +25,7 @@ import javax.ws.rs.core.Response.Status; import javax.ws.rs.core.Response.Status.Family; import java.util.LinkedList; import java.util.List; +import java.util.Optional; import io.dropwizard.auth.AuthValueFactoryProvider; import io.dropwizard.testing.junit.ResourceTestRule; @@ -72,10 +74,10 @@ public class DirectoryControllerTest { public void testFeedbackOk() { Response response = resources.getJerseyTest() - .target("/v1/directory/feedback/ok") + .target("/v1/directory/feedback-v2/ok") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, AuthHelper.VALID_PASSWORD)) - .put(Entity.text("")); + .put(Entity.entity(new DirectoryFeedbackRequest(Optional.of("test reason")), MediaType.APPLICATION_JSON_TYPE)); assertThat(response.getStatusInfo().getFamily()).isEqualTo(Family.SUCCESSFUL); } @@ -83,13 +85,57 @@ public class DirectoryControllerTest { public void testNotFoundFeedback() { Response response = resources.getJerseyTest() - .target("/v1/directory/feedback/test-not-found") + .target("/v1/directory/feedback-v2/test-not-found") .request() .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, AuthHelper.VALID_PASSWORD)) - .put(Entity.text("")); + .put(Entity.entity(new DirectoryFeedbackRequest(Optional.of("test reason")), MediaType.APPLICATION_JSON_TYPE)); assertThat(response.getStatusInfo()).isEqualTo(Status.NOT_FOUND); } + @Test + public void testFeedbackEmptyRequest() { + Response response = + resources.getJerseyTest() + .target("/v1/directory/feedback-v2/mismatch") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, AuthHelper.VALID_PASSWORD)) + .put(Entity.json("")); + assertThat(response.getStatusInfo().getFamily()).isEqualTo(Family.SUCCESSFUL); + } + + @Test + public void testFeedbackNoReason() { + Response response = + resources.getJerseyTest() + .target("/v1/directory/feedback-v2/mismatch") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, AuthHelper.VALID_PASSWORD)) + .put(Entity.entity(new DirectoryFeedbackRequest(Optional.empty()), MediaType.APPLICATION_JSON_TYPE)); + assertThat(response.getStatusInfo().getFamily()).isEqualTo(Family.SUCCESSFUL); + } + + @Test + public void testFeedbackEmptyReason() { + Response response = + resources.getJerseyTest() + .target("/v1/directory/feedback-v2/mismatch") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, AuthHelper.VALID_PASSWORD)) + .put(Entity.entity(new DirectoryFeedbackRequest(Optional.of("")), MediaType.APPLICATION_JSON_TYPE)); + assertThat(response.getStatusInfo().getFamily()).isEqualTo(Family.SUCCESSFUL); + } + + @Test + public void testFeedbackTooLargeReason() { + Response response = + resources.getJerseyTest() + .target("/v1/directory/feedback-v2/mismatch") + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, AuthHelper.VALID_PASSWORD)) + .put(Entity.entity(new DirectoryFeedbackRequest(Optional.of(new String(new char[102400]))), MediaType.APPLICATION_JSON_TYPE)); + assertThat(response.getStatusInfo().getFamily()).isEqualTo(Family.CLIENT_ERROR); + } + @Test public void testGetAuthToken() { DirectoryCredentials token =