Rename conditional parameters faces and pages to faceCount and pageCount

Author: Amir Tocker

cloudinary-core/src/main/java/com/cloudinary/Transformation.java

@@ -20,7 +20,7 @@ public class Transformation {
     protected static boolean defaultIsResponsive = false;
     protected static Object defaultDPR = null;
 
-    private static final Map DEFAULT_RESPONSIVE_WIDTH_TRANSFORMATION = ObjectUtils.asMap("width", "auto", "crop", "limit");
+    private static final Map<String, String> DEFAULT_RESPONSIVE_WIDTH_TRANSFORMATION = ObjectUtils.asMap("width", "auto", "crop", "limit");
     protected static Map responsiveWidthTransformation = null;
     private static final Pattern RANGE_VALUE_RE = Pattern.compile("^((?:\\d+\\.)?\\d+)([%pP])?$");
     private static final Pattern RANGE_RE = Pattern.compile("^(\\d+\\.)?\\d+[%pP]?\\.\\.(\\d+\\.)?\\d+[%pP]?$");
@@ -348,19 +348,29 @@ public Transformation responsiveWidth(boolean value) {
         return param("responsive_width", value);
     }
 
+    /**
+     * Start defining a condition, which will be completed with a call {@link Condition#then()}
+     * @return condition
+     */
     public Condition ifCondition() {
         return new Condition().setParent(this);
     }
+
+    /**
+     * Define a conditional transformation defined by the condition string
+     * @param condition a condition string
+     * @return the transformation for chaining
+     */
     public Transformation ifCondition(String condition) {
         return param("if", condition);
     }
+
     public Transformation ifElse() {
         chain();
         return param("if", "else");
     }
 
     public Transformation endIf() {
-
         chain();
         int transSize = this.transformations.size();
         for (int i = transSize - 1; i >= 0; i--) {

cloudinary-core/src/main/java/com/cloudinary/transformation/Condition.java

@@ -7,9 +7,11 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /**
- *
+ * Represents a condition for {@link Transformation#ifCondition()}
  */
 public class Condition {
     public static final Map OPERATORS = ObjectUtils.asMap(
@@ -22,35 +24,61 @@ public class Condition {
             "&&", "and",
             "||", "or");
 
+    public static final Map PARAMETERS = ObjectUtils.asMap(
+            "width", "w",
+            "height", "h",
+            "aspect_ratio", "ar",
+            "aspectRatio", "ar",
+            "page_count", "pc",
+            "pageCount", "pc",
+            "face_count", "fc",
+            "faceCount", "fc"
+    );
+
     protected List<String> predicateList = null;
     private Transformation parent = null;
 
-    public Condition( ) {
+    public Condition() {
         predicateList = new ArrayList<String>();
 
     }
-    public Condition( String conditionStr) {
+
+    /**
+     * Create a Condition Object. The conditionStr string will be translated to a serialized condition.
+     *
+     * @param conditionStr condition in string format
+     */
+    public Condition(String conditionStr) {
         this();
         if (conditionStr != null) {
-            predicateList.add( literal(conditionStr));
+            predicateList.add(literal(conditionStr));
         }
     }
 
     private String literal(String conditionStr) {
-        String[] list = conditionStr.split("[ _]+");
-        String[] translated = new String[list.length];
-        for (int i = 0, j = 0; i < list.length; i++) {
-            String s = list[i];
-            if (OPERATORS.containsKey(s)) {
-                translated[j++] = (String) OPERATORS.get(s);
+
+        String replacement;
+        conditionStr = conditionStr.replaceAll("[ _]+", "_");
+        Pattern replaceRE = Pattern.compile("(" + StringUtils.join(PARAMETERS.keySet(), "|") + "|[=<>&|!]+)");
+        Matcher matcher = replaceRE.matcher(conditionStr);
+        StringBuffer result = new StringBuffer(conditionStr.length());
+        while (matcher.find()) {
+            if (OPERATORS.containsKey(matcher.group())) {
+                replacement = (String) OPERATORS.get(matcher.group());
+            } else if (PARAMETERS.containsKey(matcher.group())) {
+                replacement = (String) PARAMETERS.get(matcher.group());
             } else {
-                translated[j++] = s;
+                replacement = matcher.group();
             }
+            matcher.appendReplacement(result, replacement);
         }
-        return StringUtils.join(translated, "_");
+        matcher.appendTail(result);
+        return result.toString();
     }
+
     public Transformation getParent() { return parent;}
-    public Condition setParent( Transformation parent) {
+
+    public Condition setParent(Transformation parent) {
         this.parent = parent;
         return this;
     }
@@ -62,11 +90,11 @@ public String toString() {
         return serialize();
     }
 
-    protected Condition predicate( String name, String operator, String value) {
+    protected Condition predicate(String name, String operator, String value) {
         if (OPERATORS.containsKey(operator)) {
             operator = (String) OPERATORS.get(operator);
         }
-        predicateList.add(String.format("%s_%s_%s",name, operator, value));
+        predicateList.add(String.format("%s_%s_%s", name, operator, value));
         return this;
     }
 
@@ -80,33 +108,51 @@ public Condition or() {
         return this;
     }
 
+    /**
+     * Terminates the definition of the condition and continue with Transformation definition.
+     * @return the Transformation object this Condition is attached to.
+     */
     public Transformation then() {
-        getParent().ifCondition( serialize());
+        getParent().ifCondition(serialize());
         return getParent();
     }
 
     public Condition width(String operator, Object value) {
-        predicateList.add("w_"+ operator + "_" + value);
+        predicateList.add("w_" + operator + "_" + value);
         return this;
     }
 
     public Condition height(String operator, Object value) {
-        predicateList.add("h_"+ operator + "_" + value);
+        predicateList.add("h_" + operator + "_" + value);
         return this;
     }
 
     public Condition aspectRatio(String operator, Object value) {
-        predicateList.add("ar_"+ operator + "_" + value);
+        predicateList.add("ar_" + operator + "_" + value);
         return this;
     }
 
+    /**
+     * @deprecated Use {@link #faceCount(String, Object)} instead
+     */
     public Condition faces(String operator, Object value) {
-        predicateList.add("faces_"+ operator + "_" + value);
+        return faceCount(operator, value);
+    }
+
+    public Condition faceCount(String operator, Object value) {
+        predicateList.add("fc_" + operator + "_" + value);
         return this;
     }
 
+    /**
+     * @deprecated Use {@link #pageCount(String, Object)} instead
+     */
     public Condition pages(String operator, Object value) {
-        predicateList.add("pg_"+ operator + "_" + value);
+        return pageCount(operator, value);
+    }
+
+    public Condition pageCount(String operator, Object value) {
+        predicateList.add("pc_" + operator + "_" + value);
         return this;
     }
 }

cloudinary-core/src/test/java/com/cloudinary/TransformationTest.java

@@ -45,7 +45,7 @@ public void withLiteral() throws Exception {
 
     @Test
     public void literalWithSpaces() throws Exception {
-        Map map = ObjectUtils.asMap("if", "w < 200", "crop", "fill", "height", 120, "width", 80);
+        Map map = ObjectUtils.asMap("if", "width < 200", "crop", "fill", "height", 120, "width", 80);
         List<Map> list = new ArrayList<Map>();
         list.add(map);
         Transformation transformation = new Transformation(list);
@@ -98,34 +98,34 @@ public void chainedConditions() throws Exception {
         assertEquals("should translate operators", "if_ar_gt_3:4_and_w_lte_100_or_w_gt_200,c_scale,w_50", transformation.toString());
         transformation = new Transformation().ifCondition().aspectRatio(">", "3:4").and().width("<=", 100).or().width(">", 200).then().width(50).crop("scale");
         assertEquals("should translate operators", "if_ar_gt_3:4_and_w_lte_100_or_w_gt_200,c_scale,w_50", transformation.toString());
-        transformation = new Transformation().ifCondition().aspectRatio(">=", "3:4").and().pages(">=", 100).or().pages("!=", 0).then().width(50).crop("scale");
-        assertEquals("should translate operators", "if_ar_gte_3:4_and_pg_gte_100_or_pg_ne_0,c_scale,w_50", transformation.toString());
+        transformation = new Transformation().ifCondition().aspectRatio(">=", "3:4").and().pageCount(">=", 100).or().pageCount("!=", 0).then().width(50).crop("scale");
+        assertEquals("should translate operators", "if_ar_gte_3:4_and_pc_gte_100_or_pc_ne_0,c_scale,w_50", transformation.toString());
 
     }
 
     @Test
     public void shouldSupportAndTranslateOperators() {
 
         String allOperators =
-                "if_" +
-                        "w_eq_0_and" +
-                        "_w_ne_0_or" +
-                        "_w_lt_0_and" +
-                        "_w_gt_0_and" +
-                        "_w_lte_0_and" +
-                        "_w_gte_0" +
+                        "if_"           +
+                        "w_eq_0_and"    +
+                        "_h_ne_0_or"    +
+                        "_ar_lt_0_and"   +
+                        "_pc_gt_0_and"   +
+                        "_fc_lte_0_and"  +
+                        "_w_gte_0"      +
                         ",e_grayscale";
         assertEquals("should support and translate operators:  '=', '!=', '<', '>', '<=', '>=', '&&', '||'",
                 allOperators, new Transformation().ifCondition()
                         .width("=", 0).and()
-                        .width("!=", 0).or()
-                        .width("<", 0).and()
-                        .width(">", 0).and()
-                        .width("<=", 0).and()
+                        .height("!=", 0).or()
+                        .aspectRatio("<", 0).and()
+                        .pageCount(">", 0).and()
+                        .faceCount("<=", 0).and()
                         .width(">=", 0)
                         .then().effect("grayscale").toString());
 
-        assertEquals(allOperators, new Transformation().ifCondition("w = 0 && w != 0 || w < 0 and w > 0 and w <= 0 and w >= 0")
+        assertEquals(allOperators, new Transformation().ifCondition("w = 0 && height != 0 || aspectRatio < 0 and pageCount > 0 and faceCount <= 0 and width >= 0")
                 .effect("grayscale")
                 .toString());
     }