logging: fix page size not propagating (#3047)

We had two constants called PAGE_SIZE.
This PR collapses them into one and tests building requests.

Author: pongad

google-cloud-logging/pom.xml

@@ -52,6 +52,11 @@
       <type>test-jar</type>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>com.google.truth</groupId>
+      <artifactId>truth</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>com.google.api.grpc</groupId>
       <artifactId>grpc-google-cloud-logging-v2</artifactId>

google-cloud-logging/src/main/java/com/google/cloud/logging/Logging.java

@@ -18,12 +18,11 @@
 
 import com.google.api.core.ApiFuture;
 import com.google.api.gax.paging.AsyncPage;
+import com.google.api.gax.paging.Page;
 import com.google.cloud.MonitoredResource;
 import com.google.cloud.MonitoredResourceDescriptor;
-import com.google.api.gax.paging.Page;
 import com.google.cloud.Service;
 import com.google.common.collect.ImmutableMap;
-
 import java.util.Map;
 
 public interface Logging extends AutoCloseable, Service<LoggingOptions> {
@@ -147,30 +146,31 @@ final class EntryListOption extends Option {
     private static final long serialVersionUID = -1561159676386917050L;
 
     enum OptionType implements Option.OptionType {
-      PAGE_SIZE, PAGE_TOKEN, ORDER_BY, FILTER;
+      ORDER_BY,
+      FILTER;
 
       @SuppressWarnings("unchecked")
       <T> T get(Map<Option.OptionType, ?> options) {
         return (T) options.get(this);
       }
     }
 
-    private EntryListOption(OptionType option, Object value) {
+    private EntryListOption(Option.OptionType option, Object value) {
       super(option, value);
     }
 
     /**
      * Returns an option to specify the maximum number of log entries returned per page.
      */
     public static EntryListOption pageSize(int pageSize) {
-      return new EntryListOption(OptionType.PAGE_SIZE, pageSize);
+      return new EntryListOption(ListOption.OptionType.PAGE_SIZE, pageSize);
     }
 
     /**
      * Returns an option to specify the page token from which to start listing log entries.
      */
     public static EntryListOption pageToken(String pageToken) {
-      return new EntryListOption(OptionType.PAGE_TOKEN, pageToken);
+      return new EntryListOption(ListOption.OptionType.PAGE_TOKEN, pageToken);
     }
 
     /**

google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingImpl.java

@@ -30,11 +30,11 @@
 import com.google.api.core.ApiFutureCallback;
 import com.google.api.core.ApiFutures;
 import com.google.api.gax.paging.AsyncPage;
+import com.google.api.gax.paging.Page;
 import com.google.cloud.AsyncPageImpl;
 import com.google.cloud.BaseService;
 import com.google.cloud.MonitoredResource;
 import com.google.cloud.MonitoredResourceDescriptor;
-import com.google.api.gax.paging.Page;
 import com.google.cloud.PageImpl;
 import com.google.cloud.logging.spi.v2.LoggingRpc;
 import com.google.common.base.Function;
@@ -59,13 +59,10 @@
 import com.google.logging.v2.ListMonitoredResourceDescriptorsResponse;
 import com.google.logging.v2.ListSinksRequest;
 import com.google.logging.v2.ListSinksResponse;
-import com.google.logging.v2.LogName;
-import com.google.logging.v2.MetricName;
 import com.google.logging.v2.ProjectLogName;
 import com.google.logging.v2.ProjectMetricName;
 import com.google.logging.v2.ProjectName;
 import com.google.logging.v2.ProjectSinkName;
-import com.google.logging.v2.SinkName;
 import com.google.logging.v2.UpdateLogMetricRequest;
 import com.google.logging.v2.UpdateSinkRequest;
 import com.google.logging.v2.WriteLogEntriesRequest;
@@ -597,10 +594,10 @@ private ApiFuture<Void> writeAsync(Iterable<LogEntry> logEntries, WriteOption...
         WRITE_RESPONSE_TO_VOID_FUNCTION);
   }
 
-  private static ListLogEntriesRequest listLogEntriesRequest(LoggingOptions serviceOptions,
-      Map<Option.OptionType, ?> options) {
+  static ListLogEntriesRequest listLogEntriesRequest(
+      String projectId, Map<Option.OptionType, ?> options) {
     ListLogEntriesRequest.Builder builder = ListLogEntriesRequest.newBuilder();
-    builder.addProjectIds(serviceOptions.getProjectId());
+    builder.addProjectIds(projectId);
     Integer pageSize = PAGE_SIZE.get(options);
     if (pageSize != null) {
       builder.setPageSize(pageSize);
@@ -622,7 +619,8 @@ private static ListLogEntriesRequest listLogEntriesRequest(LoggingOptions servic
 
   private static ApiFuture<AsyncPage<LogEntry>> listLogEntriesAsync(
       final LoggingOptions serviceOptions, final Map<Option.OptionType, ?> options) {
-    final ListLogEntriesRequest request = listLogEntriesRequest(serviceOptions, options);
+    final ListLogEntriesRequest request =
+        listLogEntriesRequest(serviceOptions.getProjectId(), options);
     ApiFuture<ListLogEntriesResponse> list = serviceOptions.getLoggingRpcV2().list(request);
     return transform(list, new Function<ListLogEntriesResponse, AsyncPage<LogEntry>>() {
       @Override

google-cloud-logging/src/test/java/com/google/cloud/logging/LoggingTest.java

@@ -20,15 +20,15 @@
 import static com.google.cloud.logging.Logging.SortingField;
 import static com.google.cloud.logging.Logging.SortingOrder;
 import static com.google.cloud.logging.Logging.WriteOption;
+import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
 
 import com.google.cloud.MonitoredResource;
 import com.google.cloud.logging.Logging.ListOption;
 import com.google.common.collect.ImmutableMap;
-
-import org.junit.Test;
-
+import com.google.logging.v2.ListLogEntriesRequest;
 import java.util.Map;
+import org.junit.Test;
 
 public class LoggingTest {
 
@@ -56,11 +56,11 @@ public void testListOption() {
   public void testEntryListOption() {
     EntryListOption listOption = EntryListOption.pageToken(PAGE_TOKEN);
     assertEquals(PAGE_TOKEN, listOption.getValue());
-    assertEquals(EntryListOption.OptionType.PAGE_TOKEN, listOption.getOptionType());
+    assertEquals(ListOption.OptionType.PAGE_TOKEN, listOption.getOptionType());
     // page size
     listOption = EntryListOption.pageSize(PAGE_SIZE);
     assertEquals(PAGE_SIZE, listOption.getValue());
-    assertEquals(EntryListOption.OptionType.PAGE_SIZE, listOption.getOptionType());
+    assertEquals(ListOption.OptionType.PAGE_SIZE, listOption.getOptionType());
     // filter
     listOption = EntryListOption.filter(FILTER);
     assertEquals(FILTER, listOption.getValue());
@@ -72,6 +72,14 @@ public void testEntryListOption() {
     listOption = EntryListOption.sortOrder(SortingField.TIMESTAMP, SortingOrder.DESCENDING);
     assertEquals("timestamp desc", listOption.getValue());
     assertEquals(EntryListOption.OptionType.ORDER_BY, listOption.getOptionType());
+
+    ListLogEntriesRequest request =
+        LoggingImpl.listLogEntriesRequest(
+            "some-project-id",
+            LoggingImpl.optionMap(
+                EntryListOption.pageToken(PAGE_TOKEN), EntryListOption.pageSize(PAGE_SIZE)));
+    assertThat(request.getPageToken()).isEqualTo(PAGE_TOKEN);
+    assertThat(request.getPageSize()).isEqualTo(PAGE_SIZE);
   }
 
   @Test