From 1583e2c8b8763a003b5d0d16d27cf53a97410603 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Thu, 25 Feb 2016 16:20:54 +0100 Subject: [PATCH 1/4] Add pageToken to list fields option for GCS and resourcemanager --- .../resourcemanager/ResourceManager.java | 2 +- .../testing/LocalResourceManagerHelper.java | 24 +++++--- .../LocalResourceManagerHelperTest.java | 61 ++++++++++++++++++- .../ResourceManagerImplTest.java | 32 ++++++++++ .../com/google/gcloud/storage/Storage.java | 4 +- .../gcloud/storage/StorageImplTest.java | 12 ++-- 6 files changed, 120 insertions(+), 15 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 b641fa74b584..3d27f2a33ac8 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 @@ -162,7 +162,7 @@ public static ProjectListOption pageSize(int pageSize) { */ public static ProjectListOption fields(ProjectField... fields) { StringBuilder builder = new StringBuilder(); - builder.append("projects(").append(ProjectField.selector(fields)).append(")"); + builder.append("projects(").append(ProjectField.selector(fields)).append("),nextPageToken"); return new ProjectListOption(ResourceManagerRpc.Option.FIELDS, builder.toString()); } } 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 4c26a44cd4e6..2e3b8c25db41 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 @@ -236,10 +236,18 @@ private static Map parseListOptions(String query) throws IOExcep String[] argEntry = arg.split("="); switch (argEntry[0]) { case "fields": - // List fields are in the form "projects(field1, field2, ...)" - options.put( - "fields", - argEntry[1].substring("projects(".length(), argEntry[1].length() - 1).split(",")); + // List fields are in the form "projects(field1, field2, ...),nextPageToken" + String option = argEntry[1]; + int projectStart = option.indexOf("projects("); + int projectEnd = option.indexOf(')'); + if (projectStart != -1 && projectEnd != -1 + && projectEnd > projectStart + "projects(".length()) { + String projectFields = + option.substring(projectStart + "projects(".length(), projectEnd); + options.put("projectFields", projectFields.split(",")); + option = option.replace(option.substring(projectStart, projectEnd), ""); + } + options.put("listFields", option.split(",")); break; case "filter": options.put("filter", argEntry[1].split(" ")); @@ -362,7 +370,7 @@ Response list(Map options) { if (filters != null && !isValidFilter(filters)) { return Error.INVALID_ARGUMENT.response("Could not parse the filter."); } - String[] fields = (String[]) options.get("fields"); + String[] projectFields = (String[]) options.get("projectFields"); int count = 0; String pageToken = (String) options.get("pageToken"); Integer pageSize = (Integer) options.get("pageSize"); @@ -380,7 +388,7 @@ Response list(Map options) { if (includeProject) { count++; try { - projectsSerialized.add(jsonFactory.toString(extractFields(p, fields))); + projectsSerialized.add(jsonFactory.toString(extractFields(p, projectFields))); } catch (IOException e) { return Error.INTERNAL_ERROR.response( "Error when serializing project " + p.getProjectId()); @@ -391,7 +399,9 @@ Response list(Map options) { responseBody.append("{\"projects\": ["); Joiner.on(",").appendTo(responseBody, projectsSerialized); responseBody.append(']'); - if (nextPageToken != null) { + String[] listFields = (String[]) options.get("listFields"); + if (nextPageToken != null && (listFields == null + || ImmutableSet.copyOf(listFields).contains("nextPageToken"))) { responseBody.append(", \"nextPageToken\": \""); responseBody.append(nextPageToken); responseBody.append('"'); 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 6c20c4f1ae0e..978e9c2f0416 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 @@ -333,7 +333,8 @@ public void testListPaging() { @Test public void testListFieldOptions() { Map rpcOptions = new HashMap<>(); - rpcOptions.put(ResourceManagerRpc.Option.FIELDS, "projects(projectId,name,labels)"); + rpcOptions.put(ResourceManagerRpc.Option.FIELDS, + "projects(projectId,name,labels),nextPageToken"); rpc.create(PROJECT_WITH_PARENT); Tuple> projects = rpc.list(rpcOptions); @@ -349,6 +350,64 @@ public void testListFieldOptions() { assertNull(returnedProject.getCreateTime()); } + @Test + public void testListPageTokenFieldOptions() { + Map rpcOptions = new HashMap<>(); + rpcOptions.put(ResourceManagerRpc.Option.PAGE_SIZE, 1); + rpcOptions.put(ResourceManagerRpc.Option.FIELDS, "nextPageToken,projects(projectId,name)"); + rpc.create(PARTIAL_PROJECT); + rpc.create(COMPLETE_PROJECT); + Tuple> projects = + rpc.list(rpcOptions); + assertNotNull(projects.x()); + Iterator iterator = + projects.y().iterator(); + com.google.api.services.cloudresourcemanager.model.Project returnedProject = iterator.next(); + assertEquals(COMPLETE_PROJECT.getProjectId(), returnedProject.getProjectId()); + assertEquals(COMPLETE_PROJECT.getName(), returnedProject.getName()); + assertNull(returnedProject.getLabels()); + assertNull(returnedProject.getParent()); + assertNull(returnedProject.getProjectNumber()); + assertNull(returnedProject.getLifecycleState()); + assertNull(returnedProject.getCreateTime()); + assertFalse(iterator.hasNext()); + rpcOptions.put(ResourceManagerRpc.Option.PAGE_TOKEN, projects.x()); + projects = rpc.list(rpcOptions); + iterator = projects.y().iterator(); + returnedProject = iterator.next(); + assertEquals(PARTIAL_PROJECT.getProjectId(), returnedProject.getProjectId()); + assertEquals(PARTIAL_PROJECT.getName(), returnedProject.getName()); + assertNull(returnedProject.getLabels()); + assertNull(returnedProject.getParent()); + assertNull(returnedProject.getProjectNumber()); + assertNull(returnedProject.getLifecycleState()); + assertNull(returnedProject.getCreateTime()); + assertNull(projects.x()); + } + + @Test + public void testListNoPageTokenFieldOptions() { + Map rpcOptions = new HashMap<>(); + rpcOptions.put(ResourceManagerRpc.Option.PAGE_SIZE, 1); + rpcOptions.put(ResourceManagerRpc.Option.FIELDS, "projects(projectId,name)"); + rpc.create(PARTIAL_PROJECT); + rpc.create(COMPLETE_PROJECT); + Tuple> projects = + rpc.list(rpcOptions); + assertNull(projects.x()); + Iterator iterator = + projects.y().iterator(); + com.google.api.services.cloudresourcemanager.model.Project returnedProject = iterator.next(); + assertEquals(COMPLETE_PROJECT.getProjectId(), returnedProject.getProjectId()); + assertEquals(COMPLETE_PROJECT.getName(), returnedProject.getName()); + assertNull(returnedProject.getLabels()); + assertNull(returnedProject.getParent()); + assertNull(returnedProject.getProjectNumber()); + assertNull(returnedProject.getLifecycleState()); + assertNull(returnedProject.getCreateTime()); + assertFalse(iterator.hasNext()); + } + @Test public void testListFilterOptions() { Map rpcFilterOptions = new HashMap<>(); 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 8d64652a0696..1bc233311a4d 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 @@ -213,6 +213,38 @@ public void testListFieldOptions() { assertSame(RESOURCE_MANAGER, returnedProject.resourceManager()); } + @Test + public void testListPagingWithFieldOptions() { + RESOURCE_MANAGER.create(PARTIAL_PROJECT); + RESOURCE_MANAGER.create(COMPLETE_PROJECT); + Page projects = RESOURCE_MANAGER.list(LIST_FIELDS, ProjectListOption.pageSize(1)); + assertNotNull(projects.nextPageCursor()); + Iterator iterator = projects.values().iterator(); + Project returnedProject = iterator.next(); + assertEquals(COMPLETE_PROJECT.projectId(), returnedProject.projectId()); + assertEquals(COMPLETE_PROJECT.name(), returnedProject.name()); + assertEquals(COMPLETE_PROJECT.labels(), returnedProject.labels()); + assertNull(returnedProject.parent()); + assertNull(returnedProject.projectNumber()); + assertNull(returnedProject.state()); + assertNull(returnedProject.createTimeMillis()); + assertSame(RESOURCE_MANAGER, returnedProject.resourceManager()); + assertFalse(iterator.hasNext()); + projects = projects.nextPage(); + iterator = projects.values().iterator(); + returnedProject = iterator.next(); + assertEquals(PARTIAL_PROJECT.projectId(), returnedProject.projectId()); + assertEquals(PARTIAL_PROJECT.name(), returnedProject.name()); + assertEquals(PARTIAL_PROJECT.labels(), returnedProject.labels()); + assertNull(returnedProject.parent()); + assertNull(returnedProject.projectNumber()); + assertNull(returnedProject.state()); + assertNull(returnedProject.createTimeMillis()); + assertSame(RESOURCE_MANAGER, returnedProject.resourceManager()); + assertFalse(iterator.hasNext()); + assertNull(projects.nextPageCursor()); + } + @Test public void testListFilterOptions() { ProjectInfo matchingProject = ProjectInfo.builder("matching-project") diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java index 0ebe013202b0..4acc1dbcad07 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java @@ -655,7 +655,7 @@ public static BucketListOption prefix(String prefix) { */ public static BucketListOption fields(BucketField... fields) { StringBuilder builder = new StringBuilder(); - builder.append("items(").append(BucketField.selector(fields)).append(")"); + builder.append("items(").append(BucketField.selector(fields)).append("),nextPageToken"); return new BucketListOption(StorageRpc.Option.FIELDS, builder.toString()); } } @@ -708,7 +708,7 @@ public static BlobListOption recursive(boolean recursive) { */ public static BlobListOption fields(BlobField... fields) { StringBuilder builder = new StringBuilder(); - builder.append("items(").append(BlobField.selector(fields)).append(")"); + builder.append("items(").append(BlobField.selector(fields)).append("),nextPageToken"); return new BlobListOption(StorageRpc.Option.FIELDS, builder.toString()); } } diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java index f49938c5e9c1..b14dcd057438 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java @@ -586,7 +586,8 @@ public void testListBucketsWithSelectedFields() { assertTrue(selector.contains("name")); assertTrue(selector.contains("acl")); assertTrue(selector.contains("location")); - assertEquals(24, selector.length()); + assertTrue(selector.contains("nextPageToken")); + assertEquals(38, selector.length()); assertEquals(cursor, page.nextPageCursor()); assertArrayEquals(bucketList.toArray(), Iterables.toArray(page.values(), Bucket.class)); } @@ -606,7 +607,8 @@ public void testListBucketsWithEmptyFields() { String selector = (String) capturedOptions.getValue().get(BLOB_LIST_FIELDS.rpcOption()); assertTrue(selector.contains("items")); assertTrue(selector.contains("name")); - assertEquals(11, selector.length()); + assertTrue(selector.contains("nextPageToken")); + assertEquals(25, selector.length()); assertEquals(cursor, page.nextPageCursor()); assertArrayEquals(bucketList.toArray(), Iterables.toArray(page.values(), Bucket.class)); } @@ -678,7 +680,8 @@ public void testListBlobsWithSelectedFields() { assertTrue(selector.contains("name")); assertTrue(selector.contains("contentType")); assertTrue(selector.contains("md5Hash")); - assertEquals(38, selector.length()); + assertTrue(selector.contains("nextPageToken")); + assertEquals(52, selector.length()); assertEquals(cursor, page.nextPageCursor()); assertArrayEquals(blobList.toArray(), Iterables.toArray(page.values(), Blob.class)); } @@ -706,7 +709,8 @@ public void testListBlobsWithEmptyFields() { assertTrue(selector.contains("items")); assertTrue(selector.contains("bucket")); assertTrue(selector.contains("name")); - assertEquals(18, selector.length()); + assertTrue(selector.contains("nextPageToken")); + assertEquals(32, selector.length()); assertEquals(cursor, page.nextPageCursor()); assertArrayEquals(blobList.toArray(), Iterables.toArray(page.values(), Blob.class)); } From 5fea7ce3b1796f8dea70286a6fbfd2ed7ed0eed9 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Thu, 25 Feb 2016 19:12:00 +0100 Subject: [PATCH 2/4] Use Pattern to match list fields selector --- .../testing/LocalResourceManagerHelper.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 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 2e3b8c25db41..7caf7ef0e84d 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 @@ -37,6 +37,8 @@ import java.util.concurrent.ConcurrentSkipListMap; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.zip.GZIPInputStream; /** @@ -56,6 +58,8 @@ public class LocalResourceManagerHelper { private static final URI BASE_CONTEXT; private static final Set SUPPORTED_COMPRESSION_ENCODINGS = ImmutableSet.of("gzip", "x-gzip"); + private static final Pattern LIST_FIELDS_PATTERN = + Pattern.compile("(.*?)projects\\((.*?)\\)(.*?)"); static { try { @@ -237,17 +241,11 @@ private static Map parseListOptions(String query) throws IOExcep switch (argEntry[0]) { case "fields": // List fields are in the form "projects(field1, field2, ...),nextPageToken" - String option = argEntry[1]; - int projectStart = option.indexOf("projects("); - int projectEnd = option.indexOf(')'); - if (projectStart != -1 && projectEnd != -1 - && projectEnd > projectStart + "projects(".length()) { - String projectFields = - option.substring(projectStart + "projects(".length(), projectEnd); - options.put("projectFields", projectFields.split(",")); - option = option.replace(option.substring(projectStart, projectEnd), ""); + Matcher matcher = LIST_FIELDS_PATTERN.matcher(argEntry[1]); + if (matcher.matches()) { + options.put("projectFields", matcher.group(2).split(",")); + options.put("listFields", (matcher.group(1) + matcher.group(3)).split(",")); } - options.put("listFields", option.split(",")); break; case "filter": options.put("filter", argEntry[1].split(" ")); From a62c69dec52fcb1248c173fa3f2e104af9473048 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Thu, 25 Feb 2016 21:23:25 +0100 Subject: [PATCH 3/4] Handle no project fields are selected on list --- .../testing/LocalResourceManagerHelper.java | 23 +++++++++++++++---- .../LocalResourceManagerHelperTest.java | 17 ++++++++++++++ 2 files changed, 35 insertions(+), 5 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 7caf7ef0e84d..438439c44dd0 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 @@ -60,6 +60,7 @@ public class LocalResourceManagerHelper { ImmutableSet.of("gzip", "x-gzip"); private static final Pattern LIST_FIELDS_PATTERN = Pattern.compile("(.*?)projects\\((.*?)\\)(.*?)"); + private static final String[] NO_FIELDS = {}; static { try { @@ -245,6 +246,9 @@ private static Map parseListOptions(String query) throws IOExcep if (matcher.matches()) { options.put("projectFields", matcher.group(2).split(",")); options.put("listFields", (matcher.group(1) + matcher.group(3)).split(",")); + } else { + options.put("projectFields", NO_FIELDS); + options.put("listFields", argEntry[1].split(",")); } break; case "filter": @@ -393,14 +397,23 @@ Response list(Map options) { } } } - StringBuilder responseBody = new StringBuilder(); - responseBody.append("{\"projects\": ["); - Joiner.on(",").appendTo(responseBody, projectsSerialized); - responseBody.append(']'); String[] listFields = (String[]) options.get("listFields"); + StringBuilder responseBody = new StringBuilder(); + responseBody.append('{'); + boolean commaNeeded = false; + // If fields parameter is set but no project field is selected we must return no projects. + if (!(projectFields != null && projectFields.length == 0)) { + responseBody.append("\"projects\": ["); + Joiner.on(",").appendTo(responseBody, projectsSerialized); + responseBody.append(']'); + commaNeeded = true; + } if (nextPageToken != null && (listFields == null || ImmutableSet.copyOf(listFields).contains("nextPageToken"))) { - responseBody.append(", \"nextPageToken\": \""); + if (commaNeeded) { + responseBody.append(','); + } + responseBody.append("\"nextPageToken\": \""); responseBody.append(nextPageToken); responseBody.append('"'); } 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 978e9c2f0416..a3418fff98ab 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 @@ -408,6 +408,23 @@ public void testListNoPageTokenFieldOptions() { assertFalse(iterator.hasNext()); } + @Test + public void testListPageTokenNoFieldsOptions() { + Map rpcOptions = new HashMap<>(); + rpcOptions.put(ResourceManagerRpc.Option.PAGE_SIZE, 1); + rpcOptions.put(ResourceManagerRpc.Option.FIELDS, "nextPageToken"); + rpc.create(PARTIAL_PROJECT); + rpc.create(COMPLETE_PROJECT); + Tuple> projects = + rpc.list(rpcOptions); + assertNotNull(projects.x()); + assertNull(projects.y()); + rpcOptions.put(ResourceManagerRpc.Option.PAGE_TOKEN, projects.x()); + projects = rpc.list(rpcOptions); + assertNull(projects.x()); + assertNull(projects.y()); + } + @Test public void testListFilterOptions() { Map rpcFilterOptions = new HashMap<>(); From a43962846940a6369ce25209470f5de2c6005e67 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Thu, 25 Feb 2016 23:53:46 +0100 Subject: [PATCH 4/4] Remove non-necessary commaNeeded variable --- .../resourcemanager/testing/LocalResourceManagerHelper.java | 4 +--- 1 file changed, 1 insertion(+), 3 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 438439c44dd0..a077eb6144a5 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 @@ -400,17 +400,15 @@ Response list(Map options) { String[] listFields = (String[]) options.get("listFields"); StringBuilder responseBody = new StringBuilder(); responseBody.append('{'); - boolean commaNeeded = false; // If fields parameter is set but no project field is selected we must return no projects. if (!(projectFields != null && projectFields.length == 0)) { responseBody.append("\"projects\": ["); Joiner.on(",").appendTo(responseBody, projectsSerialized); responseBody.append(']'); - commaNeeded = true; } if (nextPageToken != null && (listFields == null || ImmutableSet.copyOf(listFields).contains("nextPageToken"))) { - if (commaNeeded) { + if (responseBody.length() > 1) { responseBody.append(','); } responseBody.append("\"nextPageToken\": \"");