Skip to content

Commit 253f6c9

Browse files
chore: Refactor TableResult using builder pattern (#3130)
* chore: Refactor TableResult using builder pattern This also removed the redundant EmptyTableResult. These changes are fine as TableResult and EmptyTableResult are internal API only. * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * Remove typo comment in TableResult * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
1 parent 678b6ce commit 253f6c9

File tree

9 files changed

+159
-132
lines changed

9 files changed

+159
-132
lines changed

google-cloud-bigquery/clirr-ignored-differences.xml

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,17 @@
22
<!-- see http://www.mojohaus.org/clirr-maven-plugin/examples/ignored-differences.html -->
33
<differences>
44
<!-- TODO: REMOVE AFTER RELEASE -->
5+
<difference>
6+
<differenceType>3005</differenceType>
7+
<className>com/google/cloud/bigquery/TableResult*</className>
8+
<justification>TableResult is an internal API and it should be fine to update</justification>
9+
</difference>
10+
<difference>
11+
<differenceType>7002</differenceType>
12+
<className>com/google/cloud/bigquery/TableResult*</className>
13+
<method>*TableResult(*)</method>
14+
<justification>TableResult is an internal API and it should be fine to update</justification>
15+
</difference>
516
<difference>
617
<differenceType>7004</differenceType>
718
<className>com/google/cloud/bigquery/spi/v2/BigQueryRpc</className>
@@ -47,6 +58,16 @@
4758
<className>com/google/cloud/bigquery/TableInfo*</className>
4859
<method>*ResourceTags(*)</method>
4960
</difference>
61+
<difference>
62+
<differenceType>7013</differenceType>
63+
<className>com/google/cloud/bigquery/TableResult*</className>
64+
<method>*getPageNoSchema(*)</method>
65+
</difference>
66+
<difference>
67+
<differenceType>7013</differenceType>
68+
<className>com/google/cloud/bigquery/TableResult*</className>
69+
<method>*toBuilder(*)</method>
70+
</difference>
5071
<difference>
5172
<differenceType>7012</differenceType>
5273
<className>com/google/cloud/bigquery/Connection</className>
@@ -142,4 +163,8 @@
142163
<className>com/google/cloud/bigquery/StandardTableDefinition*</className>
143164
<method>*BigLakeConfiguration(*)</method>
144165
</difference>
166+
<difference>
167+
<differenceType>8001</differenceType>
168+
<className>com/google/cloud/bigquery/EmptyTableResult*</className>
169+
</difference>
145170
</differences>

google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQueryImpl.java

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,7 +1156,11 @@ public TableResult listTableData(
11561156
public TableResult listTableData(TableId tableId, Schema schema, TableDataListOption... options) {
11571157
Tuple<? extends Page<FieldValueList>, Long> data =
11581158
listTableData(tableId, schema, getOptions(), optionMap(options));
1159-
return new TableResult(schema, data.y(), data.x(), null);
1159+
return TableResult.newBuilder()
1160+
.setSchema(schema)
1161+
.setTotalRows(data.y())
1162+
.setPageNoSchema(data.x())
1163+
.build();
11601164
}
11611165

11621166
private static Tuple<? extends Page<FieldValueList>, Long> listTableData(
@@ -1400,30 +1404,33 @@ public com.google.api.services.bigquery.model.QueryResponse call() {
14001404
if (results.getPageToken() != null) {
14011405
JobId jobId = JobId.fromPb(results.getJobReference());
14021406
String cursor = results.getPageToken();
1403-
return new TableResult(
1404-
schema,
1405-
numRows,
1406-
new PageImpl<>(
1407-
// fetch next pages of results
1408-
new QueryPageFetcher(jobId, schema, getOptions(), cursor, optionMap(options)),
1409-
cursor,
1410-
// cache first page of result
1411-
transformTableData(results.getRows(), schema)),
1412-
// Return the JobID of the successful job
1413-
jobId,
1414-
results.getQueryId());
1407+
return TableResult.newBuilder()
1408+
.setSchema(schema)
1409+
.setTotalRows(numRows)
1410+
.setPageNoSchema(
1411+
new PageImpl<>(
1412+
// fetch next pages of results
1413+
new QueryPageFetcher(jobId, schema, getOptions(), cursor, optionMap(options)),
1414+
cursor,
1415+
transformTableData(results.getRows(), schema)))
1416+
.setJobId(jobId)
1417+
.setQueryId(results.getQueryId())
1418+
.build();
14151419
}
14161420
// only 1 page of result
1417-
return new TableResult(
1418-
schema,
1419-
numRows,
1420-
new PageImpl<>(
1421-
new TableDataPageFetcher(null, schema, getOptions(), null, optionMap(options)),
1422-
null,
1423-
transformTableData(results.getRows(), schema)),
1421+
return TableResult.newBuilder()
1422+
.setSchema(schema)
1423+
.setTotalRows(numRows)
1424+
.setPageNoSchema(
1425+
new PageImpl<>(
1426+
new TableDataPageFetcher(null, schema, getOptions(), null, optionMap(options)),
1427+
null,
1428+
transformTableData(results.getRows(), schema)))
14241429
// Return the JobID of the successful job
1425-
results.getJobReference() != null ? JobId.fromPb(results.getJobReference()) : null,
1426-
results.getQueryId());
1430+
.setJobId(
1431+
results.getJobReference() != null ? JobId.fromPb(results.getJobReference()) : null)
1432+
.setQueryId(results.getQueryId())
1433+
.build();
14271434
}
14281435

14291436
@Override

google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/EmptyTableResult.java

Lines changed: 0 additions & 32 deletions
This file was deleted.

google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/Job.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.google.api.gax.retrying.BasicResultRetryAlgorithm;
2222
import com.google.api.gax.retrying.RetrySettings;
2323
import com.google.api.gax.retrying.TimedAttemptSettings;
24+
import com.google.cloud.PageImpl;
2425
import com.google.cloud.RetryHelper;
2526
import com.google.cloud.RetryOption;
2627
import com.google.cloud.bigquery.BigQuery.JobOption;
@@ -311,8 +312,13 @@ public TableResult getQueryResults(QueryResultsOption... options)
311312
// Listing table data might fail, such as with CREATE VIEW queries.
312313
// Avoid a tabledata.list API request by returning an empty TableResult.
313314
if (response.getTotalRows() == 0) {
314-
TableResult emptyTableResult = new EmptyTableResult(response.getSchema());
315-
emptyTableResult.setJobId(job.getJobId());
315+
TableResult emptyTableResult =
316+
TableResult.newBuilder()
317+
.setSchema(response.getSchema())
318+
.setJobId(job.getJobId())
319+
.setTotalRows(0L)
320+
.setPageNoSchema(new PageImpl<FieldValueList>(null, "", null))
321+
.build();
316322
return emptyTableResult;
317323
}
318324

@@ -323,8 +329,8 @@ public TableResult getQueryResults(QueryResultsOption... options)
323329
TableResult tableResult =
324330
bigquery.listTableData(
325331
table, response.getSchema(), listOptions.toArray(new TableDataListOption[0]));
326-
tableResult.setJobId(job.getJobId());
327-
return tableResult;
332+
TableResult tableResultWithJobId = tableResult.toBuilder().setJobId(job.getJobId()).build();
333+
return tableResultWithJobId;
328334
}
329335

330336
private QueryResponse waitForQueryResults(

google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/TableResult.java

Lines changed: 54 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,9 @@
1616

1717
package com.google.cloud.bigquery;
1818

19-
import static com.google.common.base.Preconditions.checkNotNull;
20-
2119
import com.google.api.core.InternalApi;
2220
import com.google.api.gax.paging.Page;
21+
import com.google.auto.value.AutoValue;
2322
import com.google.common.base.Function;
2423
import com.google.common.base.MoreObjects;
2524
import com.google.common.collect.Iterables;
@@ -28,108 +27,95 @@
2827
import java.util.Objects;
2928
import javax.annotation.Nullable;
3029

31-
public class TableResult implements Page<FieldValueList>, Serializable {
30+
@InternalApi
31+
@AutoValue
32+
public abstract class TableResult implements Page<FieldValueList>, Serializable {
3233

33-
private static final long serialVersionUID = -4831062717210349819L;
34+
private static final long serialVersionUID = 1L;
3435

35-
@Nullable private final Schema schema;
36-
private final long totalRows;
37-
private final Page<FieldValueList> pageNoSchema;
38-
@Nullable private JobId jobId = null;
36+
@AutoValue.Builder
37+
public abstract static class Builder {
38+
public abstract TableResult.Builder setSchema(Schema schema);
3939

40-
@Nullable private final String queryId;
40+
/**
41+
* Sets the total number of rows in the complete result set, which can be more than the number
42+
* of rows in the first page of results returned by {@link #getValues()}.
43+
*/
44+
public abstract TableResult.Builder setTotalRows(Long totalRows);
4145

42-
// package-private so job id is not set outside the package.
43-
void setJobId(@Nullable JobId jobId) {
44-
this.jobId = jobId;
45-
}
46+
public abstract TableResult.Builder setJobId(JobId jobId);
4647

47-
public JobId getJobId() {
48-
return jobId;
49-
}
48+
public abstract TableResult.Builder setPageNoSchema(Page<FieldValueList> pageNoSchema);
5049

51-
public String getQueryId() {
52-
return queryId;
53-
}
50+
public abstract TableResult.Builder setQueryId(String queryId);
5451

55-
/**
56-
* If {@code schema} is non-null, {@code TableResult} adds the schema to {@code FieldValueList}s
57-
* when iterating through them. {@code pageNoSchema} must not be null.
58-
*/
59-
@InternalApi("Exposed for testing")
60-
public TableResult(
61-
Schema schema, long totalRows, Page<FieldValueList> pageNoSchema, String queryId) {
62-
this.schema = schema;
63-
this.totalRows = totalRows;
64-
this.pageNoSchema = checkNotNull(pageNoSchema);
65-
this.queryId = queryId;
52+
/** Creates a @code TableResult} object. */
53+
public abstract TableResult build();
6654
}
6755

68-
@InternalApi("Exposed for testing")
69-
public TableResult(
70-
Schema schema,
71-
long totalRows,
72-
Page<FieldValueList> pageNoSchema,
73-
JobId jobId,
74-
String queryId) {
75-
this.schema = schema;
76-
this.totalRows = totalRows;
77-
this.pageNoSchema = checkNotNull(pageNoSchema);
78-
this.jobId = jobId;
79-
this.queryId = queryId;
56+
public abstract Builder toBuilder();
57+
58+
public static Builder newBuilder() {
59+
return new AutoValue_TableResult.Builder();
8060
}
8161

8262
/** Returns the schema of the results. Null if the schema is not supplied. */
83-
public Schema getSchema() {
84-
return schema;
85-
}
63+
@Nullable
64+
public abstract Schema getSchema();
8665

87-
/**
88-
* Returns the total number of rows in the complete result set, which can be more than the number
89-
* of rows in the first page of results returned by {@link #getValues()}.
90-
*/
91-
public long getTotalRows() {
92-
return totalRows;
93-
}
66+
public abstract long getTotalRows();
67+
68+
public abstract Page<FieldValueList> getPageNoSchema();
69+
70+
@Nullable
71+
public abstract JobId getJobId();
72+
73+
@Nullable
74+
public abstract String getQueryId();
9475

9576
@Override
9677
public boolean hasNextPage() {
97-
return pageNoSchema.hasNextPage();
78+
return getPageNoSchema().hasNextPage();
9879
}
9980

10081
@Override
10182
public String getNextPageToken() {
102-
return pageNoSchema.getNextPageToken();
83+
return getPageNoSchema().getNextPageToken();
10384
}
10485

10586
@Override
10687
public TableResult getNextPage() {
107-
if (pageNoSchema.hasNextPage()) {
108-
return new TableResult(schema, totalRows, pageNoSchema.getNextPage(), queryId);
88+
if (getPageNoSchema().hasNextPage()) {
89+
return TableResult.newBuilder()
90+
.setSchema(getSchema())
91+
.setTotalRows(getTotalRows())
92+
.setPageNoSchema(getPageNoSchema().getNextPage())
93+
.setQueryId(getQueryId())
94+
.build();
10995
}
11096
return null;
11197
}
11298

11399
@Override
114100
public Iterable<FieldValueList> iterateAll() {
115-
return addSchema(pageNoSchema.iterateAll());
101+
return addSchema(getPageNoSchema().iterateAll());
116102
}
117103

118104
@Override
119105
public Iterable<FieldValueList> getValues() {
120-
return addSchema(pageNoSchema.getValues());
106+
return addSchema(getPageNoSchema().getValues());
121107
}
122108

123109
private Iterable<FieldValueList> addSchema(Iterable<FieldValueList> iter) {
124-
if (schema == null) {
110+
if (getSchema() == null) {
125111
return iter;
126112
}
127113
return Iterables.transform(
128114
iter,
129115
new Function<FieldValueList, FieldValueList>() {
130116
@Override
131117
public FieldValueList apply(FieldValueList list) {
132-
return list.withSchema(schema.getFields());
118+
return list.withSchema(getSchema().getFields());
133119
}
134120
});
135121
}
@@ -138,29 +124,31 @@ public FieldValueList apply(FieldValueList list) {
138124
public String toString() {
139125
return MoreObjects.toStringHelper(this)
140126
.add("rows", getValues())
141-
.add("schema", schema)
142-
.add("totalRows", totalRows)
127+
.add("schema", getSchema())
128+
.add("totalRows", getTotalRows())
143129
.add("cursor", getNextPageToken())
130+
.add("queryId", getQueryId())
144131
.toString();
145132
}
146133

147134
@Override
148135
public final int hashCode() {
149-
return Objects.hash(pageNoSchema, schema, totalRows);
136+
return Objects.hash(getPageNoSchema(), getSchema(), getTotalRows(), getQueryId());
150137
}
151138

152139
@Override
153140
public final boolean equals(Object obj) {
154141
if (obj == this) {
155142
return true;
156143
}
157-
if (obj == null || !obj.getClass().equals(TableResult.class)) {
144+
if (obj == null || !obj.getClass().equals(AutoValue_TableResult.class)) {
158145
return false;
159146
}
160147
TableResult response = (TableResult) obj;
161148
return Objects.equals(getNextPageToken(), response.getNextPageToken())
162149
&& Iterators.elementsEqual(getValues().iterator(), response.getValues().iterator())
163-
&& Objects.equals(schema, response.schema)
164-
&& totalRows == response.totalRows;
150+
&& Objects.equals(getSchema(), response.getSchema())
151+
&& getTotalRows() == response.getTotalRows()
152+
&& getQueryId() == response.getQueryId();
165153
}
166154
}

google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/JobTest.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,12 @@ public void testWaitForAndGetQueryResults() throws InterruptedException {
309309
Job completedJob =
310310
expectedJob.toBuilder().setStatus(new JobStatus(JobStatus.State.RUNNING)).build();
311311
Page<FieldValueList> singlePage = Pages.empty();
312-
TableResult result = new TableResult(Schema.of(), 1, singlePage, null);
312+
TableResult result =
313+
TableResult.newBuilder()
314+
.setSchema(Schema.of())
315+
.setTotalRows(1L)
316+
.setPageNoSchema(singlePage)
317+
.build();
313318
QueryResponse completedQuery =
314319
QueryResponse.newBuilder()
315320
.setCompleted(true)

0 commit comments

Comments
 (0)