Skip to content

Commit 8150e6a

Browse files
jinseopkim0jinseopkim0
committed
address feedback
1 parent 3a7e91c commit 8150e6a

4 files changed

Lines changed: 75 additions & 21 deletions

File tree

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/models/GcRuleBuilder.java‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,32 +18,46 @@
1818

1919
import com.google.bigtable.admin.v2.GcRule;
2020
import com.google.protobuf.util.Durations;
21-
import org.threeten.bp.Duration;
21+
import java.time.Duration;
2222

2323
/**
2424
* Factory for creating safe GcRule protos.
2525
*
26-
* <p>Use this class to construct GcRules instead of the raw proto builder to avoid common pitfalls
27-
* with "oneof" fields (e.g. accidentally overwriting max age with max versions).
26+
* <p>Use this class to construct {@link GcRule} instances instead of the raw proto builder
27+
* ({@link GcRule#newBuilder()}) to avoid common pitfalls with "oneof" fields (e.g. accidentally
28+
* overwriting max age with max versions).
2829
*/
2930
public final class GcRuleBuilder {
3031
private GcRuleBuilder() {} // Static utility
3132

3233
// Entry points for composite rules
3334

34-
/** Starts building an Intersection (AND) rule. */
35+
/**
36+
* Starts building an Intersection (AND) rule.
37+
*
38+
* @return A new builder for an intersection rule.
39+
*/
3540
public static IntersectionRuleBuilder intersection() {
3641
return new IntersectionRuleBuilder();
3742
}
3843

39-
/** Starts building a Union (OR) rule. */
44+
/**
45+
* Starts building a Union (OR) rule.
46+
*
47+
* @return A new builder for a union rule.
48+
*/
4049
public static UnionRuleBuilder union() {
4150
return new UnionRuleBuilder();
4251
}
4352

4453
// Entry points for simple rules (return the Proto directly)
4554

46-
/** Creates a Max Age rule. */
55+
/**
56+
* Creates a Max Age rule.
57+
*
58+
* @param age The maximum age of the cell.
59+
* @return The constructed GcRule proto.
60+
*/
4761
public static GcRule maxAge(Duration age) {
4862
long seconds = age.getSeconds();
4963
int nanos = age.getNano();
@@ -52,7 +66,12 @@ public static GcRule maxAge(Duration age) {
5266
.build();
5367
}
5468

55-
/** Creates a Max Versions rule. */
69+
/**
70+
* Creates a Max Versions rule.
71+
*
72+
* @param versions The maximum number of versions.
73+
* @return The constructed GcRule proto.
74+
*/
5675
public static GcRule maxVersions(int versions) {
5776
return GcRule.newBuilder().setMaxNumVersions(versions).build();
5877
}

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/models/IntersectionRuleBuilder.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,22 @@
2828
public final class IntersectionRuleBuilder {
2929
private final Intersection.Builder intersectionBuilder = Intersection.newBuilder();
3030

31-
/** Adds a rule to the intersection. */
31+
/**
32+
* Adds a rule to the intersection.
33+
*
34+
* @param rule The rule to add to the intersection.
35+
* @return The builder instance for chaining.
36+
*/
3237
public IntersectionRuleBuilder add(GcRule rule) {
3338
intersectionBuilder.addRules(rule);
3439
return this;
3540
}
3641

37-
/** Builds the final GcRule proto. */
42+
/**
43+
* Builds the final GcRule proto.
44+
*
45+
* @return The constructed GcRule proto.
46+
*/
3847
public GcRule build() {
3948
return GcRule.newBuilder().setIntersection(intersectionBuilder).build();
4049
}

‎google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/admin/v2/models/UnionRuleBuilder.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,22 @@
2828
public final class UnionRuleBuilder {
2929
private final Union.Builder unionBuilder = Union.newBuilder();
3030

31-
/** Adds a rule to the union. */
31+
/**
32+
* Adds a rule to the union.
33+
*
34+
* @param rule The rule to add to the union.
35+
* @return The builder instance for chaining.
36+
*/
3237
public UnionRuleBuilder add(GcRule rule) {
3338
unionBuilder.addRules(rule);
3439
return this;
3540
}
3641

37-
/** Builds the final GcRule proto. */
42+
/**
43+
* Builds the final GcRule proto.
44+
*
45+
* @return The constructed GcRule proto.
46+
*/
3847
public GcRule build() {
3948
return GcRule.newBuilder().setUnion(unionBuilder).build();
4049
}

‎google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/admin/v2/models/GcRuleBuilderTest.java‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import org.junit.Test;
2424
import org.junit.runner.RunWith;
2525
import org.junit.runners.JUnit4;
26-
import org.threeten.bp.Duration;
26+
import java.time.Duration;
2727

2828
@RunWith(JUnit4.class)
2929
public class GcRuleBuilderTest {
@@ -88,17 +88,34 @@ public void union_buildsNestedRules() {
8888

8989
@Test
9090
public void nestedComplexRules_workCorrectly() {
91-
// Union of (Version(1) OR Intersection(Age(1h) AND Version(5)))
92-
GcRule actual =
93-
GcRuleBuilder.union()
94-
.add(GcRuleBuilder.maxVersions(1))
95-
.add(
96-
GcRuleBuilder.intersection()
97-
.add(GcRuleBuilder.maxAge(Duration.ofHours(1)))
98-
.add(GcRuleBuilder.maxVersions(5))
91+
// Expected Proto structure: Union of (Version(1) OR Intersection(Age(1h) AND Version(5)))
92+
GcRule expected = GcRule.newBuilder()
93+
.setUnion(GcRule.Union.newBuilder()
94+
.addRules(GcRule.newBuilder().setMaxNumVersions(1).build())
95+
.addRules(GcRule.newBuilder()
96+
.setIntersection(GcRule.Intersection.newBuilder()
97+
.addRules(GcRule.newBuilder()
98+
.setMaxAge(Durations.fromHours(1))
99+
.build())
100+
.addRules(GcRule.newBuilder().setMaxNumVersions(5).build())
99101
.build())
100-
.build();
102+
.build())
103+
.build())
104+
.build();
105+
106+
// Using the new Builder
107+
GcRule actual = GcRuleBuilder.union()
108+
.add(GcRuleBuilder.maxVersions(1))
109+
.add(GcRuleBuilder.intersection()
110+
.add(GcRuleBuilder.maxAge(Duration.ofHours(1)))
111+
.add(GcRuleBuilder.maxVersions(5))
112+
.build())
113+
.build();
114+
115+
// Verify the structure matches the raw proto construction
116+
assertThat(actual).isEqualTo(expected);
101117

118+
// Verify specific properties
102119
assertThat(actual.getUnion().getRulesCount()).isEqualTo(2);
103120
assertThat(actual.getUnion().getRules(1).getIntersection().getRulesCount()).isEqualTo(2);
104121
}

0 commit comments

Comments
 (0)