From 059983166402c26e114d4c0a5319edc319e9701f Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 30 Dec 2023 15:08:55 -0800 Subject: [PATCH 01/15] =?UTF-8?q?Add=20`owner`=20field=20to=20`packages`?= =?UTF-8?q?=20table=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …along with a script for extracting the owner from the repo URL. --- docs/build.md | 4 +- package-lock.json | 17 ++ package.json | 2 + scripts/database/create_packages_table.sql | 9 + scripts/migrations/0001-initial-migration.sql | 1 + scripts/tools/add-owner-field.js | 162 ++++++++++++++++++ src/controllers/getPackages.js | 8 +- src/database.js | 5 + src/models/ssoPaginate.js | 5 +- src/query.js | 16 +- 10 files changed, 221 insertions(+), 8 deletions(-) create mode 100644 scripts/tools/add-owner-field.js diff --git a/docs/build.md b/docs/build.md index c944ecf5..3c6161c2 100644 --- a/docs/build.md +++ b/docs/build.md @@ -98,9 +98,9 @@ There are some additional scripts that you likely won't encounter or need during #### Limiting what's mocked in the dev server -No additionally, when working locally if you need to test a feature where you have no option but to query the remote service (This is not recommended) you do have the option to enable some remote services as needed. +When working locally if you need to test a feature where you have no option but to query the remote service (this is not recommended) you do have the option to enable some remote services as needed. -Again setting any of the below values will contact remote services, meaning you must have the proper Configuration Values set to do so, and you become at risk of causing damage to external data, any deletions that occur here will cause permanent data loss to these services. Or may get you blocked by remote services if there is abuse. +Again, setting any of the below values will contact remote services, meaning you must have the proper Configuration Values set to do so, and you become at risk of causing damage to external data, any deletions that occur here will cause permanent data loss to these services. Or may get you blocked by remote services if there is abuse. The following values can only ever be started when using `npm run start:dev` and will be ignored in all other instances. diff --git a/package-lock.json b/package-lock.json index 719c3d78..d0b3d72e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,6 +15,7 @@ "express": "^4.18.1", "express-rate-limit": "^6.7.0", "js-yaml": "^4.1.0", + "parse-github-url": "^1.0.2", "postgres": "^3.3.4", "superagent": "^8.0.9" }, @@ -7145,6 +7146,17 @@ "node": ">=6" } }, + "node_modules/parse-github-url": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/parse-github-url/-/parse-github-url-1.0.2.tgz", + "integrity": "sha512-kgBf6avCbO3Cn6+RnzRGLkUsv4ZVqv/VfAYkRsyBcgkshNvVBkRn1FEZcW0Jb+npXQWm2vHPnnOqFteZxRRGNw==", + "bin": { + "parse-github-url": "cli.js" + }, + "engines": { + "node": ">=0.10.0" + } + }, "node_modules/parse-json": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/parse-json/-/parse-json-4.0.0.tgz", @@ -14466,6 +14478,11 @@ } } }, + "parse-github-url": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/parse-github-url/-/parse-github-url-1.0.2.tgz", + "integrity": "sha512-kgBf6avCbO3Cn6+RnzRGLkUsv4ZVqv/VfAYkRsyBcgkshNvVBkRn1FEZcW0Jb+npXQWm2vHPnnOqFteZxRRGNw==" + }, "parse-json": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/parse-json/-/parse-json-4.0.0.tgz", diff --git a/package.json b/package.json index c9e27c76..b444906e 100644 --- a/package.json +++ b/package.json @@ -12,6 +12,7 @@ "test:unit": "cross-env NODE_ENV=test PULSAR_STATUS=dev jest --selectProjects Unit-Tests", "test:integration": "cross-env NODE_ENV=test PULSAR_STATUS=dev jest --selectProjects Integration-Tests", "start:dev": "cross-env PULSAR_STATUS=dev node ./src/dev_server.js", + "start:dev:run-task": "cross-env PULSAR_STATUS=dev TASK=1 node ./src/dev_server.js", "lint": "prettier --check -u -w .", "complex": "cr --newmi --config .complexrc .", "js-docs": "jsdoc2md -c ./jsdoc.conf.js ./src/*.js ./src/handlers/*.js ./docs/resources/jsdoc_typedef.js > ./docs/reference/Source_Documentation.md", @@ -47,6 +48,7 @@ "express": "^4.18.1", "express-rate-limit": "^6.7.0", "js-yaml": "^4.1.0", + "parse-github-url": "^1.0.2", "postgres": "^3.3.4", "superagent": "^8.0.9" }, diff --git a/scripts/database/create_packages_table.sql b/scripts/database/create_packages_table.sql index 512958ab..ceb513d4 100644 --- a/scripts/database/create_packages_table.sql +++ b/scripts/database/create_packages_table.sql @@ -60,3 +60,12 @@ CREATE TRIGGER trigger_now_on_updated BEFORE UPDATE ON packages FOR EACH ROW EXECUTE PROCEDURE now_on_updated_package(); + + +-- Add an `owner` field to the table + +-- GitHub username length limit is apparently 39. For future-proofing, we'll +-- envision a prefix that specifies an alternative VCS, so let's build in some +-- wiggle room. +ALTER TABLE packages +ADD COLUMN owner VARCHAR(60); diff --git a/scripts/migrations/0001-initial-migration.sql b/scripts/migrations/0001-initial-migration.sql index 5a81dc18..0b75ba7f 100644 --- a/scripts/migrations/0001-initial-migration.sql +++ b/scripts/migrations/0001-initial-migration.sql @@ -14,6 +14,7 @@ CREATE TABLE packages ( downloads BIGINT NOT NULL DEFAULT 0, stargazers_count BIGINT NOT NULL DEFAULT 0, original_stargazers BIGINT NOT NULL DEFAULT 0, + owner VARCHAR(60), data JSONB, -- constraints CONSTRAINT lowercase_names CHECK (name = LOWER(name)) diff --git a/scripts/tools/add-owner-field.js b/scripts/tools/add-owner-field.js new file mode 100644 index 00000000..0a13f287 --- /dev/null +++ b/scripts/tools/add-owner-field.js @@ -0,0 +1,162 @@ +const fs = require("fs"); +const postgres = require("postgres"); +const parseGithubUrl = require("parse-github-url"); +const { DB_HOST, DB_USER, DB_PASS, DB_DB, DB_PORT, DB_SSL_CERT } = + require("../../src/config.js").getConfig(); + +let sqlStorage; + +const LIMIT = parseInt(process.env.LIMIT ?? '-1', 10); +const OFFSET = parseInt(process.env.OFFSET ?? '0', 10); +const VERBOSE = (process.env.VERBOSE ?? '0') !== '0'; + +function log (...args) { + if (!VERBOSE) return; + return console.log(...args); +} + +function debug (...args) { + if (!VERBOSE) return; + return console.debug(...args); +} + +function warn (...args) { + if (!VERBOSE) return; + return console.warn(...args); +} + + +async function init () { + let allPointers = await getPointers(); + let totalPointers = allPointers.length; + log('Package count:', totalPointers); + + for (let { name, pointer } of allPointers) { + log(`Checking: ${name}::${pointer}`); + + if (typeof name !== "string") { + console.error( + `The package ${name}::${pointer} is invalid without its name!` + ); + continue; + } + if (typeof pointer !== "string") { + console.error( + `The package ${name}::${pointer} likely has been deleted.` + ); + continue; + } + + let { ok, content: pack } = await getPackageData(pointer); + if (!ok) { + warn(`Error getting package ${name}!`); + continue; + } + + let meta = pack?.data; + let repositoryUrl = meta?.repository.url; + if (!repositoryUrl) { + console.error(`No repository URL found for package ${name}!`); + continue; + } + + let parsed = parseGithubUrl(repositoryUrl); + if (parsed === null) { + console.error(`Could not parse repo URL for package: ${name}: ${repositoryUrl}`); + continue; + } + + let { owner } = parsed; + if (pack.owner === owner) { + debug(`Package ${name} already has the right owner!`); + continue; + } + + log('Updating owner field of package:', name); + await sqlStorage` + UPDATE packages SET owner = ${owner} WHERE pointer = ${pointer}; + `; + } + + await sqlEnd(); + process.exit(0); +} + +async function getPointers() { + sqlStorage ??= setupSQL(); + + let command; + + if (LIMIT > 0) { + command = await sqlStorage` + SELECT * FROM names LIMIT ${LIMIT} OFFSET ${OFFSET}; + `; + } else { + command = await sqlStorage` + SELECT * FROM names; + `; + } + + if (command.count === 0) { + console.log("Failed to get all package pointers."); + await sqlEnd(); + process.exit(1); + } + + return command; +} + +async function getPackageData(pointer) { + console.log('getting package data:', pointer); + + try { + sqlStorage ??= setupSQL(); + const command = await sqlStorage` + SELECT * from packages + WHERE pointer = ${pointer} + `; + + if (command.count === 0) { + return { ok: false, content: `Failed to get package data of ${pointer}` }; + } + + return { ok: true, content: command[0] }; + } catch (err) { + console.error(`ERROR!`); + console.error(err); + } +} + +function setupSQL() { + try { + let options = { + host: DB_HOST, + username: DB_USER, + password: DB_PASS, + database: DB_DB, + port: DB_PORT, + }; + if (DB_SSL_CERT) { + options.ssl = { + rejectUnauthorized: true, + ca: fs.readFileSync(DB_SSL_CERT).toString(), + }; + } + sqlStorage = postgres(options); + + return sqlStorage; + } catch (err) { + console.error(err); + process.exit(100); + } +} + +async function sqlEnd() { + if (sqlStorage !== undefined) { + await sqlStorage.end(); + console.log("Task done!"); + } + return; +} + +module.exports = init; diff --git a/src/controllers/getPackages.js b/src/controllers/getPackages.js index c1a8e713..d3215820 100644 --- a/src/controllers/getPackages.js +++ b/src/controllers/getPackages.js @@ -23,7 +23,10 @@ module.exports = { serviceType: (context, req) => { return context.query.serviceType(req); }, service: (context, req) => { return context.query.service(req); }, serviceVersion: (context, req) => { return context.query.serviceVersion(req); }, - fileExtension: (context, req) => { return context.query.fileExtension(req); } + fileExtension: (context, req) => { return context.query.fileExtension(req); }, + user: (context, req) => { + return context.query.user(req); + } }, /** @@ -51,7 +54,8 @@ module.exports = { const ssoP = new context.ssoPaginate(); - ssoP.total = packages.pagination.total; + ssoP.resultCount = packages.pagination.count; + ssoP.totalPages = packages.pagination.total; ssoP.limit = packages.pagination.limit; ssoP.buildLink(`${context.config.server_url}/api/packages`, packages.pagination.page, params); diff --git a/src/database.js b/src/database.js index 9e02a358..752e1229 100644 --- a/src/database.js +++ b/src/database.js @@ -1602,6 +1602,11 @@ async function getSortedPackages(opts, themes = false) { ? sqlStorage`WHERE ${opts.fileExtension}=ANY(v.supported_languages)` : sqlStorage`` } + ${ + typeof opts.user === "string" + ? sqlStorage`WHERE p.owner = ${opts.user}` + : sqlStorage`` + } ORDER BY p.name, v.semver_v1 DESC, v.semver_v2 DESC, v.semver_v3 DESC, v.created DESC ) SELECT *, COUNT(*) OVER() AS query_result_count diff --git a/src/models/ssoPaginate.js b/src/models/ssoPaginate.js index ef3caa47..3aaef40a 100644 --- a/src/models/ssoPaginate.js +++ b/src/models/ssoPaginate.js @@ -6,7 +6,8 @@ class SSOPaginate extends SSO { super(); this.link = ""; - this.total = 0; + this.totalPages = 0; + this.resultCount = 0; this.limit = 0; } @@ -44,7 +45,7 @@ class SSOPaginate extends SSO { handleSuccess(req, res, context) { res.append("Link", this.link); - res.append("Query-Total", this.total); + res.append("Query-Total", this.resultCount); res.append("Query-Limit", this.limit); res.status(this.successStatusCode).json(this.content); diff --git a/src/query.js b/src/query.js index d43d6def..319a8257 100644 --- a/src/query.js +++ b/src/query.js @@ -283,13 +283,24 @@ function serviceVersion(req) { * @function service * @desc Returns the service being requested. * @param {object} req - The `Request` object inherited from the Express endpoint. - * @returns {string|boolean} Returns false if the provided value is invalid, or - * nonexistant. Returns the service string otherwise. + * @returns {string|boolean} Returns false if the provided value is invalid or + * nonexistent. Returns the service string otherwise. */ function service(req) { return stringValidation(req.query.service); } +/** + * @function user + * @param {object} req - The `Request` object inherited from the Express + * endpoint. + * @returns {string|boolean} Returns false if the provided value is invalid or + * nonexistent. Returns the user name otherwise. + */ +function user (req) { + return stringValidation(req.query.user); +} + /** * @function fileExtension * @desc Returns the file extension being requested. @@ -340,5 +351,6 @@ module.exports = { serviceType, serviceVersion, service, + user, fileExtension, }; From 4a0a1f7e5a30961ea814a2e942c30a91443fe2f0 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 30 Dec 2023 15:15:05 -0800 Subject: [PATCH 02/15] Remove temporary task --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index b444906e..f845a91d 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,6 @@ "test:unit": "cross-env NODE_ENV=test PULSAR_STATUS=dev jest --selectProjects Unit-Tests", "test:integration": "cross-env NODE_ENV=test PULSAR_STATUS=dev jest --selectProjects Integration-Tests", "start:dev": "cross-env PULSAR_STATUS=dev node ./src/dev_server.js", - "start:dev:run-task": "cross-env PULSAR_STATUS=dev TASK=1 node ./src/dev_server.js", "lint": "prettier --check -u -w .", "complex": "cr --newmi --config .complexrc .", "js-docs": "jsdoc2md -c ./jsdoc.conf.js ./src/*.js ./src/handlers/*.js ./docs/resources/jsdoc_typedef.js > ./docs/reference/Source_Documentation.md", From 3643cbc7e7b8afd3859d5a3b59b331632ca983df Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 30 Dec 2023 15:16:44 -0800 Subject: [PATCH 03/15] Rename `user` to `owner` where appropriate --- src/controllers/getPackages.js | 4 ++-- src/database.js | 4 ++-- src/query.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/controllers/getPackages.js b/src/controllers/getPackages.js index d3215820..5ad4fe95 100644 --- a/src/controllers/getPackages.js +++ b/src/controllers/getPackages.js @@ -24,8 +24,8 @@ module.exports = { service: (context, req) => { return context.query.service(req); }, serviceVersion: (context, req) => { return context.query.serviceVersion(req); }, fileExtension: (context, req) => { return context.query.fileExtension(req); }, - user: (context, req) => { - return context.query.user(req); + owner: (context, req) => { + return context.query.owner(req); } }, diff --git a/src/database.js b/src/database.js index 752e1229..0a9d72dd 100644 --- a/src/database.js +++ b/src/database.js @@ -1603,8 +1603,8 @@ async function getSortedPackages(opts, themes = false) { : sqlStorage`` } ${ - typeof opts.user === "string" - ? sqlStorage`WHERE p.owner = ${opts.user}` + typeof opts.owner === "string" + ? sqlStorage`WHERE p.owner = ${opts.owner}` : sqlStorage`` } ORDER BY p.name, v.semver_v1 DESC, v.semver_v2 DESC, v.semver_v3 DESC, v.created DESC diff --git a/src/query.js b/src/query.js index 319a8257..10e723bd 100644 --- a/src/query.js +++ b/src/query.js @@ -297,8 +297,8 @@ function service(req) { * @returns {string|boolean} Returns false if the provided value is invalid or * nonexistent. Returns the user name otherwise. */ -function user (req) { - return stringValidation(req.query.user); +function owner (req) { + return stringValidation(req.query.owner); } /** From 0df91798348a68c6a1a8ef2aeaf3c96ebec3d95b Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 30 Dec 2023 15:17:20 -0800 Subject: [PATCH 04/15] Oops --- src/query.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/query.js b/src/query.js index 10e723bd..5d47d5f0 100644 --- a/src/query.js +++ b/src/query.js @@ -351,6 +351,6 @@ module.exports = { serviceType, serviceVersion, service, - user, + owner, fileExtension, }; From e74190bf4efc2b36fe8800d54a664e15ea0df156 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 30 Dec 2023 15:38:23 -0800 Subject: [PATCH 05/15] Insert the owner field on package creation and update --- src/PackageObject.js | 7 +++++++ src/database.js | 36 +++++++++++++++++++++++++++++------- tests/models/.eslintrc.js | 8 ++++++++ 3 files changed, 44 insertions(+), 7 deletions(-) create mode 100644 tests/models/.eslintrc.js diff --git a/src/PackageObject.js b/src/PackageObject.js index cc424a31..95a0ecb9 100644 --- a/src/PackageObject.js +++ b/src/PackageObject.js @@ -1,3 +1,5 @@ +const parseGithubUrl = require('parse-github-url'); + /** * @module PackageObject * @desc This Module is used to aide in building Package Objects. @@ -26,6 +28,7 @@ class PackageObject { this.stargazers_count = undefined; this.readme = undefined; this.creationMethod = undefined; + this.owner = undefined; } /** @@ -167,6 +170,10 @@ class PackageObject { */ setRepositoryURL(repoURL) { this.repository.url = repoURL; + let parsed = parseGithubUrl(repoURL); + if (parsed) { + this.owner = parsed.owner; + } return this; } diff --git a/src/database.js b/src/database.js index 0a9d72dd..d4d75db9 100644 --- a/src/database.js +++ b/src/database.js @@ -133,8 +133,8 @@ async function insertNewPackage(pack) { // No need to specify downloads and stargazers. They default at 0 on creation. // TODO: data column deprecated; to be removed insertNewPack = await sqlTrans` - INSERT INTO packages (name, creation_method, data, package_type) - VALUES (${pack.name}, ${pack.creation_method}, ${pack}, ${packageType}) + INSERT INTO packages (name, creation_method, data, package_type, owner) + VALUES (${pack.name}, ${pack.creation_method}, ${pack}, ${packageType}, ${pack.owner}) RETURNING pointer; `; } catch (e) { @@ -236,13 +236,35 @@ async function insertNewPackageVersion(packJSON, oldName = null) { // otherwise use the name in the package object directly. let packName = rename ? oldName : packJSON.name; - const packID = await getPackageByNameSimple(packName); + const pack = await getPackageByName(packName); - if (!packID.ok) { - return packID; + if (!pack.ok) { + return pack; } - const pointer = packID.content.pointer; + const pointer = pack.content.pointer; + + if (packJSON.owner !== pack.owner) { + // The package owner has changed. Whether or not this is plausible in + // the real world, it's a good idea to handle it here. + let updateOwner = {}; + let ownerUpdateFailed = false; + try { + updateOwner = await sqlTrans` + UPDATE PACKAGES + SET owner = ${packJSON.owner} + WHERE pointer = ${pointer} + RETURNING owner; + ` + } catch (e) { + // There aren't constraints on the `owner` field, so if this were to + // fail, it wouldn't be clear why. But we're handling it anyway! + ownerUpdateFailed = true; + } + if (!updateOwner?.count || ownerUpdateFailed) { + throw `Unable to update the package owner to ${packJSON.owner}.`; + } + } if (rename) { // The flow for renaming the package. @@ -545,7 +567,7 @@ async function insertNewUser(username, id, avatar) { ? { ok: true, content: command[0] } : { ok: false, - content: `Unable to create user: ${user}`, + content: `Unable to create user: ${username}`, short: "Server Error", }; } catch (err) { diff --git a/tests/models/.eslintrc.js b/tests/models/.eslintrc.js new file mode 100644 index 00000000..b21821d2 --- /dev/null +++ b/tests/models/.eslintrc.js @@ -0,0 +1,8 @@ +module.exports = { + globals: { + Joi: true + }, + rules: { + camelcase: 'off' + } +}; From 62f26845c7b19686e25d00399367674f1c4976a2 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 30 Dec 2023 15:42:41 -0800 Subject: [PATCH 06/15] Update other usages of `SSOPaginate` --- src/controllers/getPackagesSearch.js | 3 ++- src/controllers/getThemes.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/controllers/getPackagesSearch.js b/src/controllers/getPackagesSearch.js index b62b01f6..453e06ed 100644 --- a/src/controllers/getPackagesSearch.js +++ b/src/controllers/getPackagesSearch.js @@ -84,7 +84,8 @@ module.exports = { const ssoP = new context.ssoPaginate(); - ssoP.total = packs.pagination.total; + ssoP.resultCount = packs.pagination.count; + ssoP.totalPages = packs.pagination.total; ssoP.limit = packs.pagination.limit; ssoP.buildLink(`${context.config.server_url}/api/packages/search`, packs.pagination.page, params); diff --git a/src/controllers/getThemes.js b/src/controllers/getThemes.js index f989b196..f0ca2b32 100644 --- a/src/controllers/getThemes.js +++ b/src/controllers/getThemes.js @@ -56,7 +56,8 @@ module.exports = { const ssoP = new context.ssoPaginate(); - ssoP.total = packages.pagination.total; + ssoP.resultCount = packages.pagination.count; + ssoP.totalPages = packages.pagination.total; ssoP.limit = packages.pagination.limit; ssoP.buildLink(`${context.config.server_url}/api/themes`, packages.pagination.page, params); From 099483cbaa5ca6c8a22faf28ece1f067cf0d9314 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 30 Dec 2023 15:43:17 -0800 Subject: [PATCH 07/15] Address feedback from robot --- src/database.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/database.js b/src/database.js index d4d75db9..38ba57e5 100644 --- a/src/database.js +++ b/src/database.js @@ -255,7 +255,7 @@ async function insertNewPackageVersion(packJSON, oldName = null) { SET owner = ${packJSON.owner} WHERE pointer = ${pointer} RETURNING owner; - ` + `; } catch (e) { // There aren't constraints on the `owner` field, so if this were to // fail, it wouldn't be clear why. But we're handling it anyway! From 97f3f1b5e5f870a4ecbc9b591f0848d728ec40b4 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 30 Dec 2023 15:48:49 -0800 Subject: [PATCH 08/15] Add test for `owner` query parameter --- tests/unit/query.test.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/unit/query.test.js b/tests/unit/query.test.js index d4d8c111..bb8ab92d 100644 --- a/tests/unit/query.test.js +++ b/tests/unit/query.test.js @@ -177,3 +177,16 @@ describe("Verify fileExtension Returns", () => { expect(query.fileExtension(arg)).toBe(result); }); }); + +const ownerCases = [ + [{ query: { owner: "savetheclocktower" } }, "savetheclocktower"], + [{ query: { owner: '' } }, false], + [{ query: { owner: 101 } }, false], + [{ query: {} }, false], +]; + +describe("Verify owner Returns", () => { + test.each(ownerCases)("Given %o Returns %p", (arg, result) => { + expect(query.owner(arg)).toBe(result); + }); +}); From 9898d5be597e2c821ac8298a804face8985fab20 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 30 Dec 2023 15:49:29 -0800 Subject: [PATCH 09/15] Test that owner is created on package insertion --- tests/http/getPackagesPackageName.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/http/getPackagesPackageName.test.js b/tests/http/getPackagesPackageName.test.js index 148c4ab4..060a6a36 100644 --- a/tests/http/getPackagesPackageName.test.js +++ b/tests/http/getPackagesPackageName.test.js @@ -64,6 +64,7 @@ describe("Behaves as expected", () => { expect(sso.ok).toBe(true); expect(sso.content.name).toBe("get-package-test"); + expect(sso.content.owner).toBe("confused-Techie"); expect(sso).toMatchEndpointSuccessObject(endpoint); await database.removePackageByName("get-package-test", true); }); From 1699e9eff5e47b7e33a51639c40d37df3c50decb Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 30 Dec 2023 16:45:36 -0800 Subject: [PATCH 10/15] Get tests passing --- src/models/ssoPaginate.js | 4 ++-- src/query.js | 8 +++++++- tests/database/applyFeatures.test.js | 2 ++ tests/database/database.test.js | 1 + tests/database/extensionFilter.test.js | 1 + .../valid_multi_version.js | 1 + .../git.createPackage_returns/valid_one_version.js | 1 + tests/database/fixtures/lifetime/package-a.js | 1 + tests/helpers/global.setup.jest.js | 1 + tests/http/deletePackagesPackageName.test.js | 1 + ...tePackagesPackageNameVersionsVersionName.test.js | 1 + tests/http/getPackagesFeatured.test.js | 1 + tests/http/getPackagesPackageName.test.js | 1 + tests/http/getThemes.test.js | 2 ++ tests/http/getThemesFeatured.test.js | 1 + tests/http/getThemesSearch.test.js | 1 + tests/http/postPackages.test.js | 2 ++ tests/http/postPackagesPackageNameStar.test.js | 1 + tests/models/packageObjectFull.js | 7 +++++-- tests/models/packageObjectShort.js | 7 +++++-- tests/unit/PackageObject.test.js | 13 +++++++++++++ 21 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/models/ssoPaginate.js b/src/models/ssoPaginate.js index 3aaef40a..0715afa1 100644 --- a/src/models/ssoPaginate.js +++ b/src/models/ssoPaginate.js @@ -33,9 +33,9 @@ class SSOPaginate extends SSO { let linkString = ""; linkString += `<${url}?page=${currentPage}${paramString}>; rel="self", `; - linkString += `<${url}?page=${this.total}${paramString}>; rel="last"`; + linkString += `<${url}?page=${this.totalPages}${paramString}>; rel="last"`; - if (currentPage !== this.total) { + if (currentPage !== this.totalPages) { linkString += `, <${url}?page=${parseInt(currentPage) + 1}${paramString}>; rel="next"`; } diff --git a/src/query.js b/src/query.js index 5d47d5f0..2c237e4f 100644 --- a/src/query.js +++ b/src/query.js @@ -298,7 +298,13 @@ function service(req) { * nonexistent. Returns the user name otherwise. */ function owner (req) { - return stringValidation(req.query.owner); + if (!stringValidation(req.query.owner)) { + return false; + } + if (req.query.owner.length === 0) { + return false; + } + return req.query.owner; } /** diff --git a/tests/database/applyFeatures.test.js b/tests/database/applyFeatures.test.js index 18545616..567c6018 100644 --- a/tests/database/applyFeatures.test.js +++ b/tests/database/applyFeatures.test.js @@ -29,6 +29,7 @@ describe("Returns OK even if not making any changes", () => { type: "git", url: "https://github.com/confused-Techie/applyFeatures-ExitsProperly", }, + owner: "confused-Techie", downloads: 0, stargazers_count: 0, creation_method: "Test Run", @@ -99,6 +100,7 @@ describe("Adds data properly for features", () => { type: "git", url: "https://github.com/confused-Techie/applyFeatures-Proper", }, + owner: "confused-Techie", downloads: 0, stargazers_count: 0, creation_method: "Test Run", diff --git a/tests/database/database.test.js b/tests/database/database.test.js index 4820b7c9..671c35ea 100644 --- a/tests/database/database.test.js +++ b/tests/database/database.test.js @@ -479,6 +479,7 @@ describe("Package Lifecycle Tests", () => { url: "https://github.com/confused-Techie/package-backend", type: "git" }, + owner: "confused-Techie", creation_method: "Test Package", releases: { latest: "1.0.0" }, readme: "This is a readme!", diff --git a/tests/database/extensionFilter.test.js b/tests/database/extensionFilter.test.js index 5b6d5e65..a7b5b417 100644 --- a/tests/database/extensionFilter.test.js +++ b/tests/database/extensionFilter.test.js @@ -24,6 +24,7 @@ describe("Returns correct data when filtering by extension", () => { type: "git", url: "https://github.com/confused-Techie/extensionFilter-Returns", }, + owner: "confused-Techie", downloads: 0, stargazers_count: 0, creation_method: "Test run", diff --git a/tests/database/fixtures/git.createPackage_returns/valid_multi_version.js b/tests/database/fixtures/git.createPackage_returns/valid_multi_version.js index 8223dc22..d0c39579 100644 --- a/tests/database/fixtures/git.createPackage_returns/valid_multi_version.js +++ b/tests/database/fixtures/git.createPackage_returns/valid_multi_version.js @@ -6,6 +6,7 @@ module.exports = { }, creation_method: "Test Package", readme: "This is a readme!", + owner: "pulsar-edit", metadata: { name: "publish-test-valid-multi-version", license: "MIT", diff --git a/tests/database/fixtures/git.createPackage_returns/valid_one_version.js b/tests/database/fixtures/git.createPackage_returns/valid_one_version.js index 8101d680..79289635 100644 --- a/tests/database/fixtures/git.createPackage_returns/valid_one_version.js +++ b/tests/database/fixtures/git.createPackage_returns/valid_one_version.js @@ -11,6 +11,7 @@ module.exports = { license: "MIT", version: "1.0.0", }, + owner: "confused-Techie", releases: { latest: "1.0.0", }, diff --git a/tests/database/fixtures/lifetime/package-a.js b/tests/database/fixtures/lifetime/package-a.js index be81823a..72edffd0 100644 --- a/tests/database/fixtures/lifetime/package-a.js +++ b/tests/database/fixtures/lifetime/package-a.js @@ -4,6 +4,7 @@ const createPack = { type: "git", url: "https://github.com/pulsar-edit/package-a-lifetime", }, + owner: "pulsar-edit", creation_method: "Test Package", readme: "This is a readme!", metadata: { diff --git a/tests/helpers/global.setup.jest.js b/tests/helpers/global.setup.jest.js index de08a48a..8485d276 100644 --- a/tests/helpers/global.setup.jest.js +++ b/tests/helpers/global.setup.jest.js @@ -57,6 +57,7 @@ expect.extend({ let done = false; for (const response in endpoint.docs.responses) { // We use `==` to facilitate type coercion + // eslint-disable-next-line eqeqeq if (response == endpoint.endpoint.successStatus) { let obj = endpoint.docs.responses[response].content["application/json"]; diff --git a/tests/http/deletePackagesPackageName.test.js b/tests/http/deletePackagesPackageName.test.js index 18aa8197..81374522 100644 --- a/tests/http/deletePackagesPackageName.test.js +++ b/tests/http/deletePackagesPackageName.test.js @@ -49,6 +49,7 @@ describe("DELETE /api/packages/:packageName", () => { url: "https://github.com/confused-Techie/package-backend", type: "git" }, + owner: "confused-Techie", creation_method: "Test Package", releases: { latest: "1.0.0" diff --git a/tests/http/deletePackagesPackageNameVersionsVersionName.test.js b/tests/http/deletePackagesPackageNameVersionsVersionName.test.js index 6ef270a8..52b0caa3 100644 --- a/tests/http/deletePackagesPackageNameVersionsVersionName.test.js +++ b/tests/http/deletePackagesPackageNameVersionsVersionName.test.js @@ -49,6 +49,7 @@ describe("DELETE /api/packages/:packageName/versions/:versionName", () => { url: "https://github.com/confused-Techie/package-backend", type: "git" }, + owner: "confused-Techie", creation_method: "Test Package", releases: { latest: "1.0.1" diff --git a/tests/http/getPackagesFeatured.test.js b/tests/http/getPackagesFeatured.test.js index 1131a872..f835361f 100644 --- a/tests/http/getPackagesFeatured.test.js +++ b/tests/http/getPackagesFeatured.test.js @@ -29,6 +29,7 @@ describe("Behaves as expected", () => { url: "https://github.com/Spiker985/x-terminal-reloaded", type: "git" }, + owner: "Spiker985", creation_method: "Test Package", releases: { latest: "1.1.0" diff --git a/tests/http/getPackagesPackageName.test.js b/tests/http/getPackagesPackageName.test.js index 060a6a36..117e8353 100644 --- a/tests/http/getPackagesPackageName.test.js +++ b/tests/http/getPackagesPackageName.test.js @@ -31,6 +31,7 @@ describe("Behaves as expected", () => { url: "https://github.com/confused-Techie/package-backend", type: "git" }, + owner: "confused-Techie", creation_method: "Test Package", releases: { latest: "1.1.0" diff --git a/tests/http/getThemes.test.js b/tests/http/getThemes.test.js index 56b9dfb1..4e37991d 100644 --- a/tests/http/getThemes.test.js +++ b/tests/http/getThemes.test.js @@ -53,6 +53,7 @@ describe("Behaves as expected", () => { name: "test-package", theme: "syntax" }, + owner: "confused-Techie", versions: { "1.1.0": { dist: { @@ -84,6 +85,7 @@ describe("Behaves as expected", () => { expect(sso.content).toBeArray(); expect(sso.content.length).toBe(1); expect(sso.content[0].name).toBe("test-package"); + expect(sso.content[0].owner).toBe("confused-Techie"); expect(sso.link).toBe( `<${context.config.server_url}/api/themes?page=1&sort=downloads&direction=desc>;` + ' rel="self", ' diff --git a/tests/http/getThemesFeatured.test.js b/tests/http/getThemesFeatured.test.js index bcce1672..c172a32f 100644 --- a/tests/http/getThemesFeatured.test.js +++ b/tests/http/getThemesFeatured.test.js @@ -29,6 +29,7 @@ describe("Behaves as expected", () => { url: "https://github.com/confused-Techie/package-backend", type: "git" }, + owner: "confused-Techie", creation_method: "Test Package", releases: { latest: "1.1.0" diff --git a/tests/http/getThemesSearch.test.js b/tests/http/getThemesSearch.test.js index 2e2ca9fa..b4b5bd20 100644 --- a/tests/http/getThemesSearch.test.js +++ b/tests/http/getThemesSearch.test.js @@ -34,6 +34,7 @@ describe("Behaves as expected", () => { url: "https://github.com/confused-Techie/package-backend", type: "git" }, + owner: "confused-Techie", creation_method: "Test Package", releases: { latest: "1.1.0" diff --git a/tests/http/postPackages.test.js b/tests/http/postPackages.test.js index 771b3e94..660d0017 100644 --- a/tests/http/postPackages.test.js +++ b/tests/http/postPackages.test.js @@ -90,6 +90,7 @@ describe("POST /api/packages Behaves as expected", () => { type: "git" }, creation_method: "Test Package", + owner: 'confused-Techie', releases: { latest: "1.1.0" }, readme: "This is a readme!", metadata: { name: "post-packages-test-package" }, @@ -153,6 +154,7 @@ describe("POST /api/packages Behaves as expected", () => { url: "https://github.com/confused-Techie/package-backend", type: "git" }, + owner: 'confused-Techie', downloads: 0, stargazers_count: 0, creation_method: "Test Package", diff --git a/tests/http/postPackagesPackageNameStar.test.js b/tests/http/postPackagesPackageNameStar.test.js index 6401cdc6..9734c02b 100644 --- a/tests/http/postPackagesPackageNameStar.test.js +++ b/tests/http/postPackagesPackageNameStar.test.js @@ -50,6 +50,7 @@ describe("POST /api/packages/:packageName/star", () => { url: "https://github.com/confused-Techie/package-backend", type: "git" }, + owner: "confused-Techie", creation_method: "Test Package", releases: { latest: "1.0.0" diff --git a/tests/models/packageObjectFull.js b/tests/models/packageObjectFull.js index 24a6b4f0..be552728 100644 --- a/tests/models/packageObjectFull.js +++ b/tests/models/packageObjectFull.js @@ -4,7 +4,7 @@ module.exports = { type: "object", required: [ "name", "readme", "metadata", "releases", "versions", - "repository", "creation_method", "downloads", "stargazers_count", "badges" + "repository", "creation_method", "downloads", "stargazers_count", "badges", "owner" ], properties: { name: { type: "string" }, @@ -16,13 +16,15 @@ module.exports = { creation_method: { type: "string" }, downloads: { type: "string" }, stargazers_count: { type: "string" }, - badges: { type: "array" } + badges: { type: "array" }, + owner: { type: "string" } } }, example: { // This is nearly the full return of `language-powershell-revised` name: "language-powershell-revised", readme: "This is the full content of a readme file!", + owner: "confused-Techie", metadata: { // The metadata field is the `package.json` of the most recent version // With the `dist` object added @@ -88,6 +90,7 @@ module.exports = { releases: Joi.object({ latest: Joi.string().required() }).required(), + owner: Joi.string().required(), versions: Joi.object().required(), repository: Joi.object({ url: Joi.string().required(), diff --git a/tests/models/packageObjectShort.js b/tests/models/packageObjectShort.js index b39f3cd8..e0d0d969 100644 --- a/tests/models/packageObjectShort.js +++ b/tests/models/packageObjectShort.js @@ -4,7 +4,7 @@ module.exports = { type: "object", required: [ "name", "readme", "metadata", "repository", "downloads", "stargazers_count", - "releases", "badges" + "releases", "badges", "owner" ], properties: { name: { type: "string" }, @@ -15,13 +15,15 @@ module.exports = { downloads: { type: "string" }, stargazers_count: { type: "string" }, releases: { type: "object" }, - badges: { type: "array" } + badges: { type: "array" }, + owner: { type: "string" } } }, example: { // Example taken from `platformio-ide-terminal` name: "platformio-ide-terminal", readme: "This is the full content of a readme file!", + owner: "platformio", metadata: { main: "./lib/plaformio-ide-terminal", name: "platformio-ide-terminal", @@ -92,6 +94,7 @@ module.exports = { releases: Joi.object({ latest: Joi.string().required() }).required(), + owner: Joi.string().required(), repository: Joi.object({ url: Joi.string().required(), type: Joi.string().valid( diff --git a/tests/unit/PackageObject.test.js b/tests/unit/PackageObject.test.js index 3ec1f1d0..d58ee529 100644 --- a/tests/unit/PackageObject.test.js +++ b/tests/unit/PackageObject.test.js @@ -7,6 +7,19 @@ describe("Building Objects with PackageObject Return as Expected", () => { expect(obj.name).toBe("hello"); }); + test("Extracting owner", () => { + let obj = new PackageObject() + .setName("hello") + .setOwnerRepo("pulsar/hello") + .setDownloads(100) + .setStargazers(200) + .setReadme("## Hello this is a readme!") + .setRepositoryURL("https://github.com/pulsar-edit/hello") + .setRepositoryType("git"); + + expect(obj.owner).toBe("pulsar-edit"); + }); + test("Adding multiple versions", () => { let obj = new PackageObject() .setName("hello") From 4f5cb4ed7d99c86e6df64736441ba1dd3befdda9 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 30 Dec 2023 16:58:50 -0800 Subject: [PATCH 11/15] Make some `.eslintrc` changes --- .eslintrc.js | 9 --------- src/dev_server.js | 2 ++ 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index edd141ec..1485d885 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -15,20 +15,11 @@ module.exports = { { allowModules: [ "supertest", - "../node_modules/@databases/pg-test/jest/globalSetup", - "../node_modules/@databases/pg-test/jest/globalTeardown", ], }, ], "no-process-exit": "off", // Custom Rules as Determined by the maintainers of package-backend - camelcase: [ - "error", - { - ignoreGlobals: true, - ignoreImports: true, - }, - ], complexity: ["error"], eqeqeq: ["error", "smart"], "max-depth": ["error", 4], diff --git a/src/dev_server.js b/src/dev_server.js index f1b98ad6..4bc139a2 100644 --- a/src/dev_server.js +++ b/src/dev_server.js @@ -9,8 +9,10 @@ /** * This is the recommended and only way to mock how Jest would use the module. */ +/* eslint-disable node/no-unpublished-require */ const dbSetup = require("../node_modules/@databases/pg-test/jest/globalSetup"); const dbTeardown = require("../node_modules/@databases/pg-test/jest/globalTeardown"); +/* eslint-enable node/no-unpublished-require */ async function test() { await processArgs(); From d9a48507e9bdc5cdf5a16d4e3c1d32b1b4fd4c66 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 30 Dec 2023 19:41:15 -0800 Subject: [PATCH 12/15] Ensure we get `order` back from the database --- src/database.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/database.js b/src/database.js index 38ba57e5..9c6d5b6c 100644 --- a/src/database.js +++ b/src/database.js @@ -1597,7 +1597,7 @@ async function getSortedPackages(opts, themes = false) { const command = await sqlStorage` WITH latest_versions AS ( - SELECT DISTINCT ON (p.name) p.name, p.data, p.downloads, + SELECT DISTINCT ON (p.name) p.name, p.data, p.downloads, p.owner, (p.stargazers_count + p.original_stargazers) AS stargazers_count, v.semver, p.created, v.updated, p.creation_method FROM packages AS p From 69237d264c6348900a864359aa4bfaa3f7c68c1a Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 30 Dec 2023 19:41:45 -0800 Subject: [PATCH 13/15] Include the `owner` property in long/short package output --- src/utils.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/utils.js b/src/utils.js index cc57fdf9..5a859bc8 100644 --- a/src/utils.js +++ b/src/utils.js @@ -67,6 +67,7 @@ async function constructPackageObjectFull(pack) { let newPack = pack.data; newPack.name = pack.name; newPack.downloads = pack.downloads; + newPack.owner = pack.owner; newPack.stargazers_count = pack.stargazers_count; newPack.versions = parseVersions(pack.versions); // database.getPackageByName() sorts the JSON array versions in descending order, @@ -124,6 +125,7 @@ async function constructPackageObjectShort(pack) { // Remove keys that aren't intended to exist in a Package Object Short delete newPack.versions; + newPack.owner = p.owner; return newPack; }; @@ -210,10 +212,11 @@ async function constructPackageObjectJSON(pack) { * @async * @function engineFilter * @desc A complex function that provides filtering by Atom engine version. - * This should take a package with it's versions and retrieve whatever matches + * This should take a package with its versions and retrieve whatever matches * that engine version as provided. * @returns {object} The filtered object. */ +// eslint-disable-next-line complexity async function engineFilter(pack, engine) { // If a compatible version is found, we add its data to the metadata property of the package // Otherwise we return an unmodified package, so that it is usable to the consumer. @@ -238,7 +241,8 @@ async function engineFilter(pack, engine) { return pack; } - // We will want to loop through each version of the package, and check its engine version against the specified one. + // We will want to loop through each version of the package, and check its + // engine version against the specified one. let compatibleVersion = ""; for (const ver in pack.versions) { @@ -247,13 +251,15 @@ async function engineFilter(pack, engine) { continue; } - // Core Atom Packages contain '*' as the engine type, and will require a manual check. + // Core Atom Packages contain '*' as the engine type, and will require a + // manual check. if (pack.versions[ver].engines.atom === "*") { break; } // Track the upper and lower end conditions. - // Null type means not available; Bool type means available with the relative result. + // Null type means not available; Bool type means available with the + // relative result. let lowerEnd = null; let upperEnd = null; From af71ce8188033ec280cd9a1674932478d8a94941 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 30 Dec 2023 19:58:26 -0800 Subject: [PATCH 14/15] Select `owner` column on all package queries --- src/database.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/database.js b/src/database.js index 9c6d5b6c..27c9dbd6 100644 --- a/src/database.js +++ b/src/database.js @@ -602,7 +602,7 @@ async function getPackageByName(name, user = false) { SELECT ${ user ? sqlStorage`` : sqlStorage`p.pointer,` - } p.name, p.created, p.updated, p.creation_method, p.downloads, p.data, + } p.name, p.created, p.updated, p.creation_method, p.downloads, p.data, p.owner, (p.stargazers_count + p.original_stargazers) AS stargazers_count, JSONB_AGG( JSON_BUILD_OBJECT( @@ -724,7 +724,7 @@ async function getPackageCollectionByName(packArray) { // which process the returned content with constructPackageObjectShort(), // we select only the needed columns. const command = await sqlStorage` - SELECT DISTINCT ON (p.name) p.name, v.semver, p.downloads, + SELECT DISTINCT ON (p.name) p.name, v.semver, p.downloads, p.owner, (p.stargazers_count + p.original_stargazers) AS stargazers_count, p.data FROM packages AS p INNER JOIN names AS n ON (p.pointer = n.pointer AND n.name IN ${sqlStorage( @@ -1479,7 +1479,7 @@ async function simpleSearch(term, page, dir, sort, themes = false) { const command = await sqlStorage` WITH search_query AS ( - SELECT DISTINCT ON (p.name) p.name, p.data, p.downloads, + SELECT DISTINCT ON (p.name) p.name, p.data, p.downloads, p.owner, (p.stargazers_count + p.original_stargazers) AS stargazers_count, v.semver, p.created, v.updated, p.creation_method FROM packages AS p From 01ef740db2712a0787017402f5d55744a25b3231 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 30 Dec 2023 20:06:00 -0800 Subject: [PATCH 15/15] Make the linter happy --- tests/.eslintrc.js | 5 +++++ tests/http/getPackagesFeatured.test.js | 2 +- tests/http/getThemesSearch.test.js | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 tests/.eslintrc.js diff --git a/tests/.eslintrc.js b/tests/.eslintrc.js new file mode 100644 index 00000000..91b1f75f --- /dev/null +++ b/tests/.eslintrc.js @@ -0,0 +1,5 @@ +module.exports = { + globals: { + afterAll: true + } +}; diff --git a/tests/http/getPackagesFeatured.test.js b/tests/http/getPackagesFeatured.test.js index f835361f..cef86597 100644 --- a/tests/http/getPackagesFeatured.test.js +++ b/tests/http/getPackagesFeatured.test.js @@ -22,7 +22,7 @@ describe("Behaves as expected", () => { }); test("Returns proper data on success", async () => { - const addPack = await database.insertNewPackage({ + await database.insertNewPackage({ // We know a currently featured package is 'x-terminal-reloaded' name: "x-terminal-reloaded", repository: { diff --git a/tests/http/getThemesSearch.test.js b/tests/http/getThemesSearch.test.js index b4b5bd20..fd1395d8 100644 --- a/tests/http/getThemesSearch.test.js +++ b/tests/http/getThemesSearch.test.js @@ -28,7 +28,7 @@ describe("Behaves as expected", () => { }); test("Returns array on success", async () => { - const newPack = await database.insertNewPackage({ + await database.insertNewPackage({ name: "atom-material-syntax", repository: { url: "https://github.com/confused-Techie/package-backend",