diff --git a/genie-web/src/main/resources/static/scripts/components/Pagination.js b/genie-web/src/main/resources/static/scripts/components/Pagination.js index 1c1723fecf0..55898b92cd6 100644 --- a/genie-web/src/main/resources/static/scripts/components/Pagination.js +++ b/genie-web/src/main/resources/static/scripts/components/Pagination.js @@ -16,7 +16,7 @@ const Pagination = props => {
  • @@ -27,7 +27,7 @@ const Pagination = props => {
  • @@ -38,7 +38,7 @@ const Pagination = props => {
  • @@ -49,7 +49,7 @@ const Pagination = props => {
  • diff --git a/genie-web/src/test/java/com/netflix/genie/web/controllers/ClusterRestControllerIntegrationTests.java b/genie-web/src/test/java/com/netflix/genie/web/controllers/ClusterRestControllerIntegrationTests.java index 548f55f4a3b..a1c954dc7c1 100644 --- a/genie-web/src/test/java/com/netflix/genie/web/controllers/ClusterRestControllerIntegrationTests.java +++ b/genie-web/src/test/java/com/netflix/genie/web/controllers/ClusterRestControllerIntegrationTests.java @@ -17,8 +17,11 @@ */ package com.netflix.genie.web.controllers; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; import com.github.fge.jsonpatch.JsonPatch; import com.google.common.collect.Lists; +import com.google.common.collect.Maps; import com.netflix.genie.common.dto.Cluster; import com.netflix.genie.common.dto.ClusterStatus; import com.netflix.genie.common.dto.Command; @@ -27,6 +30,8 @@ import com.netflix.genie.core.jpa.repositories.JpaCommandRepository; import com.netflix.genie.web.aspect.DataServiceRetryAspect; import com.netflix.genie.web.hateoas.resources.ClusterResource; +import org.apache.catalina.util.URLEncoder; +import org.apache.http.client.utils.URLEncodedUtils; import org.hamcrest.Matchers; import org.junit.After; import org.junit.Assert; @@ -44,13 +49,17 @@ import org.springframework.restdocs.request.RequestDocumentation; import org.springframework.restdocs.snippet.Attributes; import org.springframework.retry.RetryListener; +import org.springframework.test.web.servlet.MvcResult; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; import org.springframework.test.web.servlet.result.MockMvcResultMatchers; import javax.sql.DataSource; +import java.net.URI; +import java.net.URLDecoder; import java.nio.charset.StandardCharsets; import java.sql.Connection; import java.sql.Statement; +import java.util.HashMap; import java.util.UUID; /** @@ -1174,4 +1183,97 @@ public void canRemoveCommandFromACluster() throws Exception { .andExpect(MockMvcResultMatchers.jsonPath("$", Matchers.hasSize(1))) .andExpect(MockMvcResultMatchers.jsonPath("$[0].id", Matchers.is(ID))); } + + /** + * This test "documents" a known bug in Spring HATEOAS links that results in doubly-encoded pagination links. + * https://github.com/spring-projects/spring-hateoas/issues/559 + * Currently, we work around this bug in the UI by decoding these elements (see Pagination.js). + * If this test starts failing, it may be because the behavior has been corrected, and the workaround may be + * removed. + * + * @throws Exception on error + */ + @Test + public void testPagingDoubleEncoding() throws Exception { + Assert.assertThat(this.jpaClusterRepository.count(), Matchers.is(0L)); + final String id1 = UUID.randomUUID().toString(); + final String id2 = UUID.randomUUID().toString(); + final String id3 = UUID.randomUUID().toString(); + final String name1 = "Test " + UUID.randomUUID().toString(); + final String name2 = "Test " + UUID.randomUUID().toString(); + final String name3 = "Test " + UUID.randomUUID().toString(); + final String user1 = UUID.randomUUID().toString(); + final String user2 = UUID.randomUUID().toString(); + final String user3 = UUID.randomUUID().toString(); + final String version1 = UUID.randomUUID().toString(); + final String version2 = UUID.randomUUID().toString(); + final String version3 = UUID.randomUUID().toString(); + + this.createConfigResource( + new Cluster.Builder(name1, user1, version1, ClusterStatus.UP).withId(id1).build(), + null + ); + Thread.sleep(1000); + this.createConfigResource( + new Cluster.Builder(name2, user2, version2, ClusterStatus.OUT_OF_SERVICE).withId(id2).build(), + null + ); + Thread.sleep(1000); + this.createConfigResource( + new Cluster.Builder(name3, user3, version3, ClusterStatus.TERMINATED).withId(id3).build(), + null + ); + + Assert.assertThat(this.jpaClusterRepository.count(), Matchers.is(3L)); + + final URLEncoder urlEncoder = new URLEncoder(); + + final String unencodedNameQuery = "Test %"; + final String singleEncodedNameQuery = urlEncoder.encode(unencodedNameQuery, StandardCharsets.UTF_8); + final String doubleEncodedNameQuery = urlEncoder.encode(singleEncodedNameQuery, StandardCharsets.UTF_8); + + // Query by name with wildcard and get the second page containing a single result (out of 3) + final MvcResult response = this.mvc + .perform(MockMvcRequestBuilders.get(CLUSTERS_API) + .param("name", unencodedNameQuery) + .param("size", "1") + .param("page", "1") + ) + .andExpect(MockMvcResultMatchers.status().isOk()) + .andExpect(MockMvcResultMatchers.content().contentTypeCompatibleWith(MediaTypes.HAL_JSON)) + .andExpect(MockMvcResultMatchers.content().encoding(StandardCharsets.UTF_8.name())) + .andExpect(MockMvcResultMatchers.jsonPath(CLUSTERS_LIST_PATH, Matchers.hasSize(1))) + .andReturn(); + + final JsonNode responseJsonNode = new ObjectMapper().readTree(response.getResponse().getContentAsString()); + + // Self link is not double-encoded + Assert.assertTrue( + responseJsonNode + .get("_links") + .get("self") + .get("href") + .asText() + .contains(singleEncodedNameQuery)); + + // Pagination links are double-encoded + + final String[] doubleEncodedHrefs = new String[] { + "first", "next", "prev", "last", + }; + + for (String doubleEncodedHref : doubleEncodedHrefs) { + final String linkString = responseJsonNode.get("_links").get(doubleEncodedHref).get("href").asText(); + Assert.assertNotNull(linkString); + final HashMap params = Maps.newHashMap(); + URLEncodedUtils.parse(new URI(linkString), StandardCharsets.UTF_8) + .forEach(nameValuePair -> params.put(nameValuePair.getName(), nameValuePair.getValue())); + + Assert.assertTrue(params.containsKey("name")); + // Correct: singleEncodedNameQuery, actual: doubleEncodedNameQuery + Assert.assertEquals(doubleEncodedNameQuery, params.get("name")); + final String decoded = URLDecoder.decode(params.get("name"), StandardCharsets.UTF_8.name()); + Assert.assertEquals(singleEncodedNameQuery, decoded); + } + } }