From 622463b41219c9ca9969736d678faf85888d653c Mon Sep 17 00:00:00 2001 From: Patrick Date: Thu, 7 Jan 2021 19:29:05 +0200 Subject: [PATCH] Remove flag concatenation - 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) --- __TESTS__/unit/Action/Action.test.ts | 4 +- __TESTS__/unit/Action/Flag.test.ts | 8 ++-- __TESTS__/unit/actions/Flag.test.ts | 2 +- __TESTS__/unit/actions/Transcode.test.ts | 2 +- __TESTS__/unit/types/Position.test.ts | 4 +- .../dataStructureUtils/sortMapByKey.test.ts | 2 +- src/internal/Action.ts | 39 ++++++++++++++----- src/internal/qualifier/Qualifier.ts | 2 +- src/internal/utils/dataStructureUtils.ts | 16 ++++++-- 9 files changed, 55 insertions(+), 24 deletions(-) diff --git a/__TESTS__/unit/Action/Action.test.ts b/__TESTS__/unit/Action/Action.test.ts index 31f73852..e920e804 100644 --- a/__TESTS__/unit/Action/Action.test.ts +++ b/__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', () => { diff --git a/__TESTS__/unit/Action/Flag.test.ts b/__TESTS__/unit/Action/Flag.test.ts index a3f16c68..ad3e1b14 100644 --- a/__TESTS__/unit/Action/Flag.test.ts +++ b/__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'); }); }); diff --git a/__TESTS__/unit/actions/Flag.test.ts b/__TESTS__/unit/actions/Flag.test.ts index c9495122..f7c363fe 100644 --- a/__TESTS__/unit/actions/Flag.test.ts +++ b/__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'); }); }); diff --git a/__TESTS__/unit/actions/Transcode.test.ts b/__TESTS__/unit/actions/Transcode.test.ts index e65640fe..94e6124e 100644 --- a/__TESTS__/unit/actions/Transcode.test.ts +++ b/__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', () => { diff --git a/__TESTS__/unit/types/Position.test.ts b/__TESTS__/unit/types/Position.test.ts index 8f2e3e56..a4938e7f 100644 --- a/__TESTS__/unit/types/Position.test.ts +++ b/__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'); }); }); diff --git a/__TESTS__/unit/utils/dataStructureUtils/sortMapByKey.test.ts b/__TESTS__/unit/utils/dataStructureUtils/sortMapByKey.test.ts index d03c2c92..a6fcfeaf 100644 --- a/__TESTS__/unit/utils/dataStructureUtils/sortMapByKey.test.ts +++ b/__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'); }); }); diff --git a/src/internal/Action.ts b/src/internal/Action.ts index 4e362474..14e20e90 100644 --- a/src/internal/Action.ts +++ b/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 = 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,7 +42,7 @@ class Action { */ toString(): string { this.prepareQualifiers(); - return mapToSortedArray(this.qualifiers).join(this.delimiter); + return mapToSortedArray(this.qualifiers, this.flags).join(this.delimiter); } /** @@ -45,8 +50,25 @@ class 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; diff --git a/src/internal/qualifier/Qualifier.ts b/src/internal/qualifier/Qualifier.ts index 6094b7ac..51637c44 100644 --- a/src/internal/qualifier/Qualifier.ts +++ b/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); } } diff --git a/src/internal/utils/dataStructureUtils.ts b/src/internal/utils/dataStructureUtils.ts index 203cbeff..75d1b4ad 100644 --- a/src/internal/utils/dataStructureUtils.ts +++ b/src/internal/utils/dataStructureUtils.ts @@ -1,13 +1,23 @@ +import {FlagQualifier} from "../../values/flag/FlagQualifier"; + /** * Sort a map by key * @private * @param map * @Return array of map's values sorted by key */ -function mapToSortedArray(map: Map): T[] { - const array = Array.from(map.entries()).sort(); +function mapToSortedArray(map: Map, 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]); } /**