Skip to content

Commit 7367ceb

Browse files
committed
input validation refactor
1 parent 76669cd commit 7367ceb

File tree

8 files changed

+390
-236
lines changed

8 files changed

+390
-236
lines changed

client/src/main/java/io/split/client/SplitClientImpl.java

Lines changed: 25 additions & 140 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@
99
import io.split.engine.evaluator.Evaluator;
1010
import io.split.engine.SDKReadinessGates;
1111
import io.split.engine.evaluator.EvaluatorImp;
12+
import io.split.engine.inputValidation.EventsValidator;
13+
import io.split.engine.inputValidation.KeyValidator;
14+
import io.split.engine.inputValidation.SplitNameValidator;
15+
import io.split.engine.inputValidation.TrafficTypeValidator;
1216
import io.split.engine.metrics.Metrics;
1317
import io.split.grammar.Treatments;
1418
import org.slf4j.Logger;
@@ -18,7 +22,6 @@
1822
import java.util.HashMap;
1923
import java.util.Map;
2024
import java.util.concurrent.TimeoutException;
21-
import java.util.regex.Pattern;
2225

2326
import static com.google.common.base.Preconditions.checkNotNull;
2427

@@ -28,7 +31,6 @@
2831
* @author adil
2932
*/
3033
public final class SplitClientImpl implements SplitClient {
31-
public static final Pattern EVENT_TYPE_MATCHER = Pattern.compile("^[a-zA-Z0-9][-_.:a-zA-Z0-9]{0,79}$");
3234
public static final SplitResult SPLIT_RESULT_CONTROL = new SplitResult(Treatments.CONTROL, null);
3335

3436
private static final String GET_TREATMENT_LABEL = "sdk.getTreatment";
@@ -70,59 +72,27 @@ public String getTreatment(String key, String split) {
7072

7173
@Override
7274
public String getTreatment(String key, String split, Map<String, Object> attributes) {
73-
return getTreatmentWithConfigInternal(GET_TREATMENT_LABEL, key, null, split, attributes).treatment();
75+
return getTreatmentWithConfigInternal(GET_TREATMENT_LABEL, key, null, split, attributes, "getTreatment").treatment();
7476
}
7577

7678
@Override
7779
public String getTreatment(Key key, String split, Map<String, Object> attributes) {
78-
if (key == null) {
79-
_log.error("getTreatment: you passed a null key, the key must be a non-empty string");
80-
return Treatments.CONTROL;
81-
}
82-
83-
if (key.matchingKey() == null) {
84-
_log.error("getTreatment: you passed a null matchingKey, the matchingKey must be a non-empty string");
85-
return Treatments.CONTROL;
86-
}
87-
88-
89-
if (key.bucketingKey() == null) {
90-
_log.error("getTreatment: you passed a null bucketingKey, the bucketingKey must be a non-empty string");
91-
return Treatments.CONTROL;
92-
}
93-
94-
return getTreatmentWithConfigInternal(GET_TREATMENT_LABEL, key.matchingKey(), key.bucketingKey(), split, attributes).treatment();
80+
return getTreatmentWithConfigInternal(GET_TREATMENT_LABEL, key.matchingKey(), key.bucketingKey(), split, attributes, "getTreatment").treatment();
9581
}
9682

9783
@Override
9884
public SplitResult getTreatmentWithConfig(String key, String split) {
99-
return getTreatmentWithConfigInternal(GET_TREATMENT_LABEL, key, null, split, Collections.<String, Object>emptyMap());
85+
return getTreatmentWithConfigInternal(GET_TREATMENT_LABEL, key, null, split, Collections.<String, Object>emptyMap(), "getTreatmentWithConfig");
10086
}
10187

10288
@Override
10389
public SplitResult getTreatmentWithConfig(String key, String split, Map<String, Object> attributes) {
104-
return getTreatmentWithConfigInternal(GET_TREATMENT_LABEL, key, null, split, attributes);
90+
return getTreatmentWithConfigInternal(GET_TREATMENT_LABEL, key, null, split, attributes, "getTreatmentWithConfig");
10591
}
10692

10793
@Override
10894
public SplitResult getTreatmentWithConfig(Key key, String split, Map<String, Object> attributes) {
109-
if (key == null) {
110-
_log.error("getTreatment: you passed a null key, the key must be a non-empty string");
111-
return SPLIT_RESULT_CONTROL;
112-
}
113-
114-
if (key.matchingKey() == null) {
115-
_log.error("getTreatment: you passed a null matchingKey, the matchingKey must be a non-empty string");
116-
return SPLIT_RESULT_CONTROL;
117-
}
118-
119-
120-
if (key.bucketingKey() == null) {
121-
_log.error("getTreatment: you passed a null bucketingKey, the bucketingKey must be a non-empty string");
122-
return SPLIT_RESULT_CONTROL;
123-
}
124-
125-
return getTreatmentWithConfigInternal(GET_TREATMENT_LABEL, key.matchingKey(), key.bucketingKey(), split, attributes);
95+
return getTreatmentWithConfigInternal(GET_TREATMENT_LABEL, key.matchingKey(), key.bucketingKey(), split, attributes, "getTreatmentWithConfig");
12696
}
12797

12898
@Override
@@ -178,138 +148,53 @@ private boolean track(Event event) {
178148
}
179149

180150
// Traffic Type validations
181-
if (event.trafficTypeName == null) {
182-
_log.error("track: you passed a null trafficTypeName, trafficTypeName must be a non-empty string");
183-
return false;
184-
}
185-
186-
if (event.trafficTypeName.isEmpty()) {
187-
_log.error("track: you passed an empty trafficTypeName, trafficTypeName must be a non-empty string");
151+
TrafficTypeValidator.TrafficTypeResult trafficTypeResult = TrafficTypeValidator.isValid(event.trafficTypeName, _splitCache, "track");
152+
if (!trafficTypeResult.getSuccess()) {
188153
return false;
189154
}
190-
191-
if (!event.trafficTypeName.equals(event.trafficTypeName.toLowerCase())) {
192-
_log.warn("track: trafficTypeName should be all lowercase - converting string to lowercase");
193-
event.trafficTypeName = event.trafficTypeName.toLowerCase();
194-
}
195-
196-
if (!_splitCache.trafficTypeExists(event.trafficTypeName)) {
197-
_log.warn("track: Traffic Type " + event.trafficTypeName + " does not have any corresponding Splits in this environment, " +
198-
"make sure you’re tracking your events to a valid traffic type defined in the Split console.");
199-
}
155+
event.trafficTypeName = trafficTypeResult.getValue();
200156

201157
// EventType validations
202-
if (event.eventTypeId == null) {
203-
_log.error("track: you passed a null eventTypeId, eventTypeId must be a non-empty string");
204-
return false;
205-
}
206-
207-
if (event.eventTypeId.isEmpty()) {
208-
_log.error("track:you passed an empty eventTypeId, eventTypeId must be a non-empty string");
209-
return false;
210-
}
211-
212-
if (!EVENT_TYPE_MATCHER.matcher(event.eventTypeId).find()) {
213-
_log.error("track: you passed " + event.eventTypeId + ", eventTypeId must adhere to the regular expression " +
214-
"[a-zA-Z0-9][-_.:a-zA-Z0-9]{0,79}. This means an eventTypeID must be alphanumeric, " +
215-
"cannot be more than 80 characters long, and can only include a dash, underscore, period, " +
216-
"or colon as separators of alphanumeric characters");
158+
if (!EventsValidator.typeIsValid(event.eventTypeId, "track")) {
217159
return false;
218160
}
219161

220162
// Key Validations
221-
if (event.key == null) {
222-
_log.error("track: you passed a null key, key must be a non-empty string");
163+
if (!KeyValidator.isValid(event.key, "key", _config.maxStringLength(), "track")) {
223164
return false;
224165
}
225166

226-
if (event.key.isEmpty()) {
227-
_log.error("track: you passed an empty key, key must be a non-empty string");
167+
// Properties validations
168+
EventsValidator.EventValidatorResult propertiesResult = EventsValidator.propertiesAreValid(event.properties);
169+
if (!propertiesResult.getSuccess()) {
228170
return false;
229171
}
230172

231-
if (event.key.length() > _config.maxStringLength()) {
232-
_log.error("track: key too long - must be " + _config.maxStringLength() + "characters or less");
233-
return false;
234-
}
235-
236-
int size = 1024; // We assume 1kb events without properties (750 bytes avg measured)
237-
if (null != event.properties) {
238-
if (event.properties.size() > 300) {
239-
_log.warn("Event has more than 300 properties. Some of them will be trimmed when processed");
240-
}
241-
242-
for (Map.Entry<String, Object> entry: event.properties.entrySet()) {
243-
size += entry.getKey().length();
244-
Object value = entry.getValue();
245-
if (null == value) {
246-
continue;
247-
}
248-
249-
if (!(value instanceof Number) && !(value instanceof Boolean) && !(value instanceof String)) {
250-
_log.warn(String.format("Property %s is of invalid type. Setting value to null", entry.getKey()));
251-
entry.setValue(null);
252-
}
253-
254-
if (value instanceof String) {
255-
size += ((String) value).length();
256-
}
257-
258-
if (size > Event.MAX_PROPERTIES_LENGTH_BYTES) {
259-
_log.error(String.format("The maximum size allowed for the properties is 32768 bytes. "
260-
+ "Current one is %s bytes. Event not queued", size));
261-
return false;
262-
}
263-
}
264-
265-
}
173+
event.properties = propertiesResult.getValue();
266174

267-
return _eventClient.track(event, size);
175+
return _eventClient.track(event, propertiesResult.getEventSize());
268176
}
269177

270-
private SplitResult getTreatmentWithConfigInternal(String label, String matchingKey, String bucketingKey, String split, Map<String, Object> attributes) {
178+
private SplitResult getTreatmentWithConfigInternal(String label, String matchingKey, String bucketingKey, String split, Map<String, Object> attributes, String method) {
271179
try {
272180
if (_container.isDestroyed()) {
273181
_log.error("Client has already been destroyed - no calls possible");
274182
return SPLIT_RESULT_CONTROL;
275183
}
276184

277-
if (matchingKey == null) {
278-
_log.error("getTreatmentWithConfig: you passed a null matchingKey, the matchingKey must be a non-empty string");
279-
return SPLIT_RESULT_CONTROL;
280-
}
281-
if (matchingKey.length() > _config.maxStringLength()) {
282-
_log.error("getTreatmentWithConfig: matchingKey too long - must be " + _config.maxStringLength() + " characters or less");
283-
return SPLIT_RESULT_CONTROL;
284-
}
285-
if (matchingKey.isEmpty()) {
286-
_log.error("getTreatmentWithConfig: you passed an empty string, matchingKey must be a non-empty string");
287-
return SPLIT_RESULT_CONTROL;
288-
}
289-
if (bucketingKey != null && bucketingKey.isEmpty()) {
290-
_log.error("getTreatmentWithConfig: you passed an empty string, bucketingKey must be a non-empty string");
291-
return SPLIT_RESULT_CONTROL;
292-
}
293-
if (bucketingKey != null && bucketingKey.length() > _config.maxStringLength()) {
294-
_log.error("getTreatmentWithConfig: bucketingKey too long - must be " + _config.maxStringLength() + " characters or less");
185+
if (!KeyValidator.isValid(matchingKey, "matchingKey", _config.maxStringLength(), method)) {
295186
return SPLIT_RESULT_CONTROL;
296187
}
297188

298-
if (split == null) {
299-
_log.error("getTreatmentWithConfig: you passed a null split name, split name must be a non-empty string");
189+
if (!KeyValidator.bucketingKeyIsValid(bucketingKey, _config.maxStringLength(), method)) {
300190
return SPLIT_RESULT_CONTROL;
301191
}
302192

303-
if (split.isEmpty()) {
304-
_log.error("getTreatmentWithConfig: you passed an empty split name, split name must be a non-empty string");
193+
SplitNameValidator.SplitNameResult splitNameResult = SplitNameValidator.isValid(split, method);
194+
if (!splitNameResult.getSuccess()) {
305195
return SPLIT_RESULT_CONTROL;
306196
}
307-
308-
String trimmed = split.trim();
309-
if (!trimmed.equals(split)) {
310-
_log.warn("getTreatmentWithConfig: split name \"" + split + "\" has extra whitespace, trimming");
311-
split = trimmed;
312-
}
197+
split = splitNameResult.getValue();
313198

314199
long start = System.currentTimeMillis();
315200

client/src/main/java/io/split/client/SplitFactoryBuilder.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package io.split.client;
22

3+
import io.split.engine.inputValidation.ApiKeyValidator;
34
import io.split.grammar.Treatments;
45
import org.slf4j.Logger;
56
import org.slf4j.LoggerFactory;
@@ -36,12 +37,7 @@ public static SplitFactory build(String apiToken) throws IOException, URISyntaxE
3637
* there were problems reading the override file from disk.
3738
*/
3839
public static synchronized SplitFactory build(String apiToken, SplitClientConfig config) throws IOException, URISyntaxException {
39-
if (apiToken == null) {
40-
_log.error("factory instantiation: you passed a null apiToken, apiToken must be a non-empty string");
41-
}
42-
if (apiToken.isEmpty()) {
43-
_log.error("factory instantiation: you passed and empty apiToken, apiToken be a non-empty string");
44-
}
40+
ApiKeyValidator.validate(apiToken);
4541

4642
if (LocalhostSplitFactory.LOCALHOST.equals(apiToken)) {
4743
return LocalhostSplitFactory.createLocalhostSplitFactory(config);

0 commit comments

Comments
 (0)