Fix old legacy normalization issues that were migrated to url-gen

.eslintrc

@@ -31,7 +31,7 @@
     "prefer-const": "error",
     "vars-on-top" : "error",
     "no-cond-assign": "error",
-    "comma-dangle" : "error",
+    "@typescript-eslint/ban-ts-comment" : "warn",
     "comma-spacing" : "error",
     "no-multi-spaces" : "error",
     "prefer-template" : "error",

__TESTS__/backwardsComaptibility/transformationLegacyTests/expression.test.ts → __TESTS__/backwardsComaptibility/transformationLegacyTests/legacyExpression.test.ts

@@ -1,7 +1,7 @@
 import {stringOrNumber} from "../../../src/types/types";
 import {legacyNormalizeExpression} from "../../../src/backwards/utils/legacyNormalizeExpression";
 
-describe("Expression normalization", function () {
+describe("Legacy Expression normalization", function () {
   const cases: Record<string, [stringOrNumber, string]> = {
     'null is not affected': [null, null],
     'number replaced with a string value': [10, '10'],

__TESTS__/unit/actions/NamedTransformation.test.ts

@@ -11,13 +11,13 @@ describe('Tests for Transformation Action -- NamedTransformation', () => {
     ).toEqual(tImage);
   });
 
-  it('Creates a cloudinaryURL with name', () => {
+  it('Creates a cloudinaryURL with name that has an underscore', () => {
     const url = createNewImage('sample')
-      .namedTransformation(name('foobar'))
+      .namedTransformation(name('_foobar'))
       .setPublicID('sample')
       .toURL();
 
-    expect(url).toBe('https://res.cloudinary.com/demo/image/upload/t_foobar/sample');
+    expect(url).toBe('https://res.cloudinary.com/demo/image/upload/t__foobar/sample');
   });
 
   it('Creates a cloudinaryURL with name and resize', () => {

__TESTS__/unit/actions/Variable.test.ts

@@ -5,7 +5,6 @@ import {Expression} from "../../../src/qualifiers/expression";
 const {set} = Variable;
 const {expression} = Expression;
 
-
 describe('Tests for Transformation Action -- Variable', () => {
   it('tests common variable values', () => {
     expect(set('a', 30).toString()).toBe('$a_30');

__TESTS__/unit/expression.test.ts

@@ -0,0 +1,47 @@
+import {expression} from "../../src/qualifiers/expression";
+
+const cases: Record<string, [string, string]> = {
+  'empty string is not affected': ['', ''],
+  'normalize greater than': ['$foo > $bar', '$foo_gt_$bar'],
+  'custom tags': ['if_!my_custom_tag! in tags', 'if_!my_custom_tag!_in_tags'],
+  'single space is replaced with a single underscore': [' ', '_'],
+  'blank string is replaced with a single underscore': ['   ', '___'],
+  'underscore is not affected': ['_', '_'],
+  'sequence of underscores and spaces is changed to just underscores': [' _ __  _', '________'],
+  'arbitrary text is not affected': ['foobar', 'foobar'],
+  'duration is recognized as a variable and replaced with du': ['duration', 'du'],
+  'double ampersand replaced with and operator': ['foo && bar', 'foo_and_bar'],
+  'double ampersand with no space at the end is not affected': ['foo&&bar', 'foo&&bar'],
+  'width recognized as variable and replaced with w': ['width', 'w'],
+  'initial aspect ratio recognized as variable and replaced with iar': ['initial_aspect_ratio', 'iar'],
+  '$width recognized as user variable and not affected': ['$width', '$width'],
+  '$initial_aspect_ratio recognized as user variable followed by aspect_ratio variable': [
+    '$initial_aspect_ratio',
+    '$initial_ar'
+  ],
+  '$mywidth recognized as user variable and not affected': ['$mywidth', '$mywidth'],
+  '$widthwidth recognized as user variable and not affected': ['$widthwidth', '$widthwidth'],
+  '$_width recognized as user variable and not affected': ['$_width', '$_width'],
+  '$__width recognized as user variable and not affected': ['$__width', '$__width'],
+  '$$width recognized as user variable and not affected': ['$$width', '$$width'],
+  '$height recognized as user variable and not affected': ['$height_100', '$height_100'],
+  '$heightt_100 recognized as user variable and not affected': ['$heightt_100', '$heightt_100'],
+  '$$height_100 recognized as user variable and not affected': ['$$height_100', '$$height_100'],
+  '$heightmy_100 recognized as user variable and not affected': ['$heightmy_100', '$heightmy_100'],
+  '$myheight_100 recognized as user variable and not affected': ['$myheight_100', '$myheight_100'],
+  '$heightheight_100 recognized as user variable and not affected': [
+    '$heightheight_100',
+    '$heightheight_100'
+  ],
+  '$theheight_100 recognized as user variable and not affected': ['$theheight_100', '$theheight_100'],
+  '$__height_100 recognized as user variable and not affected': ['$__height_100', '$__height_100']
+};
+
+describe('Tests for Transformation Action -- Variable', () => {
+  it('tests expressions values', () => {
+    Object.keys(cases).forEach(function (testDescription) {
+      const [input, expected] = cases[testDescription];
+      expect(expression(input).toString()).toBe(expected);
+    });
+  });
+});

src/internal/internalConstants.ts

@@ -47,17 +47,35 @@ export const CONDITIONAL_OPERATORS = {
   "/": "div",
   "+": "add",
   "-": "sub",
-  "^": "pow",
-  "initial_width": "iw",
-  "initial_height": "ih",
-  "width": "w",
-  "height": "h",
+  "^": "pow"
+};
+
+export const RESERVED_NAMES = {
   "aspect_ratio": "ar",
-  "initial_aspect_ratio": "iar",
-  "trimmed_aspect_ratio": "tar",
+  "aspectRatio": "ar",
   "current_page": "cp",
+  "currentPage": "cp",
+  "duration": "du",
   "face_count": "fc",
+  "faceCount": "fc",
+  "height": "h",
+  "initial_aspect_ratio": "iar",
+  "initial_height": "ih",
+  "initial_width": "iw",
+  "initialAspectRatio": "iar",
+  "initialHeight": "ih",
+  "initialWidth": "iw",
+  "initial_duration": "idu",
+  "initialDuration": "idu",
   "page_count": "pc",
+  "page_x": "px",
+  "page_y": "py",
+  "pageCount": "pc",
+  "pageX": "px",
+  "pageY": "py",
+  "tags": "tags",
+  "width": "w",
+  "trimmed_aspect_ratio": "tar",
   "current_public_id": "cpi",
   "initial_density": "idn",
   "page_names": "pgnames"

src/qualifiers/expression.ts

@@ -1,4 +1,4 @@
-import {CONDITIONAL_OPERATORS} from "../internal/internalConstants.js";
+import {CONDITIONAL_OPERATORS, RESERVED_NAMES} from "../internal/internalConstants.js";
 import {ExpressionQualifier} from "./expression/ExpressionQualifier.js";
 
 
@@ -17,11 +17,53 @@ import {ExpressionQualifier} from "./expression/ExpressionQualifier.js";
  * @return {Qualifiers.Expression.ExpressionQualifier}
  */
 function expression(exp: string): ExpressionQualifier {
-  return new ExpressionQualifier(exp
-    .toString()
-    .split(" ")
-    .map((val: keyof typeof CONDITIONAL_OPERATORS) => CONDITIONAL_OPERATORS[val] || val)
-    .join("_"));
+  // Prepare the CONDITIONAL_OPERATORS object to be used in a regex
+  // Properly escape |, +, ^ and *
+  // This step also adds a regex space ( \s ) around each operator, since these are only replaced when wrapped with spaces
+  // $foo * $bar is replaced to $foo_mul_$bar
+  // $foo*bar is treated AS-IS.
+  const reservedOperatorList = Object.keys(CONDITIONAL_OPERATORS).map((key) => {
+    return `\\s${key.replace(/(\*|\+|\^|\|)/g, '\\$1')}\\s`;
+  });
+
+  // reservedOperatorList is now an array of values, joining with | creates the regex list
+  const regexSafeOperatorList = reservedOperatorList.join('|');
+  const operatorsReplaceRE = new RegExp(`(${regexSafeOperatorList})`, "g");
+
+  // First, we replace all the operators
+  // Notice how we pad the matched operators with `_`, this is following the step above.
+  // This turns $foo * $bar into $foo_mul_$bar (notice how the spaces were replaced with an underscore
+  const stringWithOperators = exp.toString()
+    .replace(operatorsReplaceRE, (match: string) => {
+      // match contains spaces around the expression, we need to trim it as the original list
+      // does not contain spaces.
+      return `_${CONDITIONAL_OPERATORS[match.trim() as keyof typeof CONDITIONAL_OPERATORS]}_`;
+    });
+
+
+  // Handle reserved names (width, height, etc.)
+  const ReservedNames = Object.keys(RESERVED_NAMES);
+  const regexSafeReservedNameList = ReservedNames.join('|');
+  // Gather all statements that begin with a dollar sign, underscore or a space
+  // Gather all RESERVED NAMES
+  // $foo_bar is matched
+  // height is matched
+  const reservedNamesRE = new RegExp(`(\\$_*[^_ ]+)|${regexSafeReservedNameList}`, "g");
+
+  // Since this regex captures both user variables and our reserved keywords, we need to add some logic in the replacer
+  const stringWithVariables = stringWithOperators.replace(reservedNamesRE, (match) => {
+    // Do not do anything to user variables (anything starting with $)
+    if (match.startsWith('$')) {
+      return match;
+    } else {
+      return RESERVED_NAMES[match as keyof typeof RESERVED_NAMES] || match;
+    }
+  });
+
+  // Serialize remaining spaces with an underscore
+  const finalExpressionString = stringWithVariables.replace(/\s/g, '_');
+
+  return new ExpressionQualifier(finalExpressionString);
 }
 
 // as a namespace