Merge pull request #178 from splitio/input-validation-refactor

Input validation refactor

Author: sanzmauro

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

@@ -9,16 +9,22 @@
 import io.split.engine.evaluator.Evaluator;
 import io.split.engine.SDKReadinessGates;
 import io.split.engine.evaluator.EvaluatorImp;
+import io.split.engine.evaluator.Labels;
+
 import io.split.engine.metrics.Metrics;
 import io.split.grammar.Treatments;
+import io.split.inputValidation.EventsValidator;
+import io.split.inputValidation.KeyValidator;
+import io.split.inputValidation.SplitNameValidator;
+import io.split.inputValidation.TrafficTypeValidator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.TimeoutException;
-import java.util.regex.Pattern;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
@@ -28,11 +34,10 @@
  * @author adil
  */
 public final class SplitClientImpl implements SplitClient {
-    public static final Pattern EVENT_TYPE_MATCHER = Pattern.compile("^[a-zA-Z0-9][-_.:a-zA-Z0-9]{0,79}$");
     public static final SplitResult SPLIT_RESULT_CONTROL = new SplitResult(Treatments.CONTROL, null);
 
-    private static final String GET_TREATMENT_LABEL = "sdk.getTreatment";
-    private static final String DEFINITION_NOT_FOUND = "definition not found";
+    private static final String GET_TREATMENT = "getTreatment";
+    private static final String GET_TREATMENT_WITH_CONFIG = "getTreatmentWithConfig";
 
     private static final Logger _log = LoggerFactory.getLogger(SplitClientImpl.class);
 
@@ -70,59 +75,27 @@ public String getTreatment(String key, String split) {
 
     @Override
     public String getTreatment(String key, String split, Map<String, Object> attributes) {
-        return getTreatmentWithConfigInternal(GET_TREATMENT_LABEL, key, null, split, attributes).treatment();
+        return getTreatmentWithConfigInternal(GET_TREATMENT, key, null, split, attributes).treatment();
     }
 
     @Override
     public String getTreatment(Key key, String split, Map<String, Object> attributes) {
-        if (key == null) {
-            _log.error("getTreatment: you passed a null key, the key must be a non-empty string");
-            return Treatments.CONTROL;
-        }
-
-        if (key.matchingKey() == null) {
-            _log.error("getTreatment: you passed a null matchingKey, the matchingKey must be a non-empty string");
-            return Treatments.CONTROL;
-        }
-
-
-        if (key.bucketingKey() == null) {
-            _log.error("getTreatment: you passed a null bucketingKey, the bucketingKey must be a non-empty string");
-            return Treatments.CONTROL;
-        }
-
-        return getTreatmentWithConfigInternal(GET_TREATMENT_LABEL, key.matchingKey(), key.bucketingKey(), split, attributes).treatment();
+        return getTreatmentWithConfigInternal(GET_TREATMENT, key.matchingKey(), key.bucketingKey(), split, attributes).treatment();
     }
 
     @Override
     public SplitResult getTreatmentWithConfig(String key, String split) {
-        return getTreatmentWithConfigInternal(GET_TREATMENT_LABEL, key, null, split, Collections.<String, Object>emptyMap());
+        return getTreatmentWithConfigInternal(GET_TREATMENT_WITH_CONFIG, key, null, split, Collections.<String, Object>emptyMap());
     }
 
     @Override
     public SplitResult getTreatmentWithConfig(String key, String split, Map<String, Object> attributes) {
-        return getTreatmentWithConfigInternal(GET_TREATMENT_LABEL, key, null, split, attributes);
+        return getTreatmentWithConfigInternal(GET_TREATMENT_WITH_CONFIG, key, null, split, attributes);
     }
 
     @Override
     public SplitResult getTreatmentWithConfig(Key key, String split, Map<String, Object> attributes) {
-        if (key == null) {
-            _log.error("getTreatment: you passed a null key, the key must be a non-empty string");
-            return SPLIT_RESULT_CONTROL;
-        }
-
-        if (key.matchingKey() == null) {
-            _log.error("getTreatment: you passed a null matchingKey, the matchingKey must be a non-empty string");
-            return SPLIT_RESULT_CONTROL;
-        }
-
-
-        if (key.bucketingKey() == null) {
-            _log.error("getTreatment: you passed a null bucketingKey, the bucketingKey must be a non-empty string");
-            return SPLIT_RESULT_CONTROL;
-        }
-
-        return getTreatmentWithConfigInternal(GET_TREATMENT_LABEL, key.matchingKey(), key.bucketingKey(), split, attributes);
+        return getTreatmentWithConfigInternal(GET_TREATMENT_WITH_CONFIG, key.matchingKey(), key.bucketingKey(), split, attributes);
     }
 
     @Override
@@ -178,144 +151,59 @@ private boolean track(Event event) {
         }
 
         // Traffic Type validations
-        if (event.trafficTypeName == null) {
-            _log.error("track: you passed a null trafficTypeName, trafficTypeName must be a non-empty string");
-            return false;
-        }
-
-        if (event.trafficTypeName.isEmpty()) {
-            _log.error("track: you passed an empty trafficTypeName, trafficTypeName must be a non-empty string");
+        Optional<String> trafficTypeResult = TrafficTypeValidator.isValid(event.trafficTypeName, _splitCache, "track");
+        if (!trafficTypeResult.isPresent()) {
             return false;
         }
-
-        if (!event.trafficTypeName.equals(event.trafficTypeName.toLowerCase())) {
-            _log.warn("track: trafficTypeName should be all lowercase - converting string to lowercase");
-            event.trafficTypeName = event.trafficTypeName.toLowerCase();
-        }
-
-        if (!_splitCache.trafficTypeExists(event.trafficTypeName)) {
-            _log.warn("track: Traffic Type " + event.trafficTypeName + " does not have any corresponding Splits in this environment, " +
-                    "make sure you’re tracking your events to a valid traffic type defined in the Split console.");
-        }
+        event.trafficTypeName = trafficTypeResult.get();
 
         // EventType validations
-        if (event.eventTypeId == null) {
-            _log.error("track: you passed a null eventTypeId, eventTypeId must be a non-empty string");
-            return false;
-        }
-
-        if (event.eventTypeId.isEmpty()) {
-            _log.error("track:you passed an empty eventTypeId, eventTypeId must be a non-empty string");
-            return false;
-        }
-
-        if (!EVENT_TYPE_MATCHER.matcher(event.eventTypeId).find()) {
-            _log.error("track: you passed " + event.eventTypeId + ", eventTypeId must adhere to the regular expression " +
-                    "[a-zA-Z0-9][-_.:a-zA-Z0-9]{0,79}. This means an eventTypeID must be alphanumeric, " +
-                    "cannot be more than 80 characters long, and can only include a dash, underscore, period, " +
-                    "or colon as separators of alphanumeric characters");
+        if (!EventsValidator.typeIsValid(event.eventTypeId, "track")) {
             return false;
         }
 
         // Key Validations
-        if (event.key == null) {
-            _log.error("track: you passed a null key, key must be a non-empty string");
+        if (!KeyValidator.isValid(event.key, "key", _config.maxStringLength(), "track")) {
             return false;
         }
 
-        if (event.key.isEmpty()) {
-            _log.error("track: you passed an empty key, key must be a non-empty string");
+        // Properties validations
+        EventsValidator.EventValidatorResult propertiesResult = EventsValidator.propertiesAreValid(event.properties);
+        if (!propertiesResult.getSuccess()) {
             return false;
         }
 
-        if (event.key.length() > _config.maxStringLength()) {
-            _log.error("track: key too long - must be " + _config.maxStringLength() + "characters or less");
-            return false;
-        }
-
-        int size = 1024; // We assume 1kb events without properties (750 bytes avg measured)
-        if (null != event.properties) {
-            if (event.properties.size() > 300) {
-                _log.warn("Event has more than 300 properties. Some of them will be trimmed when processed");
-            }
-
-            for (Map.Entry<String, Object> entry: event.properties.entrySet()) {
-                size += entry.getKey().length();
-                Object value = entry.getValue();
-                if (null == value) {
-                    continue;
-                }
-
-                if (!(value instanceof Number) && !(value instanceof Boolean) && !(value instanceof String)) {
-                    _log.warn(String.format("Property %s is of invalid type. Setting value to null", entry.getKey()));
-                    entry.setValue(null);
-                }
-
-                if (value instanceof String) {
-                    size += ((String) value).length();
-                }
-
-                if (size > Event.MAX_PROPERTIES_LENGTH_BYTES) {
-                    _log.error(String.format("The maximum size allowed for the properties is 32768 bytes. "
-                        + "Current one is %s bytes. Event not queued", size));
-                    return false;
-                }
-            }
-
-        }
+        event.properties = propertiesResult.getValue();
 
-        return _eventClient.track(event, size);
+        return _eventClient.track(event, propertiesResult.getEventSize());
     }
 
-    private SplitResult getTreatmentWithConfigInternal(String label, String matchingKey, String bucketingKey, String split, Map<String, Object> attributes) {
+    private SplitResult getTreatmentWithConfigInternal(String method, String matchingKey, String bucketingKey, String split, Map<String, Object> attributes) {
         try {
             if (_container.isDestroyed()) {
                 _log.error("Client has already been destroyed - no calls possible");
                 return SPLIT_RESULT_CONTROL;
             }
 
-            if (matchingKey == null) {
-                _log.error("getTreatmentWithConfig: you passed a null matchingKey, the matchingKey must be a non-empty string");
-                return SPLIT_RESULT_CONTROL;
-            }
-            if (matchingKey.length() > _config.maxStringLength()) {
-                _log.error("getTreatmentWithConfig: matchingKey too long - must be " + _config.maxStringLength() + " characters or less");
-                return SPLIT_RESULT_CONTROL;
-            }
-            if (matchingKey.isEmpty()) {
-                _log.error("getTreatmentWithConfig: you passed an empty string, matchingKey must be a non-empty string");
-                return SPLIT_RESULT_CONTROL;
-            }
-            if (bucketingKey != null && bucketingKey.isEmpty()) {
-                _log.error("getTreatmentWithConfig: you passed an empty string, bucketingKey must be a non-empty string");
-                return SPLIT_RESULT_CONTROL;
-            }
-            if (bucketingKey != null && bucketingKey.length() > _config.maxStringLength()) {
-                _log.error("getTreatmentWithConfig: bucketingKey too long - must be " + _config.maxStringLength() + " characters or less");
+            if (!KeyValidator.isValid(matchingKey, "matchingKey", _config.maxStringLength(), method)) {
                 return SPLIT_RESULT_CONTROL;
             }
 
-            if (split == null) {
-                _log.error("getTreatmentWithConfig: you passed a null split name, split name must be a non-empty string");
+            if (!KeyValidator.bucketingKeyIsValid(bucketingKey, _config.maxStringLength(), method)) {
                 return SPLIT_RESULT_CONTROL;
             }
 
-            if (split.isEmpty()) {
-                _log.error("getTreatmentWithConfig: you passed an empty split name, split name must be a non-empty string");
+            Optional<String> splitNameResult = SplitNameValidator.isValid(split, method);
+            if (!splitNameResult.isPresent()) {
                 return SPLIT_RESULT_CONTROL;
             }
-
-            String trimmed = split.trim();
-            if (!trimmed.equals(split)) {
-                _log.warn("getTreatmentWithConfig: split name \"" + split + "\" has extra whitespace, trimming");
-                split = trimmed;
-            }
+            split = splitNameResult.get();
 
             long start = System.currentTimeMillis();
 
             EvaluatorImp.TreatmentLabelAndChangeNumber result = _evaluator.evaluateFeature(matchingKey, bucketingKey, split, attributes);
 
-            if (result.treatment.equals(Treatments.CONTROL) && result.label.equals(DEFINITION_NOT_FOUND) && _gates.isSDKReadyNow()) {
+            if (result.treatment.equals(Treatments.CONTROL) && result.label.equals(Labels.DEFINITION_NOT_FOUND) && _gates.isSDKReadyNow()) {
                 _log.warn(
                         "getTreatment: you passed \"" + split + "\" that does not exist in this environment, " +
                                 "please double check what Splits exist in the web console.");
@@ -327,7 +215,7 @@ private SplitResult getTreatmentWithConfigInternal(String label, String matching
                     split,
                     start,
                     result.treatment,
-                    label,
+                    String.format("sdk.%s", method),
                     _config.labelsEnabled() ? result.label : null,
                     result.changeNumber,
                     attributes

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

@@ -1,5 +1,6 @@
 package io.split.client;
 
+import io.split.inputValidation.ApiKeyValidator;
 import io.split.grammar.Treatments;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -36,12 +37,7 @@ public static SplitFactory build(String apiToken) throws IOException, URISyntaxE
      *                                               there were problems reading the override file from disk.
      */
     public static synchronized SplitFactory build(String apiToken, SplitClientConfig config) throws IOException, URISyntaxException {
-        if (apiToken == null) {
-            _log.error("factory instantiation: you passed a null apiToken, apiToken must be a non-empty string");
-        }
-        if (apiToken.isEmpty()) {
-            _log.error("factory instantiation: you passed and empty apiToken, apiToken be a non-empty string");
-        }
+        ApiKeyValidator.validate(apiToken);
 
         if (LocalhostSplitFactory.LOCALHOST.equals(apiToken)) {
             return LocalhostSplitFactory.createLocalhostSplitFactory(config);