Skip to content

Commit a152835

Browse files
cloudinary-pkoniucloudinary-pkoniu
committed
chore: pr comments fixes
1 parent 401d5ef commit a152835

7 files changed

Lines changed: 71 additions & 76 deletions

File tree

‎lib/api_client/execute_request.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ function execute_request(method, params, auth, api_url, callback, options = {})
4040
query: parsedUrl.search ? parsedUrl.search.substring(1) : ''
4141
};
4242

43-
// Only add port if not default (to match url.parse() behavior)
4443
if (parsedUrl.port) {
4544
request_options.port = parsedUrl.port;
4645
}

‎lib/config.js‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,9 @@ function parseCloudinaryConfigFromEnvURL(ENV_STR) {
7474
}
7575

7676
function extendCloudinaryConfigFromQuery(ENV_URL, confToExtend = {}) {
77-
const uri = new URL(ENV_URL);
78-
if (uri.search) {
79-
const query = {};
80-
uri.searchParams.forEach((value, key) => {
81-
query[key] = value;
82-
});
77+
const url = new URL(ENV_URL);
78+
if (url.search) {
79+
const query = Object.fromEntries(url.searchParams);
8380
entries(query).forEach(([key, value]) => putNestedValue(confToExtend, key, value));
8481
}
8582
}

‎lib/uploader.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,6 @@ function post(url, post_data, boundary, file, callback, options) {
570570
let filename = options.stream ? options.filename ? options.filename : "file" : basename(file);
571571
file_header = Buffer.from(encodeFilePart(boundary, 'application/octet-stream', 'file', filename), 'binary');
572572
}
573-
// Use WHATWG URL API instead of deprecated url.parse()
574573
const parsedUrl = new URL(url);
575574
let post_options = {
576575
protocol: parsedUrl.protocol,
@@ -580,7 +579,6 @@ function post(url, post_data, boundary, file, callback, options) {
580579
query: parsedUrl.search ? parsedUrl.search.substring(1) : ''
581580
};
582581

583-
// Only add port if not default (to match url.parse() behavior)
584582
if (parsedUrl.port) {
585583
post_options.port = parsedUrl.port;
586584
}

‎lib/utils/index.js‎

Lines changed: 36 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
const crypto = require("crypto");
88
const querystring = require("querystring");
9-
const {URL} = require("url");
9+
const { URL } = require("url");
1010

1111
// Functions used internally
1212
const compact = require("lodash/compact");
@@ -33,7 +33,7 @@ const isUndefined = require("lodash/isUndefined");
3333
const smart_escape = require("./encoding/smart_escape");
3434
const consumeOption = require('./parsing/consumeOption');
3535
const toArray = require('./parsing/toArray');
36-
let {base64EncodeURL} = require('./encoding/base64EncodeURL');
36+
let { base64EncodeURL } = require('./encoding/base64EncodeURL');
3737
const encodeDoubleArray = require('./encoding/encodeDoubleArray');
3838

3939
const config = require("../config");
@@ -501,14 +501,14 @@ function build_eager(transformations) {
501501
*/
502502
function build_custom_headers(headers) {
503503
switch (true) {
504-
case headers == null:
505-
return void 0;
506-
case isArray(headers):
507-
return headers.join("\n");
508-
case isObject(headers):
509-
return entries(headers).map(([k, v]) => `${k}:${v}`).join("\n");
510-
default:
511-
return headers;
504+
case headers == null:
505+
return void 0;
506+
case isArray(headers):
507+
return headers.join("\n");
508+
case isObject(headers):
509+
return entries(headers).map(([k, v]) => `${k}:${v}`).join("\n");
510+
default:
511+
return headers;
512512
}
513513
}
514514

@@ -545,7 +545,7 @@ function generate_transformation_string(options) {
545545
let base_transformations = toArray(consumeOption(options, "transformation", []));
546546
let named_transformation = [];
547547
if (base_transformations.some(isObject)) {
548-
base_transformations = base_transformations.map(tr => utils.generate_transformation_string(isObject(tr) ? clone(tr) : {transformation: tr}));
548+
base_transformations = base_transformations.map(tr => utils.generate_transformation_string(isObject(tr) ? clone(tr) : { transformation: tr }));
549549
} else {
550550
named_transformation = base_transformations.join(".");
551551
base_transformations = [];
@@ -877,7 +877,6 @@ function url(public_id, options = {}) {
877877
return (part != null) && part !== '';
878878
}).join('/').replace(/ /g, '%20');
879879
if (sign_url && !isEmpty(auth_token)) {
880-
// Use WHATWG URL API instead of deprecated url.parse()
881880
// For relative URLs, provide a dummy base URL
882881
const parsedUrl = new URL(resultUrl, 'http://dummy');
883882
auth_token.url = parsedUrl.pathname + parsedUrl.search;
@@ -1148,7 +1147,7 @@ function sort_object_by_key(object) {
11481147
}
11491148

11501149
function merge(hash1, hash2) {
1151-
return {...hash1, ...hash2};
1150+
return { ...hash1, ...hash2 };
11521151
}
11531152

11541153
function sign_request(params, options = {}) {
@@ -1437,15 +1436,15 @@ const OFFSET_ANY_PATTERN_RE = RegExp(`(${OFFSET_ANY_PATTERN})\\.\\.(${OFFSET_ANY
14371436
// Split a range into the start and end values
14381437
function split_range(range) { // :nodoc:
14391438
switch (range.constructor) {
1440-
case String:
1441-
if (!OFFSET_ANY_PATTERN_RE.test(range)) {
1442-
return range;
1443-
}
1444-
return range.split("..");
1445-
case Array:
1446-
return [first(range), last(range)];
1447-
default:
1448-
return [null, null];
1439+
case String:
1440+
if (!OFFSET_ANY_PATTERN_RE.test(range)) {
1441+
return range;
1442+
}
1443+
return range.split("..");
1444+
case Array:
1445+
return [first(range), last(range)];
1446+
default:
1447+
return [null, null];
14491448
}
14501449
}
14511450

@@ -1468,23 +1467,23 @@ function norm_range_value(value) { // :nodoc:
14681467
*/
14691468
function process_video_params(param) {
14701469
switch (param.constructor) {
1471-
case Object: {
1472-
let video = "";
1473-
if ('codec' in param) {
1474-
video = param.codec;
1475-
if ('profile' in param) {
1476-
video += ":" + param.profile;
1477-
if ('level' in param) {
1478-
video += ":" + param.level;
1470+
case Object: {
1471+
let video = "";
1472+
if ('codec' in param) {
1473+
video = param.codec;
1474+
if ('profile' in param) {
1475+
video += ":" + param.profile;
1476+
if ('level' in param) {
1477+
video += ":" + param.level;
1478+
}
14791479
}
14801480
}
1481+
return video;
14811482
}
1482-
return video;
1483-
}
1484-
case String:
1485-
return param;
1486-
default:
1487-
return null;
1483+
case String:
1484+
return param;
1485+
default:
1486+
return null;
14881487
}
14891488
}
14901489

@@ -1537,7 +1536,7 @@ exports.create_source_tag = function create_source_tag(src, source_type, codecs
15371536
};
15381537

15391538
function build_explicit_api_params(public_id, options = {}) {
1540-
return [exports.build_upload_params(extend({}, {public_id}, options))];
1539+
return [exports.build_upload_params(extend({}, { public_id }, options))];
15411540
}
15421541

15431542
function generate_responsive_breakpoints_string(breakpoints) {

‎test/integration/api/uploader/archivespec.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const cloudinary = require("../../../../cloudinary");
99
const helper = require("../../../spechelper");
1010

1111
const testConstants = require('../../../testUtils/testConstants');
12+
const { REQ_TIMEOUT_IN_TEST } = require("../../../testUtils/constants");
1213
const callReusableTest = require('../../../testUtils/reusableTests/reusableTests').callReusableTest;
1314
const registerReusableTest = require('../../../testUtils/reusableTests/reusableTests').registerReusableTest;
1415

@@ -120,8 +121,8 @@ describe("archive", function () {
120121
});
121122
});
122123
});
123-
req.setTimeout(30000, () => {
124-
req.destroy(new Error(`Request timed out after 30000ms: ${this.archive_result}`));
124+
req.setTimeout(REQ_TIMEOUT_IN_TEST, () => {
125+
req.destroy(new Error(`Request timed out after ${REQ_TIMEOUT_IN_TEST}ms: ${this.archive_result}`));
125126
});
126127
req.on('error', (e) => done(e));
127128
});

‎test/testUtils/assertions/beASignedDownloadUrl.js‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,31 @@ const ERRORS = {
1313
FIELD_MUST_NOT_EQUAL_VALUE: (key, value) => `expected field ${key} not to equal ${value}`
1414
}
1515

16+
function normalizePhpStyleArrayQueryParams(queryParams) {
17+
for (let param in queryParams) {
18+
if (param.endsWith("[]")) {
19+
const normalized = param.slice(0, -2);
20+
if (Object.prototype.hasOwnProperty.call(queryParams, normalized)) {
21+
const existing = queryParams[normalized];
22+
const incoming = queryParams[param];
23+
if (Array.isArray(existing) && Array.isArray(incoming)) {
24+
queryParams[normalized] = existing.concat(incoming);
25+
} else if (Array.isArray(existing)) {
26+
queryParams[normalized] = existing.concat([incoming]);
27+
} else if (Array.isArray(incoming)) {
28+
queryParams[normalized] = [existing].concat(incoming);
29+
} else {
30+
// Both are scalars; keep deterministic ordering
31+
queryParams[normalized] = [existing, incoming];
32+
}
33+
} else {
34+
queryParams[normalized] = queryParams[param];
35+
}
36+
delete queryParams[param];
37+
}
38+
}
39+
}
40+
1641
/**
1742
* Asserts that a given string is a signed url.
1843
*
@@ -28,9 +53,6 @@ expect.Assertion.prototype.beASignedDownloadUrl = function (path, params) {
2853
// eslint-disable-next-line no-nested-ternary
2954
const urlString = (typeof apiUrl === "string") ? apiUrl : (apiUrl && apiUrl.href) ? apiUrl.href : String(apiUrl);
3055
const urlOptions = new URL(urlString);
31-
// NOTE: tests run with `jsdom-global()` (old jsdom version), whose URL implementation
32-
// may not include `searchParams`. Parse the query string ourselves to stay compatible,
33-
// and avoid using deprecated `url.parse()`.
3456
const rawQuery = (urlOptions && typeof urlOptions.search === "string") ? urlOptions.search : "";
3557
const queryParams = querystring.parse(rawQuery.startsWith("?") ? rawQuery.slice(1) : rawQuery);
3658

@@ -40,29 +62,7 @@ expect.Assertion.prototype.beASignedDownloadUrl = function (path, params) {
4062
};
4163
const expectedParams = Object.assign(defaultParams, params);
4264

43-
// Rename PHP-style multi-value params to strip '[]' from their names, e.g. urls[] -> urls
44-
for (let param in queryParams) {
45-
if (param.endsWith("[]")) {
46-
const normalized = param.slice(0, -2);
47-
if (Object.prototype.hasOwnProperty.call(queryParams, normalized)) {
48-
const existing = queryParams[normalized];
49-
const incoming = queryParams[param];
50-
if (Array.isArray(existing) && Array.isArray(incoming)) {
51-
queryParams[normalized] = existing.concat(incoming);
52-
} else if (Array.isArray(existing)) {
53-
queryParams[normalized] = existing.concat([incoming]);
54-
} else if (Array.isArray(incoming)) {
55-
queryParams[normalized] = [existing].concat(incoming);
56-
} else {
57-
// Both are scalars; keep deterministic ordering
58-
queryParams[normalized] = [existing, incoming];
59-
}
60-
} else {
61-
queryParams[normalized] = queryParams[param];
62-
}
63-
delete queryParams[param];
64-
}
65-
}
65+
normalizePhpStyleArrayQueryParams(queryParams);
6666

6767
this.assert("timestamp" in queryParams, function () {
6868
return ERRORS.MUST_CONTAIN_QUERY_PARAMETER("timestamp");

‎test/testUtils/assertions/beServedByCloudinary.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ const cloneDeep = require('lodash/cloneDeep');
22
const http = require('http');
33
const https = require('https');
44
const cloudinary = require('../../../cloudinary');
5+
const { REQ_TIMEOUT_IN_TEST } = require('../constants');
56

67

78
expect.Assertion.prototype.beServedByCloudinary = function (done) {
@@ -28,8 +29,8 @@ expect.Assertion.prototype.beServedByCloudinary = function (done) {
2829
res.resume();
2930
});
3031

31-
req.setTimeout(30000, () => {
32-
req.destroy(new Error(`Request timed out after 30000ms: ${actual}`));
32+
req.setTimeout(REQ_TIMEOUT_IN_TEST, () => {
33+
req.destroy(new Error(`Request timed out after ${REQ_TIMEOUT_IN_TEST}ms: ${actual}`));
3334
});
3435

3536
req.on('error', (e) => done(e));

0 commit comments

Comments
 (0)