Skip to content

Commit 9d7c142

Browse files
committed
PR Fixes
1 parent 1a878a1 commit 9d7c142

File tree

3 files changed

+64
-20
lines changed

3 files changed

+64
-20
lines changed

src/main/java/com/google/firebase/remoteconfig/Condition.java

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import com.google.firebase.internal.Nullable;
2525
import com.google.firebase.remoteconfig.internal.TemplateResponse.ConditionResponse;
2626

27+
import java.util.Objects;
28+
2729
/**
2830
* Represents a Remote Config condition that can be included in a {@link Template}.
2931
* A condition targets a specific group of users. A list of these conditions make up
@@ -38,7 +40,7 @@ public final class Condition {
3840
/**
3941
* Creates a new {@link Condition}.
4042
*
41-
* @param name A non-null, non-empty, and unique name of this condition.
43+
* @param name A non-null, non-empty, and unique name of this condition.
4244
* @param expression A non-null and non-empty expression of this condition.
4345
*/
4446
public Condition(@NonNull String name, @NonNull String expression) {
@@ -47,7 +49,23 @@ public Condition(@NonNull String name, @NonNull String expression) {
4749
"condition expression must not be null or empty");
4850
this.name = name;
4951
this.expression = expression;
50-
this.tagColor = TagColor.UNSPECIFIED;
52+
}
53+
54+
/**
55+
* Creates a new {@link Condition}.
56+
*
57+
* @param name A non-null, non-empty, and unique name of this condition.
58+
* @param expression A non-null and non-empty expression of this condition.
59+
* @param tagColor A non-null tag color of this condition.
60+
*/
61+
public Condition(@NonNull String name, @NonNull String expression, @NonNull TagColor tagColor) {
62+
checkArgument(!Strings.isNullOrEmpty(name), "condition name must not be null or empty");
63+
checkArgument(!Strings.isNullOrEmpty(expression),
64+
"condition expression must not be null or empty");
65+
checkNotNull(tagColor, "condition tag color must not be null");
66+
this.name = name;
67+
this.expression = expression;
68+
this.tagColor = tagColor;
5169
}
5270

5371
Condition(@NonNull ConditionResponse conditionResponse) {
@@ -64,7 +82,7 @@ public Condition(@NonNull String name, @NonNull String expression) {
6482
/**
6583
* Gets the name of the condition.
6684
*
67-
* @return The {@link String} name of the condition.
85+
* @return The name of the condition.
6886
*/
6987
@NonNull
7088
public String getName() {
@@ -74,7 +92,7 @@ public String getName() {
7492
/**
7593
* Gets the expression of the condition.
7694
*
77-
* @return The {@link String} expression of the condition.
95+
* @return The expression of the condition.
7896
*/
7997
@NonNull
8098
public String getExpression() {
@@ -84,7 +102,7 @@ public String getExpression() {
84102
/**
85103
* Gets the tag color of the condition.
86104
*
87-
* @return The {@link String} tag color of the condition.
105+
* @return The tag color of the condition.
88106
*/
89107
@NonNull
90108
public TagColor getTagColor() {
@@ -127,18 +145,30 @@ public Condition setExpression(@NonNull String expression) {
127145
* with the condition.
128146
*
129147
* @param tagColor The tag color of this condition.
130-
* Passing null sets to {@code TagColor.UNSPECIFIED}
131148
* @return This {@link Condition}.
132149
*/
133150
public Condition setTagColor(@Nullable TagColor tagColor) {
134-
this.tagColor = tagColor == null ? TagColor.UNSPECIFIED : tagColor;
151+
this.tagColor = tagColor;
135152
return this;
136153
}
137154

138155
ConditionResponse toConditionResponse() {
139156
return new ConditionResponse()
140157
.setName(this.name)
141158
.setExpression(this.expression)
142-
.setTagColor(this.tagColor.getColor());
159+
.setTagColor(this.tagColor == null ? null : this.tagColor.getColor());
160+
}
161+
162+
@Override
163+
public boolean equals(Object o) {
164+
if (this == o) {
165+
return true;
166+
}
167+
if (o == null || getClass() != o.getClass()) {
168+
return false;
169+
}
170+
Condition condition = (Condition) o;
171+
return Objects.equals(name, condition.name)
172+
&& Objects.equals(expression, condition.expression) && tagColor == condition.tagColor;
143173
}
144174
}

src/main/java/com/google/firebase/remoteconfig/TagColor.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,24 @@
1+
/*
2+
* Copyright 2020 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
117
package com.google.firebase.remoteconfig;
218

19+
/**
20+
* Colors that are associated with conditions for display purposes.
21+
*/
322
public enum TagColor {
423
BLUE("BLUE"),
524
BROWN("BROWN"),

src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
import com.google.firebase.testing.TestUtils;
4646

4747
import java.io.IOException;
48-
import java.util.ArrayList;
4948
import java.util.List;
5049
import java.util.Map;
5150

@@ -112,20 +111,16 @@ public void testGetTemplate() throws Exception {
112111

113112
// Check Conditions
114113
List<Condition> actualConditions = template.getConditions();
115-
List<Condition> expectedConditions = new ArrayList<>();
116-
expectedConditions
117-
.add(new Condition("ios_en", "device.os == 'ios' && device.country in ['us', 'uk']")
118-
.setTagColor(TagColor.INDIGO));
119-
expectedConditions
120-
.add(new Condition("android_en",
114+
List<Condition> expectedConditions = ImmutableList.of(
115+
new Condition("ios_en", "device.os == 'ios' && device.country in ['us', 'uk']")
116+
.setTagColor(TagColor.INDIGO),
117+
new Condition("android_en",
121118
"device.os == 'android' && device.country in ['us', 'uk']")
122-
.setTagColor(TagColor.UNSPECIFIED));
119+
.setTagColor(TagColor.UNSPECIFIED)
120+
);
123121
assertEquals(expectedConditions.size(), actualConditions.size());
124122
for (int i = 0; i < expectedConditions.size(); i++) {
125-
assertEquals(expectedConditions.get(i).getName(), actualConditions.get(i).getName());
126-
assertEquals(expectedConditions.get(i).getExpression(),
127-
actualConditions.get(i).getExpression());
128-
assertEquals(expectedConditions.get(i).getTagColor(), actualConditions.get(i).getTagColor());
123+
assertEquals(expectedConditions.get(i), actualConditions.get(i));
129124
}
130125
}
131126

0 commit comments

Comments
 (0)