From 1f6a7b3f07fbfb8f9cfdfb7cc83c13f32577c1f4 Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Wed, 17 Feb 2016 17:28:13 -0800 Subject: [PATCH 1/4] support paging --- .../resourcemanager/ResourceManager.java | 5 +--- .../testing/LocalResourceManagerHelper.java | 27 ++++++++++++++----- .../LocalResourceManagerHelperTest.java | 26 +++++++++++++++--- .../ResourceManagerImplTest.java | 18 ++++++++++++- 4 files changed, 62 insertions(+), 14 deletions(-) diff --git a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManager.java b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManager.java index af772dce6b60..b641fa74b584 100644 --- a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManager.java +++ b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManager.java @@ -147,8 +147,6 @@ public static ProjectListOption pageToken(String pageToken) { * *

The server can return fewer projects than requested. When there are more results than the * page size, the server will return a page token that can be used to fetch other results. - * Note: pagination is not yet supported; the server currently ignores this field and returns - * all results. */ public static ProjectListOption pageSize(int pageSize) { return new ProjectListOption(ResourceManagerRpc.Option.PAGE_SIZE, pageSize); @@ -228,8 +226,7 @@ public static ProjectListOption fields(ProjectField... fields) { * *

This method returns projects in an unspecified order. New projects do not necessarily appear * at the end of the list. Use {@link ProjectListOption} to filter this list, set page size, and - * set page tokens. Note that pagination is currently not implemented by the Cloud Resource - * Manager API. + * set page tokens. * * @see Cloud diff --git a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java index 25c763276b3b..f164766ade7a 100644 --- a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java +++ b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java @@ -34,7 +34,7 @@ import java.util.Map; import java.util.Random; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentSkipListMap; import java.util.logging.Level; import java.util.logging.Logger; import java.util.zip.GZIPInputStream; @@ -71,7 +71,7 @@ public class LocalResourceManagerHelper { ImmutableSet.of('-', '\'', '"', ' ', '!'); private final HttpServer server; - private final ConcurrentHashMap projects = new ConcurrentHashMap<>(); + private final ConcurrentSkipListMap projects = new ConcurrentSkipListMap<>(); private final int port; private static class Response { @@ -245,10 +245,10 @@ private static Map parseListOptions(String query) { options.put("filter", argEntry[1].split(" ")); break; case "pageToken": - // support pageToken when Cloud Resource Manager supports this (#421) + options.put("pageToken", argEntry[1]); break; case "pageSize": - // support pageSize when Cloud Resource Manager supports this (#421) + options.put("pageSize", Integer.parseInt(argEntry[1])); break; } } @@ -353,16 +353,27 @@ Response get(String projectId, String[] fields) { } Response list(Map options) { - // Use pageSize and pageToken options when Cloud Resource Manager does so (#421) List projectsSerialized = new ArrayList<>(); String[] filters = (String[]) options.get("filter"); if (filters != null && !isValidFilter(filters)) { return Error.INVALID_ARGUMENT.response("Could not parse the filter."); } String[] fields = (String[]) options.get("fields"); + int count = 0; + String pageToken = (String) options.get("pageToken"); + Integer pageSize = (Integer) options.get("pageSize"); + String nextPageToken = null; for (Project p : projects.values()) { + if (pageToken != null && p.getProjectId().compareTo(pageToken) < 0) { + continue; + } + if (pageSize != null && count >= pageSize) { + nextPageToken = p.getProjectId(); + break; + } boolean includeProject = includeProject(p, filters); if (includeProject) { + count++; try { projectsSerialized.add(jsonFactory.toString(extractFields(p, fields))); } catch (IOException e) { @@ -374,7 +385,11 @@ Response list(Map options) { StringBuilder responseBody = new StringBuilder(); responseBody.append("{\"projects\": ["); Joiner.on(",").appendTo(responseBody, projectsSerialized); - responseBody.append("]}"); + responseBody.append("]"); + if (nextPageToken != null) { + responseBody.append(", \"nextPageToken\": \"" + nextPageToken + "\""); + } + responseBody.append("}"); return new Response(HTTP_OK, responseBody.toString()); } diff --git a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java index 7eb0156d4e56..b5cf579cb860 100644 --- a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java +++ b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java @@ -19,6 +19,7 @@ import org.junit.Test; import java.util.HashMap; +import java.util.Iterator; import java.util.Map; public class LocalResourceManagerHelperTest { @@ -278,7 +279,7 @@ public void testGetWithOptions() { public void testList() { Tuple> projects = rpc.list(EMPTY_RPC_OPTIONS); - assertNull(projects.x()); // change this when #421 is resolved + assertNull(projects.x()); assertFalse(projects.y().iterator().hasNext()); rpc.create(COMPLETE_PROJECT); RESOURCE_MANAGER_HELPER.changeLifecycleState( @@ -296,12 +297,31 @@ public void testList() { } } + @Test + public void testListPaging() { + Map rpcOptions = new HashMap<>(); + rpcOptions.put(ResourceManagerRpc.Option.PAGE_SIZE, 1); + rpc.create(PARTIAL_PROJECT); + rpc.create(COMPLETE_PROJECT); + Tuple> projects = + rpc.list(rpcOptions); + assertNotNull(projects.x()); + Iterator iterator = + projects.y().iterator(); + compareReadWriteFields(COMPLETE_PROJECT, iterator.next()); + assertFalse(iterator.hasNext()); + rpcOptions = new HashMap<>(); + rpcOptions.put(ResourceManagerRpc.Option.PAGE_TOKEN, projects.x()); + projects = rpc.list(rpcOptions); + iterator = projects.y().iterator(); + compareReadWriteFields(PARTIAL_PROJECT, iterator.next()); + assertFalse(iterator.hasNext()); + } + @Test public void testListFieldOptions() { Map rpcOptions = new HashMap<>(); rpcOptions.put(ResourceManagerRpc.Option.FIELDS, "projects(projectId,name,labels)"); - rpcOptions.put(ResourceManagerRpc.Option.PAGE_TOKEN, "somePageToken"); - rpcOptions.put(ResourceManagerRpc.Option.PAGE_SIZE, 1); rpc.create(PROJECT_WITH_PARENT); Tuple> projects = rpc.list(rpcOptions); diff --git a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java index 37c54718fb4a..868d437a6f00 100644 --- a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java +++ b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java @@ -42,6 +42,7 @@ import org.junit.Test; import org.junit.rules.ExpectedException; +import java.util.Iterator; import java.util.Map; public class ResourceManagerImplTest { @@ -166,7 +167,7 @@ public void testGetWithOptions() { @Test public void testList() { Page projects = RESOURCE_MANAGER.list(); - assertFalse(projects.values().iterator().hasNext()); // TODO: change this when #421 is resolved + assertFalse(projects.values().iterator().hasNext()); RESOURCE_MANAGER.create(PARTIAL_PROJECT); RESOURCE_MANAGER.create(COMPLETE_PROJECT); for (Project p : RESOURCE_MANAGER.list().values()) { @@ -181,6 +182,21 @@ public void testList() { } } + @Test + public void tsetListPaging() { + RESOURCE_MANAGER.create(PARTIAL_PROJECT); + RESOURCE_MANAGER.create(COMPLETE_PROJECT); + Page page = RESOURCE_MANAGER.list(ProjectListOption.pageSize(1)); + assertNotNull(page.nextPageCursor()); + Iterator iterator = page.values().iterator(); + compareReadWriteFields(COMPLETE_PROJECT, iterator.next()); + assertFalse(iterator.hasNext()); + page = page.nextPage(); + iterator = page.values().iterator(); + compareReadWriteFields(PARTIAL_PROJECT, iterator.next()); + assertFalse(iterator.hasNext()); + } + @Test public void testListFieldOptions() { RESOURCE_MANAGER.create(COMPLETE_PROJECT); From 6b322fa1a0cacb7c79b1709df78fe6163d48ce07 Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Thu, 18 Feb 2016 08:30:30 -0800 Subject: [PATCH 2/4] minor fixes --- .../testing/LocalResourceManagerHelper.java | 21 ++++++++++++------- .../LocalResourceManagerHelperTest.java | 1 + .../ResourceManagerImplTest.java | 3 ++- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java index f164766ade7a..4a88a9527a34 100644 --- a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java +++ b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java @@ -248,7 +248,9 @@ private static Map parseListOptions(String query) { options.put("pageToken", argEntry[1]); break; case "pageSize": - options.put("pageSize", Integer.parseInt(argEntry[1])); + int pageSize = Integer.valueOf(argEntry[1]); + checkArgument(pageSize > 0, "Page size must be greater than 0."); + options.put("pageSize", pageSize); break; } } @@ -363,10 +365,11 @@ Response list(Map options) { String pageToken = (String) options.get("pageToken"); Integer pageSize = (Integer) options.get("pageSize"); String nextPageToken = null; - for (Project p : projects.values()) { - if (pageToken != null && p.getProjectId().compareTo(pageToken) < 0) { - continue; - } + Map projectsToScan = projects; + if (pageToken != null) { + projectsToScan = projects.tailMap(pageToken); + } + for (Project p : projectsToScan.values()) { if (pageSize != null && count >= pageSize) { nextPageToken = p.getProjectId(); break; @@ -385,11 +388,13 @@ Response list(Map options) { StringBuilder responseBody = new StringBuilder(); responseBody.append("{\"projects\": ["); Joiner.on(",").appendTo(responseBody, projectsSerialized); - responseBody.append("]"); + responseBody.append(']'); if (nextPageToken != null) { - responseBody.append(", \"nextPageToken\": \"" + nextPageToken + "\""); + responseBody.append(", \"nextPageToken\": \""); + responseBody.append(nextPageToken); + responseBody.append('"'); } - responseBody.append("}"); + responseBody.append('}'); return new Response(HTTP_OK, responseBody.toString()); } diff --git a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java index b5cf579cb860..ebcb7dc48a65 100644 --- a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java +++ b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java @@ -316,6 +316,7 @@ public void testListPaging() { iterator = projects.y().iterator(); compareReadWriteFields(PARTIAL_PROJECT, iterator.next()); assertFalse(iterator.hasNext()); + assertNull(projects.x()); } @Test diff --git a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java index 868d437a6f00..8d64652a0696 100644 --- a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java +++ b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java @@ -183,7 +183,7 @@ public void testList() { } @Test - public void tsetListPaging() { + public void testListPaging() { RESOURCE_MANAGER.create(PARTIAL_PROJECT); RESOURCE_MANAGER.create(COMPLETE_PROJECT); Page page = RESOURCE_MANAGER.list(ProjectListOption.pageSize(1)); @@ -195,6 +195,7 @@ public void tsetListPaging() { iterator = page.values().iterator(); compareReadWriteFields(PARTIAL_PROJECT, iterator.next()); assertFalse(iterator.hasNext()); + assertNull(page.nextPageCursor()); } @Test From df32901b0d711c15728d4dcf184f64903fc14f8f Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Thu, 18 Feb 2016 09:51:13 -0800 Subject: [PATCH 3/4] Propagate page size error to user --- .../testing/LocalResourceManagerHelper.java | 6 ++++-- .../LocalResourceManagerHelperTest.java | 11 +++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java index 4a88a9527a34..4c26a44cd4e6 100644 --- a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java +++ b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/testing/LocalResourceManagerHelper.java @@ -228,7 +228,7 @@ private static String[] parseFields(String query) { return null; } - private static Map parseListOptions(String query) { + private static Map parseListOptions(String query) throws IOException { Map options = new HashMap<>(); if (query != null) { String[] args = query.split("&"); @@ -249,7 +249,9 @@ private static Map parseListOptions(String query) { break; case "pageSize": int pageSize = Integer.valueOf(argEntry[1]); - checkArgument(pageSize > 0, "Page size must be greater than 0."); + if (pageSize < 1) { + throw new IOException("Page size must be greater than 0."); + } options.put("pageSize", pageSize); break; } diff --git a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java index ebcb7dc48a65..1c2e0ff9c667 100644 --- a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java +++ b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java @@ -297,6 +297,17 @@ public void testList() { } } + @Test + public void testInvalidListPaging() { + Map rpcOptions = new HashMap<>(); + rpcOptions.put(ResourceManagerRpc.Option.PAGE_SIZE, -1); + try { + rpc.list(rpcOptions); + } catch (Exception e) { + assertEquals("Page size must be greater than 0.", e.getMessage()); + } + } + @Test public void testListPaging() { Map rpcOptions = new HashMap<>(); From b9cd1a7aad8756330522baaa1db676938c9e261c Mon Sep 17 00:00:00 2001 From: Ajay Kannan Date: Thu, 18 Feb 2016 13:40:58 -0800 Subject: [PATCH 4/4] catch ResourceManagerException instead of Exception --- .../gcloud/resourcemanager/LocalResourceManagerHelperTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java index 1c2e0ff9c667..6c20c4f1ae0e 100644 --- a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java +++ b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/LocalResourceManagerHelperTest.java @@ -303,7 +303,7 @@ public void testInvalidListPaging() { rpcOptions.put(ResourceManagerRpc.Option.PAGE_SIZE, -1); try { rpc.list(rpcOptions); - } catch (Exception e) { + } catch (ResourceManagerException e) { assertEquals("Page size must be greater than 0.", e.getMessage()); } }