Remove flag concatenation (#297)

- Add support for addFlag('foo') directly, without a flagQualifier instance
- Remove support for flag concatenation addFlag('foo').addFlag('bar') -> fl_foo,fl_bar (instead of fl_foo.bar)

Author: patrick-tolosa

__TESTS__/unit/Action/Action.test.ts

@@ -78,7 +78,7 @@ describe('Tests for Transformation Action', () => {
       .addAction(action)
       .toURL();
 
-    expect(url).toBe('https://res.cloudinary.com/demo/image/upload/fl_first_flag.second_flag,l_sample/sample');
+    expect(url).toBe('https://res.cloudinary.com/demo/image/upload/fl_first_flag,fl_second_flag,l_sample/sample');
   });
 
   it('Correctly sorts qualifiers', () => {
@@ -89,7 +89,7 @@ describe('Tests for Transformation Action', () => {
       .addFlag(new FlagQualifier('b'))
       .addQualifier(new Qualifier('c', '3'));
 
-    expect(action.toString()).toBe('a_1,b_2,c_3,fl_a.b');
+    expect(action.toString()).toBe('a_1,b_2,c_3,fl_a,fl_b');
   });
 
   it('Add and read actions tags', () => {

__TESTS__/unit/Action/Flag.test.ts

@@ -1,13 +1,15 @@
 import {FlagQualifier} from "../../../src/values/flag/FlagQualifier";
+import {Action} from "../../../src/internal/Action";
 
 describe('Tests for Flag', () => {
   it('Creates a Flag', () => {
     const flag = new FlagQualifier("single_flag");
     expect(flag.toString()).toBe('fl_single_flag');
   });
-  it('Creates a Flag with multiple values', () => {
-    const flag = new FlagQualifier(["first_flag", "second_flag"]);
 
-    expect(flag.toString()).toBe('fl_first_flag.second_flag');
+  it ('Creates an action with multiple flags', () => {
+    const str = new Action().addFlag('foo').addFlag('bar').toString();
+
+    expect(str).toEqual('fl_bar,fl_foo');
   });
 });

__TESTS__/unit/actions/Flag.test.ts

@@ -100,6 +100,6 @@ describe('Tests for Transformation Action -- Flag', () => {
       .setPublicID('sample')
       .toURL();
 
-    expect(url).toBe('https://res.cloudinary.com/demo/image/upload/ar_1.0,c_fill,fl_keep_iptc.keep_attribution,w_400/sample');
+    expect(url).toBe('https://res.cloudinary.com/demo/image/upload/ar_1.0,c_fill,fl_keep_attribution,fl_keep_iptc,w_400/sample');
   });
 });

__TESTS__/unit/actions/Transcode.test.ts

@@ -111,7 +111,7 @@ describe('Tests for Transformation Action -- Transcode', () => {
       .setPublicID('sample')
       .toURL();
 
-    expect(url).toBe('https://res.cloudinary.com/demo/video/upload/f_webp,fl_awebp.animated/sample');
+    expect(url).toBe('https://res.cloudinary.com/demo/video/upload/f_webp,fl_animated,fl_awebp/sample');
   });
 
   it('Creates a cloudinaryURL with toAnimated and delay', () => {

__TESTS__/unit/types/Position.test.ts

@@ -21,7 +21,7 @@ describe('Position Qualifier', () => {
       .offsetY(10)
       .toString();
 
-    expect(posString).toBe('fl_no_overflow.tiled,g_north,x_10,y_10');
+    expect(posString).toBe('fl_no_overflow,fl_tiled,g_north,x_10,y_10');
   });
 
   it('Tests the toString() method of Position (FocusOn Gravity)', () => {
@@ -33,6 +33,6 @@ describe('Position Qualifier', () => {
       .offsetY(10)
       .toString();
 
-    expect(posString).toBe('fl_no_overflow.tiled,g_cat,x_10,y_10');
+    expect(posString).toBe('fl_no_overflow,fl_tiled,g_cat,x_10,y_10');
   });
 });

__TESTS__/unit/utils/dataStructureUtils/sortMapByKey.test.ts

@@ -7,6 +7,6 @@ describe('Tests for sortMapByKey', () => {
     map.set('a', 'a');
     map.set('b', 'b');
 
-    expect(mapToSortedArray(map).join(',')).toBe('a,b,c');
+    expect(mapToSortedArray(map, []).join(',')).toBe('a,b,c');
   });
 });

src/internal/Action.ts

@@ -10,6 +10,11 @@ class Action {
   // We're using map, to overwrite existing keys. for example:
   // addParam(w_100).addQualifier(w_200) should result in w_200. and not w_100,w_200
   qualifiers: Map<string, Qualifier> = new Map();
+
+  // Unlike regular qualifiers, there can be multiple flags in each url component /fl_1,fl_2/
+  // If the falgs are added to the qualifiers map, only a single flag could exist in a component (it's a map)
+  // So flags are stored separately until the very end because of that reason
+  flags: FlagQualifier[] = [];
   private delimiter = ','; // {qualifier}{delimiter}{qualifier} for example: `${'w_100'}${','}${'c_fill'}`
   protected prepareQualifiers():void {}
   private actionTag = ''; // A custom name tag to identify this action in the future
@@ -37,16 +42,33 @@ class Action {
    */
   toString(): string {
     this.prepareQualifiers();
-    return mapToSortedArray(this.qualifiers).join(this.delimiter);
+    return mapToSortedArray(this.qualifiers, this.flags).join(this.delimiter);
   }
 
   /**
    * @description Adds the parameter to the action.
    * @param {SDK.Qualifier} qualifier
    * @return {this}
    */
-  addQualifier(qualifier: Qualifier): this {
-    this.qualifiers.set(qualifier.key, qualifier);
+  addQualifier(qualifier: Qualifier | string): this {
+    // if string, find the key and value
+    if (typeof qualifier === 'string') {
+      const [key, value] = qualifier.toLowerCase().split('_');
+
+
+      if (key === 'fl') {
+        // if string qualifier is a flag, store it in the flags arrays
+        this.flags.push(new FlagQualifier(value));
+      } else {
+        // if the string qualifier is not a flag, create a new qualifier from it
+        this.qualifiers.set(key, new Qualifier(key, value));
+      }
+
+    } else {
+      // if a qualifier object, insert to the qualifiers map
+      this.qualifiers.set(qualifier.key, qualifier);
+    }
+
     return this;
   }
 
@@ -55,14 +77,11 @@ class Action {
    * @param {Values.Flag} flag
    * @return {this}
    */
-  addFlag(flag: FlagQualifier): this {
-    const existingFlag = this.qualifiers.get('fl_');
-    flag.qualifierValue.setDelimiter('.');
-
-    if (existingFlag){
-      existingFlag.addValue(flag.qualifierValue);
+  addFlag(flag: FlagQualifier | string): this {
+    if (typeof flag === 'string') {
+      this.flags.push(new FlagQualifier(flag));
     } else {
-      this.qualifiers.set('fl_', flag);
+      this.flags.push(flag);
     }
 
     return this;

src/internal/qualifier/Qualifier.ts

@@ -14,7 +14,7 @@ class Qualifier {
       this.qualifierValue = qualifierValue;
     } else {
       this.qualifierValue = new QualifierValue();
-      this.qualifierValue.addValue(qualifierValue).setDelimiter('.');
+      this.qualifierValue.addValue(qualifierValue);
     }
   }
 

src/internal/utils/dataStructureUtils.ts

@@ -1,13 +1,23 @@
+import {FlagQualifier} from "../../values/flag/FlagQualifier";
+
 /**
  * Sort a map by key
  * @private
  * @param map <string, unknown>
  * @Return array of map's values sorted by key
  */
-function mapToSortedArray<T>(map: Map<string, T>): T[] {
-  const array = Array.from(map.entries()).sort();
+function mapToSortedArray<T>(map: Map<string, T | FlagQualifier>, flags: FlagQualifier[]): (T | FlagQualifier)[] {
+  const array = Array.from(map.entries());
+
+  // objects from the Array.from() method above are stored in array of arrays:
+  // [[qualifierKey, QualifierObj], [qualifierKey, QualifierObj]]
+  // Flags is an array of FlagQualifierObj
+  // We need to convert it to the same form: [flagQualifier] =>  ['fl', flagQualifier]
+  flags.forEach((flag) => {
+    array.push(['fl', flag]); // push ['fl', flagQualifier]
+  });
 
-  return array.map((v) => v[1]);
+  return array.sort().map((v) => v[1]);
 }
 
 /**