Fix variables.

* Fix variable regex.
* Make Expression.serialize return normalized expression
* Fix variable sorting.
* Add tests for variable order.

Author: Amir Tocker

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

@@ -12,6 +12,7 @@
 
 @SuppressWarnings({"rawtypes", "unchecked"})
 public class Transformation {
+    public static final String VAR_NAME_RE = "^\\$[a-zA-Z][a-zA-Z0-9]+$";
     protected Map transformation;
     protected List<Map> transformations;
     protected String htmlWidth;
@@ -601,21 +602,21 @@ public String generate(Map options) {
             components.add(0, "if_" + Expression.normalize(ifValue));
         }
 
-        List<String> varParams = new ArrayList<String>();
+        SortedSet<String> varParams = new TreeSet<String>();
         for( Object k: options.keySet()) {
             String key = (String) k;
-            if(key.matches("^\\$[a-zA-Z0-9]+$")) {
+            if(key.matches(VAR_NAME_RE)) {
                 varParams.add(key + "_" + ObjectUtils.asString(options.get(k)));
             }
         }
 
-        String variables = processVar((Expression[]) options.get("variables"));
-        if (variables != null) {
-            varParams.add(variables);
+        if (!varParams.isEmpty()) {
+            components.add(StringUtils.join(varParams, ","));
         }
 
-        if (varParams != null && !varParams.isEmpty()) {
-            components.add(StringUtils.join(varParams, ","));
+        String variables = processVar((Expression[]) options.get("variables"));
+        if (variables != null) {
+            components.add(variables);
         }
 
         for (Map.Entry<String, String> param : params.entrySet()) {

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

@@ -113,7 +113,7 @@ public T setParent(Transformation parent) {
     }
 
     public String serialize() {
-        return StringUtils.join(expressions, "_");
+        return normalize(StringUtils.join(expressions, "_"));
     }
 
     @Override

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

@@ -142,7 +142,7 @@ public void endIf2() throws Exception {
         assertEquals("force the if_else clause to be chained", "if_w_gt_100_and_w_lt_200/c_scale,w_50/if_else/c_crop,w_100/if_end", transformation.toString());
 
     }
-    
+
     @Test
     public void testArrayShouldDefineASetOfVariables() {
         // using methods
@@ -153,7 +153,22 @@ public void testArrayShouldDefineASetOfVariables() {
                 .width("$foo * 200");
         assertEquals("if_fc_gt_2,$z_5,$foo_$z_mul_2,c_scale,w_$foo_mul_200", t.generate());
     }
-    
+
+    @Test
+    public void testShouldSortDefinedVariable(){
+        Transformation t = new Transformation().variable("$second", 1).variable("$first", 2);
+        assertEquals("$first_2,$second_1", t.generate());
+    }
+
+    @Test
+    public void testShouldPlaceDefinedVariablesBeforeOrdered(){
+        Transformation t = new Transformation()
+                .variables(variable("$z", 5), variable("$foo", "$z * 2"))
+                .variable("$second", 1)
+                .variable("$first", 2);
+        assertEquals("$first_2,$second_1,$z_5,$foo_$z_mul_2", t.generate());
+    }
+
     @Test
     public void testVariable(){
         // using strings
@@ -166,7 +181,7 @@ public void testVariable(){
                 .endIf();
         assertEquals("$foo_10/if_fc_gt_2/c_scale,w_$foo_mul_200_div_fc/if_end", t.generate());
     }
-    
+
     @Test
     public void testShouldSupportTextValues() {
         Transformation t = new Transformation();