Skip to content

Commit 7bd2901

Browse files
pongadpongad
authored
logging: fix page size not propagating (#3047)
We had two constants called PAGE_SIZE. This PR collapses them into one and tests building requests.
1 parent 63e028b commit 7bd2901

4 files changed

Lines changed: 30 additions & 19 deletions

File tree

‎google-cloud-logging/pom.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@
5252
<type>test-jar</type>
5353
<scope>test</scope>
5454
</dependency>
55+
<dependency>
56+
<groupId>com.google.truth</groupId>
57+
<artifactId>truth</artifactId>
58+
<scope>test</scope>
59+
</dependency>
5560
<dependency>
5661
<groupId>com.google.api.grpc</groupId>
5762
<artifactId>grpc-google-cloud-logging-v2</artifactId>

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,11 @@
1818

1919
import com.google.api.core.ApiFuture;
2020
import com.google.api.gax.paging.AsyncPage;
21+
import com.google.api.gax.paging.Page;
2122
import com.google.cloud.MonitoredResource;
2223
import com.google.cloud.MonitoredResourceDescriptor;
23-
import com.google.api.gax.paging.Page;
2424
import com.google.cloud.Service;
2525
import com.google.common.collect.ImmutableMap;
26-
2726
import java.util.Map;
2827

2928
public interface Logging extends AutoCloseable, Service<LoggingOptions> {
@@ -147,30 +146,31 @@ final class EntryListOption extends Option {
147146
private static final long serialVersionUID = -1561159676386917050L;
148147

149148
enum OptionType implements Option.OptionType {
150-
PAGE_SIZE, PAGE_TOKEN, ORDER_BY, FILTER;
149+
ORDER_BY,
150+
FILTER;
151151

152152
@SuppressWarnings("unchecked")
153153
<T> T get(Map<Option.OptionType, ?> options) {
154154
return (T) options.get(this);
155155
}
156156
}
157157

158-
private EntryListOption(OptionType option, Object value) {
158+
private EntryListOption(Option.OptionType option, Object value) {
159159
super(option, value);
160160
}
161161

162162
/**
163163
* Returns an option to specify the maximum number of log entries returned per page.
164164
*/
165165
public static EntryListOption pageSize(int pageSize) {
166-
return new EntryListOption(OptionType.PAGE_SIZE, pageSize);
166+
return new EntryListOption(ListOption.OptionType.PAGE_SIZE, pageSize);
167167
}
168168

169169
/**
170170
* Returns an option to specify the page token from which to start listing log entries.
171171
*/
172172
public static EntryListOption pageToken(String pageToken) {
173-
return new EntryListOption(OptionType.PAGE_TOKEN, pageToken);
173+
return new EntryListOption(ListOption.OptionType.PAGE_TOKEN, pageToken);
174174
}
175175

176176
/**

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

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@
3030
import com.google.api.core.ApiFutureCallback;
3131
import com.google.api.core.ApiFutures;
3232
import com.google.api.gax.paging.AsyncPage;
33+
import com.google.api.gax.paging.Page;
3334
import com.google.cloud.AsyncPageImpl;
3435
import com.google.cloud.BaseService;
3536
import com.google.cloud.MonitoredResource;
3637
import com.google.cloud.MonitoredResourceDescriptor;
37-
import com.google.api.gax.paging.Page;
3838
import com.google.cloud.PageImpl;
3939
import com.google.cloud.logging.spi.v2.LoggingRpc;
4040
import com.google.common.base.Function;
@@ -59,13 +59,10 @@
5959
import com.google.logging.v2.ListMonitoredResourceDescriptorsResponse;
6060
import com.google.logging.v2.ListSinksRequest;
6161
import com.google.logging.v2.ListSinksResponse;
62-
import com.google.logging.v2.LogName;
63-
import com.google.logging.v2.MetricName;
6462
import com.google.logging.v2.ProjectLogName;
6563
import com.google.logging.v2.ProjectMetricName;
6664
import com.google.logging.v2.ProjectName;
6765
import com.google.logging.v2.ProjectSinkName;
68-
import com.google.logging.v2.SinkName;
6966
import com.google.logging.v2.UpdateLogMetricRequest;
7067
import com.google.logging.v2.UpdateSinkRequest;
7168
import com.google.logging.v2.WriteLogEntriesRequest;
@@ -597,10 +594,10 @@ private ApiFuture<Void> writeAsync(Iterable<LogEntry> logEntries, WriteOption...
597594
WRITE_RESPONSE_TO_VOID_FUNCTION);
598595
}
599596

600-
private static ListLogEntriesRequest listLogEntriesRequest(LoggingOptions serviceOptions,
601-
Map<Option.OptionType, ?> options) {
597+
static ListLogEntriesRequest listLogEntriesRequest(
598+
String projectId, Map<Option.OptionType, ?> options) {
602599
ListLogEntriesRequest.Builder builder = ListLogEntriesRequest.newBuilder();
603-
builder.addProjectIds(serviceOptions.getProjectId());
600+
builder.addProjectIds(projectId);
604601
Integer pageSize = PAGE_SIZE.get(options);
605602
if (pageSize != null) {
606603
builder.setPageSize(pageSize);
@@ -622,7 +619,8 @@ private static ListLogEntriesRequest listLogEntriesRequest(LoggingOptions servic
622619

623620
private static ApiFuture<AsyncPage<LogEntry>> listLogEntriesAsync(
624621
final LoggingOptions serviceOptions, final Map<Option.OptionType, ?> options) {
625-
final ListLogEntriesRequest request = listLogEntriesRequest(serviceOptions, options);
622+
final ListLogEntriesRequest request =
623+
listLogEntriesRequest(serviceOptions.getProjectId(), options);
626624
ApiFuture<ListLogEntriesResponse> list = serviceOptions.getLoggingRpcV2().list(request);
627625
return transform(list, new Function<ListLogEntriesResponse, AsyncPage<LogEntry>>() {
628626
@Override

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,15 @@
2020
import static com.google.cloud.logging.Logging.SortingField;
2121
import static com.google.cloud.logging.Logging.SortingOrder;
2222
import static com.google.cloud.logging.Logging.WriteOption;
23+
import static com.google.common.truth.Truth.assertThat;
2324
import static org.junit.Assert.assertEquals;
2425

2526
import com.google.cloud.MonitoredResource;
2627
import com.google.cloud.logging.Logging.ListOption;
2728
import com.google.common.collect.ImmutableMap;
28-
29-
import org.junit.Test;
30-
29+
import com.google.logging.v2.ListLogEntriesRequest;
3130
import java.util.Map;
31+
import org.junit.Test;
3232

3333
public class LoggingTest {
3434

@@ -56,11 +56,11 @@ public void testListOption() {
5656
public void testEntryListOption() {
5757
EntryListOption listOption = EntryListOption.pageToken(PAGE_TOKEN);
5858
assertEquals(PAGE_TOKEN, listOption.getValue());
59-
assertEquals(EntryListOption.OptionType.PAGE_TOKEN, listOption.getOptionType());
59+
assertEquals(ListOption.OptionType.PAGE_TOKEN, listOption.getOptionType());
6060
// page size
6161
listOption = EntryListOption.pageSize(PAGE_SIZE);
6262
assertEquals(PAGE_SIZE, listOption.getValue());
63-
assertEquals(EntryListOption.OptionType.PAGE_SIZE, listOption.getOptionType());
63+
assertEquals(ListOption.OptionType.PAGE_SIZE, listOption.getOptionType());
6464
// filter
6565
listOption = EntryListOption.filter(FILTER);
6666
assertEquals(FILTER, listOption.getValue());
@@ -72,6 +72,14 @@ public void testEntryListOption() {
7272
listOption = EntryListOption.sortOrder(SortingField.TIMESTAMP, SortingOrder.DESCENDING);
7373
assertEquals("timestamp desc", listOption.getValue());
7474
assertEquals(EntryListOption.OptionType.ORDER_BY, listOption.getOptionType());
75+
76+
ListLogEntriesRequest request =
77+
LoggingImpl.listLogEntriesRequest(
78+
"some-project-id",
79+
LoggingImpl.optionMap(
80+
EntryListOption.pageToken(PAGE_TOKEN), EntryListOption.pageSize(PAGE_SIZE)));
81+
assertThat(request.getPageToken()).isEqualTo(PAGE_TOKEN);
82+
assertThat(request.getPageSize()).isEqualTo(PAGE_SIZE);
7583
}
7684

7785
@Test

0 commit comments

Comments
 (0)