Skip to content

Commit

Permalink
feat: Add support for Table resource tags (#3046)
Browse files Browse the repository at this point in the history
* Refactor labels into annotations to be used for resource tags

* Fix issue where changelog was updated during a refactoring

* Fix issue where changelog was updated during a refactoring

* Add resource tags to Tables

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* fix: clirr

* Update TableInfoTest to include ResourceTags

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
  • Loading branch information
PhongChuong and gcf-owl-bot[bot] authored Jan 12, 2024
1 parent a7ffacf commit 7d61111
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 32 deletions.
5 changes: 5 additions & 0 deletions google-cloud-bigquery/clirr-ignored-differences.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@
<className>com/google/cloud/bigquery/RoutineInfo*</className>
<method>*RemoteFunctionOptions(*)</method>
</difference>
<difference>
<differenceType>7013</differenceType>
<className>com/google/cloud/bigquery/TableInfo*</className>
<method>*ResourceTags(*)</method>
</difference>
<difference>
<differenceType>7012</differenceType>
<className>com/google/cloud/bigquery/Connection</className>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@
import javax.annotation.Nullable;

@AutoValue
abstract class Labels implements Serializable {
abstract class Annotations implements Serializable {
private static final long serialVersionUID = 1L;
static final Labels ZERO = of(Collections.<String, String>emptyMap());
static final Annotations ZERO = of(Collections.<String, String>emptyMap());

@Nullable
abstract Map<String, String> userMap();
Expand Down Expand Up @@ -59,21 +59,21 @@ Map<String, String> toPb() {
return Collections.unmodifiableMap(pbMap);
}

private static Labels of(Map<String, String> userMap) {
private static Annotations of(Map<String, String> userMap) {
Preconditions.checkArgument(
userMap == null || !userMap.containsKey(null), "null keys are not supported");
return new AutoValue_Labels(userMap);
return new AutoValue_Annotations(userMap);
}

static Labels fromUser(Map<String, String> map) {
static Annotations fromUser(Map<String, String> map) {
if (map == null || map instanceof ImmutableMap) {
return of(map);
}
// Not ImmutableMap; we support null values!
return of(Collections.unmodifiableMap(new HashMap<>(map)));
}

static Labels fromPb(Map<String, String> pb) {
static Annotations fromPb(Map<String, String> pb) {
if (Data.isNull(pb)) {
return of(null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public Dataset apply(DatasetInfo datasetInfo) {
private final Long lastModified;
private final String location;
private final String selfLink;
private final Labels labels;
private final Annotations labels;
private final EncryptionConfiguration defaultEncryptionConfiguration;
private final Long defaultPartitionExpirationMs;
private final String defaultCollation;
Expand Down Expand Up @@ -194,7 +194,7 @@ static final class BuilderImpl extends Builder {
private Long lastModified;
private String location;
private String selfLink;
private Labels labels = Labels.ZERO;
private Annotations labels = Annotations.ZERO;
private EncryptionConfiguration defaultEncryptionConfiguration;
private Long defaultPartitionExpirationMs;
private String defaultCollation;
Expand Down Expand Up @@ -247,7 +247,7 @@ public Acl apply(Dataset.Access accessPb) {
this.lastModified = datasetPb.getLastModifiedTime();
this.location = datasetPb.getLocation();
this.selfLink = datasetPb.getSelfLink();
this.labels = Labels.fromPb(datasetPb.getLabels());
this.labels = Annotations.fromPb(datasetPb.getLabels());
if (datasetPb.getDefaultEncryptionConfiguration() != null) {
this.defaultEncryptionConfiguration =
new EncryptionConfiguration.Builder(datasetPb.getDefaultEncryptionConfiguration())
Expand Down Expand Up @@ -337,7 +337,7 @@ Builder setSelfLink(String selfLink) {
*/
@Override
public Builder setLabels(Map<String, String> labels) {
this.labels = Labels.fromUser(labels);
this.labels = Annotations.fromUser(labels);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public Model apply(ModelInfo ModelInfo) {
private final Long creationTime;
private final Long lastModifiedTime;
private final Long expirationTime;
private final Labels labels;
private final Annotations labels;
private final String location;
private final ImmutableList<TrainingRun> trainingRunList;
private final ImmutableList<StandardSQLField> featureColumnList;
Expand Down Expand Up @@ -132,7 +132,7 @@ static class BuilderImpl extends Builder {
private Long creationTime;
private Long lastModifiedTime;
private Long expirationTime;
private Labels labels = Labels.ZERO;
private Annotations labels = Annotations.ZERO;
private String location;
private List<TrainingRun> trainingRunList = Collections.emptyList();
private List<StandardSQLField> labelColumnList = Collections.emptyList();
Expand Down Expand Up @@ -169,7 +169,7 @@ static class BuilderImpl extends Builder {
this.creationTime = modelPb.getCreationTime();
this.lastModifiedTime = modelPb.getLastModifiedTime();
this.expirationTime = modelPb.getExpirationTime();
this.labels = Labels.fromPb(modelPb.getLabels());
this.labels = Annotations.fromPb(modelPb.getLabels());
this.location = modelPb.getLocation();
if (modelPb.getTrainingRuns() != null) {
this.trainingRunList = modelPb.getTrainingRuns();
Expand Down Expand Up @@ -238,7 +238,7 @@ public Builder setModelId(ModelId modelId) {

@Override
public Builder setLabels(Map<String, String> labels) {
this.labels = Labels.fromUser(labels);
this.labels = Annotations.fromUser(labels);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,12 @@ public Builder setLabels(Map<String, String> labels) {
return this;
}

@Override
public Builder setResourceTags(Map<String, String> resourceTags) {
infoBuilder.setResourceTags(resourceTags);
return this;
}

@Override
public Builder setRequirePartitionFilter(Boolean requirePartitionFilter) {
infoBuilder.setRequirePartitionFilter(requirePartitionFilter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ public Table apply(TableInfo tableInfo) {
private final BigInteger numRows;
private final TableDefinition definition;
private final EncryptionConfiguration encryptionConfiguration;
private final Labels labels;
private final Annotations labels;

private final Annotations resourceTags;
private final Boolean requirePartitionFilter;
private final String defaultCollation;

Expand Down Expand Up @@ -152,6 +154,9 @@ public abstract static class Builder {
@BetaApi
public abstract Builder setLabels(Map<String, String> labels);

/** Sets the resource tags applied to this table. */
public abstract Builder setResourceTags(Map<String, String> resourceTags);

/** Creates a {@code TableInfo} object. */
public abstract TableInfo build();

Expand Down Expand Up @@ -191,7 +196,9 @@ static class BuilderImpl extends Builder {
private BigInteger numRows;
private TableDefinition definition;
private EncryptionConfiguration encryptionConfiguration;
private Labels labels = Labels.ZERO;
private Annotations labels = Annotations.ZERO;

private Annotations resourceTags = Annotations.ZERO;
private Boolean requirePartitionFilter;
private String defaultCollation;
private CloneDefinition cloneDefinition;
Expand Down Expand Up @@ -222,6 +229,7 @@ static class BuilderImpl extends Builder {
this.definition = tableInfo.definition;
this.encryptionConfiguration = tableInfo.encryptionConfiguration;
this.labels = tableInfo.labels;
this.resourceTags = tableInfo.resourceTags;
this.requirePartitionFilter = tableInfo.requirePartitionFilter;
this.defaultCollation = tableInfo.defaultCollation;
this.cloneDefinition = tableInfo.cloneDefinition;
Expand Down Expand Up @@ -255,7 +263,8 @@ static class BuilderImpl extends Builder {
this.encryptionConfiguration =
new EncryptionConfiguration.Builder(tablePb.getEncryptionConfiguration()).build();
}
this.labels = Labels.fromPb(tablePb.getLabels());
this.labels = Annotations.fromPb(tablePb.getLabels());
this.resourceTags = Annotations.fromPb(tablePb.getResourceTags());
this.requirePartitionFilter = tablePb.getRequirePartitionFilter();
this.defaultCollation = tablePb.getDefaultCollation();
if (tablePb.getCloneDefinition() != null) {
Expand Down Expand Up @@ -394,7 +403,13 @@ public Builder setEncryptionConfiguration(EncryptionConfiguration configuration)

@Override
public Builder setLabels(Map<String, String> labels) {
this.labels = Labels.fromUser(labels);
this.labels = Annotations.fromUser(labels);
return this;
}

@Override
public Builder setResourceTags(Map<String, String> resourceTags) {
this.resourceTags = Annotations.fromUser(resourceTags);
return this;
}

Expand Down Expand Up @@ -449,6 +464,7 @@ public TableInfo build() {
this.definition = builder.definition;
this.encryptionConfiguration = builder.encryptionConfiguration;
this.labels = builder.labels;
this.resourceTags = builder.resourceTags;
this.requirePartitionFilter = builder.requirePartitionFilter;
this.defaultCollation = builder.defaultCollation;
this.cloneDefinition = builder.cloneDefinition;
Expand Down Expand Up @@ -610,6 +626,11 @@ public Map<String, String> getLabels() {
return labels.userMap();
}

/** Return a map for resource tags applied to the table. */
public Map<String, String> getResourceTags() {
return resourceTags.userMap();
}

/**
* Returns true if a partition filter (that can be used for partition elimination) is required for
* queries over this table.
Expand Down Expand Up @@ -660,6 +681,7 @@ public String toString() {
.add("definition", definition)
.add("encryptionConfiguration", encryptionConfiguration)
.add("labels", labels)
.add("resourceTags", resourceTags)
.add("requirePartitionFilter", requirePartitionFilter)
.add("defaultCollation", defaultCollation)
.add("cloneDefinition", cloneDefinition)
Expand Down Expand Up @@ -724,6 +746,7 @@ Table toPb() {
tablePb.setEncryptionConfiguration(encryptionConfiguration.toPb());
}
tablePb.setLabels(labels.toPb());
tablePb.setResourceTags(resourceTags.toPb());
tablePb.setRequirePartitionFilter(requirePartitionFilter);
if (defaultCollation != null) {
tablePb.setDefaultCollation(defaultCollation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,50 +25,50 @@
import java.util.Map;
import org.junit.Test;

public class LabelsTest {
public class AnnotationsTest {
@Test
public void testFromUser() {
assertThat(Labels.fromUser(null).userMap()).isNull();
assertThat(Annotations.fromUser(null).userMap()).isNull();

HashMap<String, String> user = new HashMap<>();
assertThat(Labels.fromUser(user).userMap()).isEmpty();
assertThat(Annotations.fromUser(user).userMap()).isEmpty();

user.put("a", "b");
Labels labels = Labels.fromUser(user);
assertThat(labels.userMap()).containsExactly("a", "b");
Annotations annotations = Annotations.fromUser(user);
assertThat(annotations.userMap()).containsExactly("a", "b");

// Changing map afterwards does not change the labels.
// Changing map afterwards does not change the annotation.
user.put("c", "d");
assertThat(labels.userMap()).containsExactly("a", "b");
assertThat(annotations.userMap()).containsExactly("a", "b");
}

@Test
public void testFromToPb() {
assertThat(Labels.fromPb(null).toPb()).isNull();
assertThat(Annotations.fromPb(null).toPb()).isNull();

HashMap<String, String> pb = new HashMap<>();
assertThat(Labels.fromPb(pb).toPb()).isNull();
assertThat(Annotations.fromPb(pb).toPb()).isNull();

pb.put("a", "b");
assertThat(Labels.fromPb(pb).toPb()).isEqualTo(pb);
assertThat(Annotations.fromPb(pb).toPb()).isEqualTo(pb);

pb.put("c", Data.NULL_STRING);
assertThat(Labels.fromPb(pb).toPb()).isEqualTo(pb);
assertThat(Annotations.fromPb(pb).toPb()).isEqualTo(pb);

Map<String, String> jsonNullMap = Data.nullOf(HashMap.class);
assertThat(Data.isNull(Labels.fromPb(jsonNullMap).toPb())).isTrue();
assertThat(Data.isNull(Annotations.fromPb(jsonNullMap).toPb())).isTrue();
}

@Test
public void testNullKey() {
try {
Labels.fromUser(Collections.singletonMap((String) null, "foo"));
Annotations.fromUser(Collections.singletonMap((String) null, "foo"));
fail("null key shouldn't work");
} catch (IllegalArgumentException e) {
}

try {
Labels.fromPb(Collections.singletonMap((String) null, "foo"));
Annotations.fromPb(Collections.singletonMap((String) null, "foo"));
fail("null key shouldn't work");
} catch (IllegalArgumentException e) {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ public class TableInfoTest {
.setNumRows(BigInteger.valueOf(NUM_ROWS))
.setSelfLink(SELF_LINK)
.setLabels(Collections.singletonMap("a", "b"))
.setResourceTags(Collections.singletonMap("resourceTagA", "resourceTagB"))
.setRequirePartitionFilter(REQUIRE_PARTITION_FILTER)
.build();
private static final TableInfo VIEW_INFO =
Expand Down Expand Up @@ -309,6 +310,7 @@ private void compareTableInfo(TableInfo expected, TableInfo value) {
assertEquals(expected.getNumRows(), value.getNumRows());
assertEquals(expected.getSelfLink(), value.getSelfLink());
assertEquals(expected.getLabels(), value.getLabels());
assertEquals(expected.getResourceTags(), value.getResourceTags());
assertEquals(expected.getRequirePartitionFilter(), value.getRequirePartitionFilter());
assertEquals(expected.toString(), value.toString());
assertEquals(expected.hashCode(), value.hashCode());
Expand Down

0 comments on commit 7d61111

Please sign in to comment.