Skip to content

Commit d2f81e1

Browse files
patrick-tolosapatrick-tolosa
authored
Fix old legacy normalization issues that were migrated to url-gen (#435)
* Fix old legacy normalization issues that were migrated to url-gen
1 parent 47acb2b commit d2f81e1

File tree

7 files changed

+125
-19
lines changed

7 files changed

+125
-19
lines changed

‎.eslintrc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
"prefer-const": "error",
3232
"vars-on-top" : "error",
3333
"no-cond-assign": "error",
34-
"comma-dangle" : "error",
34+
"@typescript-eslint/ban-ts-comment" : "warn",
3535
"comma-spacing" : "error",
3636
"no-multi-spaces" : "error",
3737
"prefer-template" : "error",

‎__TESTS__/backwardsComaptibility/transformationLegacyTests/expression.test.ts renamed to ‎__TESTS__/backwardsComaptibility/transformationLegacyTests/legacyExpression.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import {stringOrNumber} from "../../../src/types/types";
22
import {legacyNormalizeExpression} from "../../../src/backwards/utils/legacyNormalizeExpression";
33

4-
describe("Expression normalization", function () {
4+
describe("Legacy Expression normalization", function () {
55
const cases: Record<string, [stringOrNumber, string]> = {
66
'null is not affected': [null, null],
77
'number replaced with a string value': [10, '10'],

‎__TESTS__/unit/actions/NamedTransformation.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@ describe('Tests for Transformation Action -- NamedTransformation', () => {
1111
).toEqual(tImage);
1212
});
1313

14-
it('Creates a cloudinaryURL with name', () => {
14+
it('Creates a cloudinaryURL with name that has an underscore', () => {
1515
const url = createNewImage('sample')
16-
.namedTransformation(name('foobar'))
16+
.namedTransformation(name('_foobar'))
1717
.setPublicID('sample')
1818
.toURL();
1919

20-
expect(url).toBe('https://res.cloudinary.com/demo/image/upload/t_foobar/sample');
20+
expect(url).toBe('https://res.cloudinary.com/demo/image/upload/t__foobar/sample');
2121
});
2222

2323
it('Creates a cloudinaryURL with name and resize', () => {

‎__TESTS__/unit/actions/Variable.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import {Expression} from "../../../src/qualifiers/expression";
55
const {set} = Variable;
66
const {expression} = Expression;
77

8-
98
describe('Tests for Transformation Action -- Variable', () => {
109
it('tests common variable values', () => {
1110
expect(set('a', 30).toString()).toBe('$a_30');

‎__TESTS__/unit/expression.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import {expression} from "../../src/qualifiers/expression";
2+
3+
const cases: Record<string, [string, string]> = {
4+
'empty string is not affected': ['', ''],
5+
'normalize greater than': ['$foo > $bar', '$foo_gt_$bar'],
6+
'custom tags': ['if_!my_custom_tag! in tags', 'if_!my_custom_tag!_in_tags'],
7+
'single space is replaced with a single underscore': [' ', '_'],
8+
'blank string is replaced with a single underscore': [' ', '___'],
9+
'underscore is not affected': ['_', '_'],
10+
'sequence of underscores and spaces is changed to just underscores': [' _ __ _', '________'],
11+
'arbitrary text is not affected': ['foobar', 'foobar'],
12+
'duration is recognized as a variable and replaced with du': ['duration', 'du'],
13+
'double ampersand replaced with and operator': ['foo && bar', 'foo_and_bar'],
14+
'double ampersand with no space at the end is not affected': ['foo&&bar', 'foo&&bar'],
15+
'width recognized as variable and replaced with w': ['width', 'w'],
16+
'initial aspect ratio recognized as variable and replaced with iar': ['initial_aspect_ratio', 'iar'],
17+
'$width recognized as user variable and not affected': ['$width', '$width'],
18+
'$initial_aspect_ratio recognized as user variable followed by aspect_ratio variable': [
19+
'$initial_aspect_ratio',
20+
'$initial_ar'
21+
],
22+
'$mywidth recognized as user variable and not affected': ['$mywidth', '$mywidth'],
23+
'$widthwidth recognized as user variable and not affected': ['$widthwidth', '$widthwidth'],
24+
'$_width recognized as user variable and not affected': ['$_width', '$_width'],
25+
'$__width recognized as user variable and not affected': ['$__width', '$__width'],
26+
'$$width recognized as user variable and not affected': ['$$width', '$$width'],
27+
'$height recognized as user variable and not affected': ['$height_100', '$height_100'],
28+
'$heightt_100 recognized as user variable and not affected': ['$heightt_100', '$heightt_100'],
29+
'$$height_100 recognized as user variable and not affected': ['$$height_100', '$$height_100'],
30+
'$heightmy_100 recognized as user variable and not affected': ['$heightmy_100', '$heightmy_100'],
31+
'$myheight_100 recognized as user variable and not affected': ['$myheight_100', '$myheight_100'],
32+
'$heightheight_100 recognized as user variable and not affected': [
33+
'$heightheight_100',
34+
'$heightheight_100'
35+
],
36+
'$theheight_100 recognized as user variable and not affected': ['$theheight_100', '$theheight_100'],
37+
'$__height_100 recognized as user variable and not affected': ['$__height_100', '$__height_100']
38+
};
39+
40+
describe('Tests for Transformation Action -- Variable', () => {
41+
it('tests expressions values', () => {
42+
Object.keys(cases).forEach(function (testDescription) {
43+
const [input, expected] = cases[testDescription];
44+
expect(expression(input).toString()).toBe(expected);
45+
});
46+
});
47+
});

‎src/internal/internalConstants.ts

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,35 @@ export const CONDITIONAL_OPERATORS = {
4747
"/": "div",
4848
"+": "add",
4949
"-": "sub",
50-
"^": "pow",
51-
"initial_width": "iw",
52-
"initial_height": "ih",
53-
"width": "w",
54-
"height": "h",
50+
"^": "pow"
51+
};
52+
53+
export const RESERVED_NAMES = {
5554
"aspect_ratio": "ar",
56-
"initial_aspect_ratio": "iar",
57-
"trimmed_aspect_ratio": "tar",
55+
"aspectRatio": "ar",
5856
"current_page": "cp",
57+
"currentPage": "cp",
58+
"duration": "du",
5959
"face_count": "fc",
60+
"faceCount": "fc",
61+
"height": "h",
62+
"initial_aspect_ratio": "iar",
63+
"initial_height": "ih",
64+
"initial_width": "iw",
65+
"initialAspectRatio": "iar",
66+
"initialHeight": "ih",
67+
"initialWidth": "iw",
68+
"initial_duration": "idu",
69+
"initialDuration": "idu",
6070
"page_count": "pc",
71+
"page_x": "px",
72+
"page_y": "py",
73+
"pageCount": "pc",
74+
"pageX": "px",
75+
"pageY": "py",
76+
"tags": "tags",
77+
"width": "w",
78+
"trimmed_aspect_ratio": "tar",
6179
"current_public_id": "cpi",
6280
"initial_density": "idn",
6381
"page_names": "pgnames"

‎src/qualifiers/expression.ts

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {CONDITIONAL_OPERATORS} from "../internal/internalConstants.js";
1+
import {CONDITIONAL_OPERATORS, RESERVED_NAMES} from "../internal/internalConstants.js";
22
import {ExpressionQualifier} from "./expression/ExpressionQualifier.js";
33

44

@@ -17,11 +17,53 @@ import {ExpressionQualifier} from "./expression/ExpressionQualifier.js";
1717
* @return {Qualifiers.Expression.ExpressionQualifier}
1818
*/
1919
function expression(exp: string): ExpressionQualifier {
20-
return new ExpressionQualifier(exp
21-
.toString()
22-
.split(" ")
23-
.map((val: keyof typeof CONDITIONAL_OPERATORS) => CONDITIONAL_OPERATORS[val] || val)
24-
.join("_"));
20+
// Prepare the CONDITIONAL_OPERATORS object to be used in a regex
21+
// Properly escape |, +, ^ and *
22+
// This step also adds a regex space ( \s ) around each operator, since these are only replaced when wrapped with spaces
23+
// $foo * $bar is replaced to $foo_mul_$bar
24+
// $foo*bar is treated AS-IS.
25+
const reservedOperatorList = Object.keys(CONDITIONAL_OPERATORS).map((key) => {
26+
return `\\s${key.replace(/(\*|\+|\^|\|)/g, '\\$1')}\\s`;
27+
});
28+
29+
// reservedOperatorList is now an array of values, joining with | creates the regex list
30+
const regexSafeOperatorList = reservedOperatorList.join('|');
31+
const operatorsReplaceRE = new RegExp(`(${regexSafeOperatorList})`, "g");
32+
33+
// First, we replace all the operators
34+
// Notice how we pad the matched operators with `_`, this is following the step above.
35+
// This turns $foo * $bar into $foo_mul_$bar (notice how the spaces were replaced with an underscore
36+
const stringWithOperators = exp.toString()
37+
.replace(operatorsReplaceRE, (match: string) => {
38+
// match contains spaces around the expression, we need to trim it as the original list
39+
// does not contain spaces.
40+
return `_${CONDITIONAL_OPERATORS[match.trim() as keyof typeof CONDITIONAL_OPERATORS]}_`;
41+
});
42+
43+
44+
// Handle reserved names (width, height, etc.)
45+
const ReservedNames = Object.keys(RESERVED_NAMES);
46+
const regexSafeReservedNameList = ReservedNames.join('|');
47+
// Gather all statements that begin with a dollar sign, underscore or a space
48+
// Gather all RESERVED NAMES
49+
// $foo_bar is matched
50+
// height is matched
51+
const reservedNamesRE = new RegExp(`(\\$_*[^_ ]+)|${regexSafeReservedNameList}`, "g");
52+
53+
// Since this regex captures both user variables and our reserved keywords, we need to add some logic in the replacer
54+
const stringWithVariables = stringWithOperators.replace(reservedNamesRE, (match) => {
55+
// Do not do anything to user variables (anything starting with $)
56+
if (match.startsWith('$')) {
57+
return match;
58+
} else {
59+
return RESERVED_NAMES[match as keyof typeof RESERVED_NAMES] || match;
60+
}
61+
});
62+
63+
// Serialize remaining spaces with an underscore
64+
const finalExpressionString = stringWithVariables.replace(/\s/g, '_');
65+
66+
return new ExpressionQualifier(finalExpressionString);
2567
}
2668

2769
// as a namespace

0 commit comments

Comments
 (0)