From b69792ae33047bb4cec1eb829fbf1b410a49a687 Mon Sep 17 00:00:00 2001 From: Digitalone Date: Sun, 22 Jan 2023 17:38:58 +0100 Subject: [PATCH 1/6] add deleted bool column to versions table --- scripts/database/create_versions_table.sql | 1 + src/dev-runner/migrations/0001-initial-migration.sql | 1 + 2 files changed, 2 insertions(+) diff --git a/scripts/database/create_versions_table.sql b/scripts/database/create_versions_table.sql index 4ad80bb9..b3e3a8a1 100644 --- a/scripts/database/create_versions_table.sql +++ b/scripts/database/create_versions_table.sql @@ -14,6 +14,7 @@ CREATE TABLE versions ( created TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT CURRENT_TIMESTAMP, updated TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT CURRENT_TIMESTAMP, meta JSONB, + deleted BOOLEAN NOT NULL DEFAULT FALSE, -- generated columns semver_v1 INTEGER GENERATED ALWAYS AS (CAST ((regexp_match(semver, '^(\d+)\.(\d+)\.(\d+)'))[1] AS INTEGER)) STORED, diff --git a/src/dev-runner/migrations/0001-initial-migration.sql b/src/dev-runner/migrations/0001-initial-migration.sql index 8661ec19..04578fe4 100644 --- a/src/dev-runner/migrations/0001-initial-migration.sql +++ b/src/dev-runner/migrations/0001-initial-migration.sql @@ -75,6 +75,7 @@ CREATE TABLE versions ( updated TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT CURRENT_TIMESTAMP, engine JSONB NOT NULL, meta JSONB, + deleted BOOLEAN NOT NULL DEFAULT FALSE, -- generated columns semver_v1 INTEGER GENERATED ALWAYS AS (CAST ((regexp_match(semver, '^(\d+)\.(\d+)\.(\d+)'))[1] AS INTEGER)) STORED, From 0e745a1e8da43e0324c48fd40f83caa0a71aa661 Mon Sep 17 00:00:00 2001 From: Digitalone Date: Sun, 22 Jan 2023 17:47:01 +0100 Subject: [PATCH 2/6] deprecate status enum in versions table --- src/database.js | 207 ++++++++------------------------ src/handlers/package_handler.js | 2 +- src/utils.js | 15 +-- 3 files changed, 53 insertions(+), 171 deletions(-) diff --git a/src/database.js b/src/database.js index cc713e6a..375e6ed6 100644 --- a/src/database.js +++ b/src/database.js @@ -136,6 +136,7 @@ async function insertNewPackage(pack) { let insertNewPack = {}; try { // 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}, ${packData}, ${packageType}) @@ -175,8 +176,9 @@ async function insertNewPackage(pack) { // Populate versions table let versionCount = 0; const pv = pack.versions; + // TODO: status column deprecated; to be removed. + const status = "published"; for (const ver of Object.keys(pv)) { - const status = ver === latest ? "latest" : "published"; // Since many packages don't define an engine field, // we will do it for them if not present, @@ -293,59 +295,22 @@ async function insertNewPackageVersion(packJSON, packageData, oldName = null) { packName = packJSON.name; } - // Here we need to check if the current latest version is lower than the new one - // which we want to publish. - const latestVersion = await sqlTrans` - SELECT id, status, semver - FROM versions - WHERE package = ${pointer} AND status = 'latest'; - `; - - if (latestVersion.count === 0) { - throw `There is no current latest version for ${packName}. The package is broken.`; - } - - const higherSemver = utils.semverGt( - utils.semverArray(packJSON.version), - utils.semverArray(latestVersion[0].semver) - ); - if (!higherSemver) { - throw `Cannot publish a new version with semver lower or equal than the current latest one.`; - } - - // The new version can be published. First switch the current "latest" to "published". - const updateLastVer = await sqlTrans` - UPDATE versions - SET status = 'published' - WHERE id = ${latestVersion[0].id} - RETURNING semver, status; - `; - - if (updateLastVer.count === 0) { - throw `Unable to modify last published version ${packName}`; - } - - // Then update the package object in the related column of packages table. - const updatePackageData = await sqlTrans` - UPDATE packages - SET data = ${packageData} - WHERE pointer = ${pointer} - RETURNING name; - `; + // We used to check if the new version was higher than the latest, but this is + // too cumbersome to do and the publisher has the responsibility to push an + // higher version to be signaled in Pulsar for the update, so we just try to + // insert whatever we got. + // The only requirement is that the provided semver is not already present + // in the database for the targeted package. - if (updatePackageData.count === 0) { - throw `Unable to update the full package object of the new version ${packName}`; - } - - // We can finally insert the new latest version. const license = packJSON.license ?? defaultLicense; const engine = packJSON.engines ?? defaultEngine; let addVer = {}; try { + // TODO: status column deprecated; to be removed addVer = await sqlTrans` INSERT INTO versions (package, status, semver, license, engine, meta) - VALUES(${pointer}, 'latest', ${packJSON.version}, ${license}, ${engine}, ${packJSON}) + VALUES(${pointer}, 'published', ${packJSON.version}, ${license}, ${engine}, ${packJSON}) RETURNING semver, status; `; } catch (e) { @@ -357,6 +322,19 @@ async function insertNewPackageVersion(packJSON, packageData, oldName = null) { throw `Unable to create a new version for ${packName}`; } + // Then update the package object in the related column of packages table. + // TODO: This is deprecated and will removed in the future. + const updatePackageData = await sqlTrans` + UPDATE packages + SET data = ${packageData} + WHERE pointer = ${pointer} + RETURNING name; + `; + + if (updatePackageData.count === 0) { + throw `Unable to update the full package object of the new version ${packName}`; + } + return { ok: true, content: `Successfully added new version: ${packName}@${packJSON.version}`, @@ -518,14 +496,13 @@ async function getPackageByName(name, user = false) { user ? sqlStorage`` : sqlStorage`'id', v.id, 'package', v.package,` - } 'status', v.status, 'semver', v.semver, - 'license', v.license, 'engine', v.engine, 'meta', v.meta + } 'semver', v.semver, 'license', v.license, 'engine', v.engine, 'meta', v.meta ) ORDER BY v.semver_v1 DESC, v.semver_v2 DESC, v.semver_v3 DESC, v.created DESC ) AS versions FROM packages p INNER JOIN names n ON (p.pointer = n.pointer AND n.name = ${name}) - INNER JOIN versions v ON (p.pointer = v.package AND v.status != 'removed') + INNER JOIN versions v ON (p.pointer = v.package AND v.deleted IS FALSE) GROUP BY p.pointer; `; @@ -593,10 +570,10 @@ async function getPackageVersionByNameAndVersion(name, version) { sqlStorage ??= setupSQL(); const command = await sqlStorage` - SELECT v.semver, v.status, v.license, v.engine, v.meta + SELECT v.semver, v.license, v.engine, v.meta FROM packages p INNER JOIN names n ON (p.pointer = n.pointer AND n.name = ${name}) - INNER JOIN versions v ON (p.pointer = v.package AND v.semver = ${version} AND v.status != 'removed'); + INNER JOIN versions v ON (p.pointer = v.package AND v.semver = ${version} AND v.deleted IS FALSE); `; return command.count !== 0 @@ -638,7 +615,7 @@ async function getPackageCollectionByName(packArray) { INNER JOIN names n ON (p.pointer = n.pointer AND n.name IN ${sqlStorage( packArray )}) - INNER JOIN versions v ON (p.pointer = v.package AND v.status != 'removed') + INNER JOIN versions v ON (p.pointer = v.package AND v.deleted IS FALSE) ORDER BY p.name, v.semver_v1 DESC, v.semver_v2 DESC, v.semver_v3 DESC, v.created DESC; `; @@ -667,10 +644,11 @@ async function getPackageCollectionByID(packArray) { sqlStorage ??= setupSQL(); const command = await sqlStorage` - SELECT data - FROM packages AS p - INNER JOIN versions AS v ON (p.pointer = v.package AND v.status = 'latest') + SELECT DISTINCT ON (p.name) p.name, v.meta + FROM packages p + INNER JOIN versions AS v ON (p.pointer = v.package AND v.deleted IS FALSE) WHERE pointer IN ${sqlStorage(packArray)} + ORDER BY p.name, v.semver_v1 DESC, v.semver_v2 DESC, v.semver_v3 DESC, v.created DESC; `; return command.count !== 0 @@ -901,8 +879,8 @@ async function removePackageByName(name) { /** * @async * @function removePackageVersion - * @description Mark a version of a specific package as removed. This does not delete the record, - * just mark the status as removed, but only if one published version remain available. + * @description Mark a version of a specific package as deleted. This does not delete the record, + * just mark the boolean deleted flag as true, but only if one published version remains available. * This also makes sure that a new latest version is selected in case the previous one is removed. * @param {string} packName - The package name. * @param {string} semVer - The version to remove. @@ -927,57 +905,27 @@ async function removePackageVersion(packName, semVer) { const pointer = packID.content.pointer; - // Retrieve all non-removed versions sorted from latest to older + // Retrieve all non-removed versions to count them const getVersions = await sqlTrans` - SELECT id, semver, status + SELECT id FROM versions - WHERE package = ${pointer} AND status != 'removed' - ORDER BY semver_v1 DESC, semver_v2 DESC, semver_v3 DESC, created DESC; + WHERE package = ${pointer} AND deleted IS FALSE; `; const versionCount = getVersions.count; - if (versionCount === 0) { - throw `No published version available for ${packName} package`; - } - - // Having all versions, we loop them to find: - // - the id of the version to remove - // - if its status is "latest" - let removeLatest = false; - let versionId = null; - for (const v of getVersions) { - if (v.semver === semVer) { - versionId = v.id; - removeLatest = v.status === "latest"; - break; - } - } - - if (versionId === null) { - // Do not use throw here because we specify Not Found reason. - return { - ok: false, - content: `There's no version ${semVer} to remove for ${packName} package`, - short: "Not Found", - }; - } - - // We have the version to remove, but for the package integrity we have to make sure that - // at least one published version is still available after the removal. - // This is not possible if the version count is only 1. - if (versionCount === 1) { - throw `It's not possible to leave the ${packName} without at least one published version`; + if (versionCount < 2) { + throw `${packName} package has less than 2 published versions: deletion not allowed.`; } - // The package will have published versions, so we can remove the targeted semVer. - const updateRemovedStatus = await sqlTrans` + // We can remove the targeted semVer. + const markDeletedVersion = await sqlTrans` UPDATE versions - SET status = 'removed' - WHERE id = ${versionId} + SET DELETED = TRUE + WHERE package = ${pointer} AND semver = ${semVer} RETURNING id; `; - if (updateRemovedStatus.count === 0) { + if (markDeletedVersion.count === 0) { // Do not use throw here because we specify Not Found reason. return { ok: false, @@ -986,68 +934,9 @@ async function removePackageVersion(packName, semVer) { }; } - if (!removeLatest) { - // We have removed a published versions and the latest one is still available. - return { - ok: true, - content: `Successfully removed ${semVer} version of ${packName} package.`, - }; - } - - // We have removed the version with the "latest" status, so now we have to select - // a new version between the remaining ones which will obtain "latest" status. - // No need to compare versions here. We have an array ordered from latest to older, - // just pick the first one not equal to semVer - let latestVersionId = null; - let latestSemver = null; - for (const v of getVersions) { - if (v.id === versionId) { - // Skip the removed version - continue; - } - latestVersionId = v.id; - latestSemver = v.semver; - } - - if (latestVersionId === null) { - throw `An error occurred while selecting the highest versions of ${packName}`; - } - - // Mark the targeted highest version as latest. - const commandLatest = await sqlTrans` - UPDATE versions - SET status = 'latest' - WHERE id = ${latestVersionId} - RETURNING id, meta; - `; - - if (commandLatest.count === 0) { - return { - ok: false, - content: `Unable to remove ${semVer} version of ${packName} package.`, - short: "Not Found", - }; - } - - // Let's save the meta data object of the new latest version so - // we can update it inside the packages table since we use the - // packages.data column to report the full object of the lastest version. - const latestDataObject = commandLatest[0].meta; - - const updatePackageData = await sqlTrans` - UPDATE packages - SET data = ${latestDataObject} - WHERE pointer = ${pointer} - RETURNING name; - `; - - if (updatePackageData.count === 0) { - throw `Unable to update the full package object of the new version ${latestSemver}`; - } - return { ok: true, - content: `Removed ${semVer} of ${packName} and ${latestSemver} is the new latest version.`, + content: `Successfully removed ${semVer} version of ${packName} package.`, }; }) .catch((err) => { @@ -1485,7 +1374,7 @@ async function simpleSearch(term, page, dir, sort, themes = false) { ? sqlStorage`AND p.package_type = 'theme'` : sqlStorage`` }) - INNER JOIN versions AS v ON (p.pointer = v.package AND v.status != 'removed') + INNER JOIN versions AS v ON (p.pointer = v.package AND v.deleted IS FALSE) 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 @@ -1597,7 +1486,7 @@ async function getSortedPackages(page, dir, method, themes = false) { (p.stargazers_count + p.original_stargazers) AS stargazers_count, v.semver, p.created, v.updated FROM packages p - INNER JOIN versions AS v ON (p.pointer = v.package AND v.status != 'removed' + INNER JOIN versions AS v ON (p.pointer = v.package AND v.deleted IS FALSE ${ themes === true ? sqlStorage`AND p.package_type = 'theme'` diff --git a/src/handlers/package_handler.js b/src/handlers/package_handler.js index d5feaf89..97c56cc4 100644 --- a/src/handlers/package_handler.js +++ b/src/handlers/package_handler.js @@ -866,7 +866,7 @@ async function getPackagesVersionTarball(req, res) { // the download to take place from their servers. // But right before, lets do a couple simple checks to make sure we are sending to a legit site. - const tarballURL = pack.content.meta.tarball_url ?? ""; + const tarballURL = pack.content.meta?.tarball_url ?? ""; let hostname = ""; // Try to extract the hostname diff --git a/src/utils.js b/src/utils.js index 95d9b3ce..d3d7586b 100644 --- a/src/utils.js +++ b/src/utils.js @@ -61,23 +61,16 @@ async function constructPackageObjectFull(pack) { return retVer; }; - const findLatestVersion = (vers) => { - for (const v of vers) { - if (v.status === "latest") { - return v.semver; - } - } - return null; - }; + // database.getPackageByName() sorts the JSON array versions in descending order, + // so no need to find the latest, it's the first one. + const latestVer = pack.versions.length !== 0 ? pack.versions[0] : []; let newPack = pack.data; newPack.name = pack.name; newPack.downloads = pack.downloads; newPack.stargazers_count = pack.stargazers_count; newPack.versions = parseVersions(pack.versions); - newPack.releases = { - latest: findLatestVersion(pack.versions), - }; + newPack.releases = { latest: latestVer }; logger.generic(6, "Built Package Object Full without Error"); From c9acfa4b36b678da2e6f32acea4360ad83c40574 Mon Sep 17 00:00:00 2001 From: Digitalone Date: Sun, 22 Jan 2023 23:29:23 +0100 Subject: [PATCH 3/6] prepare to deprecate package.data in favour of versions.meta --- src/database.js | 32 +++++------------- src/git.js | 10 ++++-- src/handlers/package_handler.js | 39 +++++++++++++++++++--- src/tests_integration/database.test.js | 46 ++++++++++---------------- src/utils.js | 6 ++-- 5 files changed, 71 insertions(+), 62 deletions(-) diff --git a/src/database.js b/src/database.js index 375e6ed6..50a7a69f 100644 --- a/src/database.js +++ b/src/database.js @@ -232,7 +232,7 @@ async function insertNewPackage(pack) { * @param {string|null} oldName - If provided, the old name to be replaced for the renaming of the package. * @returns {object} A server status object. */ -async function insertNewPackageVersion(packJSON, packageData, oldName = null) { +async function insertNewPackageVersion(packJSON, oldName = null) { sqlStorage ??= setupSQL(); // We are expected to receive a standard `package.json` file. @@ -322,19 +322,6 @@ async function insertNewPackageVersion(packJSON, packageData, oldName = null) { throw `Unable to create a new version for ${packName}`; } - // Then update the package object in the related column of packages table. - // TODO: This is deprecated and will removed in the future. - const updatePackageData = await sqlTrans` - UPDATE packages - SET data = ${packageData} - WHERE pointer = ${pointer} - RETURNING name; - `; - - if (updatePackageData.count === 0) { - throw `Unable to update the full package object of the new version ${packName}`; - } - return { ok: true, content: `Successfully added new version: ${packName}@${packJSON.version}`, @@ -489,7 +476,7 @@ async function getPackageByName(name, user = false) { ${ user ? sqlStorage`` : sqlStorage`p.pointer,` } p.name, p.created, p.updated, p.creation_method, p.downloads, - (p.stargazers_count + p.original_stargazers) AS stargazers_count, p.data, + (p.stargazers_count + p.original_stargazers) AS stargazers_count, JSONB_AGG( JSON_BUILD_OBJECT( ${ @@ -610,7 +597,7 @@ async function getPackageCollectionByName(packArray) { // we select only the needed columns. const command = await sqlStorage` SELECT DISTINCT ON (p.name) p.name, v.semver, p.downloads, - (p.stargazers_count + p.original_stargazers) AS stargazers_count, p.data + (p.stargazers_count + p.original_stargazers) AS stargazers_count, v.meta AS data FROM packages p INNER JOIN names n ON (p.pointer = n.pointer AND n.name IN ${sqlStorage( packArray @@ -667,7 +654,7 @@ async function getPackageCollectionByID(packArray) { /** * @async * @function updatePackageStargazers - * @description Uses the package name (or pointer if provided) to update its stargazers count. + * @description Internal util that uses the package name (or pointer if provided) to update its stargazers count. * @param {string} name - The package name. * @param {string} pointer - The package id (if given, the search by name is skipped). * @returns {object} The effected server status object. @@ -698,7 +685,7 @@ async function updatePackageStargazers(name, pointer = null) { UPDATE packages SET stargazers_count = ${starCount} WHERE pointer = ${pointer} - RETURNING name, downloads, (stargazers_count + original_stargazers) AS stargazers_count, data; + RETURNING name, (stargazers_count + original_stargazers) AS stargazers_count; `; return updateStar.count !== 0 @@ -734,7 +721,7 @@ async function updatePackageIncrementDownloadByName(name) { SET downloads = p.downloads + 1 FROM names n WHERE n.pointer = p.pointer AND n.name = ${name} - RETURNING p.name, p.downloads, p.data; + RETURNING p.name, p.downloads; `; return command.count !== 0 @@ -770,7 +757,7 @@ async function updatePackageDecrementDownloadByName(name) { SET downloads = GREATEST(p.downloads - 1, 0) FROM names n WHERE n.pointer = p.pointer AND n.name = ${name} - RETURNING p.name, p.downloads, p.data; + RETURNING p.name, p.downloads; `; return command.count !== 0 @@ -1362,7 +1349,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, v.meta AS data, p.downloads, (p.stargazers_count + p.original_stargazers) AS stargazers_count, v.semver, p.created, v.updated FROM packages p @@ -1482,7 +1469,7 @@ async function getSortedPackages(page, dir, method, 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, v.meta AS data, p.downloads, (p.stargazers_count + p.original_stargazers) AS stargazers_count, v.semver, p.created, v.updated FROM packages p @@ -1668,7 +1655,6 @@ module.exports = { getPackageVersionByNameAndVersion, updatePackageIncrementDownloadByName, updatePackageDecrementDownloadByName, - updatePackageStargazers, getFeaturedThemes, simpleSearch, updateIncrementStar, diff --git a/src/git.js b/src/git.js index 00d3a007..3c571655 100644 --- a/src/git.js +++ b/src/git.js @@ -229,7 +229,8 @@ async function createPackage(repo, user) { // Currently there is no purpose to store the type of repo. But for the time being, // we will assume this could be used in the future as a way to determine how to interact with a repo. - newPack.repository = selectPackageRepository(pack.repository); + const packRepo = selectPackageRepository(pack.repository); + newPack.repository = packRepo; // Now during migration packages will have a 'versions' key, but otherwise the standard // package will just have a 'version'. @@ -276,7 +277,12 @@ async function createPackage(repo, user) { continue; } - newPack.versions[ver] = versionMetadata; + newPack.versions[ver] = { + name: packName, + repository: packRepo, + readme: readme, + metadata: versionMetadata, + }; versionCount++; // Check latest version. diff --git a/src/handlers/package_handler.js b/src/handlers/package_handler.js index 97c56cc4..aa6ee340 100644 --- a/src/handlers/package_handler.js +++ b/src/handlers/package_handler.js @@ -421,9 +421,19 @@ async function deletePackagesName(req, res) { return; } + const packMetadata = packageExists.content?.versions[0]?.meta; + + if (packMetadata === null) { + await common.handleError(req, res, { + ok: false, + short: "Not Found", + content: `Cannot retrieve metadata for ${params.packageName} package`, + }); + } + const gitowner = await git.ownership( user.content, - utils.getOwnerRepoFromPackage(packageExists.content.data) + utils.getOwnerRepoFromPackage(packMetadata), ); if (!gitowner.ok) { @@ -626,8 +636,18 @@ async function postPackagesVersion(req, res) { return; } + const meta = packExists.content?.versions[0]?.meta; + + if (meta === null) { + await common.handleError(req, res, { + ok: false, + short: "Not Found", + content: `Cannot retrieve metadata for ${params.packageName} package`, + }); + } + // Get `owner/repo` string format from package. - let ownerRepo = utils.getOwnerRepoFromPackage(packExists.content.data); + let ownerRepo = utils.getOwnerRepoFromPackage(meta); // Now it's important to note, that getPackageJSON was intended to be an internal function. // As such does not return a Server Status Object. This may change later, but for now, @@ -683,7 +703,7 @@ async function postPackagesVersion(req, res) { metadata: packMetadata, }; - const newName = packMetadata.name; + const newName = packageData.name; const currentName = packExists.content.name; if (newName !== currentName && !params.rename) { logger.generic( @@ -755,7 +775,6 @@ async function postPackagesVersion(req, res) { // Now add the new Version key. const addVer = await database.insertNewPackageVersion( - packMetadata, packageData, rename ? currentName : null ); @@ -952,9 +971,19 @@ async function deletePackageVersion(req, res) { return; } + const packMetadata = packageExists.content?.versions[0]?.meta; + + if (packMetadata === null) { + await common.handleError(req, res, { + ok: false, + short: "Not Found", + content: `Cannot retrieve metadata for ${params.packageName} package`, + }); + } + const gitowner = await git.ownership( user.content, - utils.getOwnerRepoFromPackage(packageExists.content.data) + utils.getOwnerRepoFromPackage(packMetadata) ); if (!gitowner.ok) { diff --git a/src/tests_integration/database.test.js b/src/tests_integration/database.test.js index 1dc7aef2..4c2df4bd 100644 --- a/src/tests_integration/database.test.js +++ b/src/tests_integration/database.test.js @@ -92,7 +92,7 @@ describe("Package Lifecycle Tests", () => { const pack = require("./fixtures/lifetime/package-a.js"); // === Is the package name available? - const nameIsAvailable = await database.packageNameAvailability("package-a-lifetime"); + const nameIsAvailable = await database.packageNameAvailability(pack.createPack.name); expect(nameIsAvailable.ok).toBeTruthy(); // === Let's publish our package @@ -117,19 +117,10 @@ describe("Package Lifecycle Tests", () => { expect(getAfterPublish.content.downloads).toEqual("0"); // Original stargazers already added to stargazers count expect(getAfterPublish.content.stargazers_count).toEqual("0"); - expect(getAfterPublish.content.data.name).toEqual(pack.createPack.name); - expect(getAfterPublish.content.data.readme).toEqual(pack.createPack.readme); - expect(getAfterPublish.content.data.repository).toEqual( - pack.createPack.repository - ); - expect(getAfterPublish.content.data.metadata).toEqual( - pack.createPack.metadata - ); expect(getAfterPublish.content.versions.length).toEqual(1); // Only 1 ver was provided expect(getAfterPublish.content.versions[0].semver).toEqual( pack.createPack.metadata.version ); - expect(getAfterPublish.content.versions[0].status).toEqual("latest"); expect(getAfterPublish.content.versions[0].license).toEqual("NONE"); expect(getAfterPublish.content.versions[0].package).toBeDefined(); @@ -138,7 +129,7 @@ describe("Package Lifecycle Tests", () => { expect(dupPublish.ok).toBeFalsy(); // === Let's rename our package - const NEW_NAME = "package-a-lifetime-rename"; + const NEW_NAME = `${pack.createPack.name}-rename`; const newName = await database.insertNewPackageName( NEW_NAME, pack.createPack.name @@ -201,7 +192,7 @@ describe("Package Lifecycle Tests", () => { ); expect(removeOnlyVersion.ok).toBeFalsy(); expect(removeOnlyVersion.content).toEqual( - `It's not possible to leave the ${NEW_NAME} without at least one published version` + `${NEW_NAME} package has less than 2 published versions: deletion not allowed.` ); // === Now let's add a version @@ -225,7 +216,6 @@ describe("Package Lifecycle Tests", () => { expect(getAfterVer.ok).toBeTruthy(); expect(getAfterVer.content.versions.length).toEqual(2); expect(getAfterVer.content.versions[0].semver).toEqual(v1_0_1.version); - expect(getAfterVer.content.versions[0].status).toEqual("latest"); expect(getAfterVer.content.versions[0].license).toEqual(v1_0_1.license); expect(getAfterVer.content.versions[0].meta.name).toEqual(v1_0_1.name); expect(getAfterVer.content.versions[0].meta.version).toEqual( @@ -242,7 +232,7 @@ describe("Package Lifecycle Tests", () => { ); expect(dupVer.ok).toBeFalsy(); expect(dupVer.content).toEqual( - "Cannot publish a new version with semver lower or equal than the current latest one." + `Not allowed to publish a version already present for ${pack.createPack.name}` ); // === Can we get this specific version with the new name @@ -251,7 +241,6 @@ describe("Package Lifecycle Tests", () => { v1_0_1.version ); expect(getNewVerOnly.ok).toBeTruthy(); - expect(getNewVerOnly.content.status).toEqual("latest"); expect(getNewVerOnly.content.semver).toEqual(v1_0_1.version); expect(getNewVerOnly.content.meta.name).toEqual(pack.createPack.name); @@ -261,7 +250,6 @@ describe("Package Lifecycle Tests", () => { pack.createPack.metadata.version ); expect(getOldVerOnly.ok).toBeTruthy(); - expect(getOldVerOnly.content.status).toEqual("published"); expect(getOldVerOnly.content.semver).toEqual( pack.createPack.metadata.version ); @@ -299,6 +287,17 @@ describe("Package Lifecycle Tests", () => { expect(downPackOld.content.name).toEqual(NEW_NAME); expect(downPackOld.content.downloads).toEqual("1"); + // === Can we remove a non-existing version? + const noPubVer = "3.3.3"; + const removeNonExistingVersion = await database.removePackageVersion( + NEW_NAME, + noPubVer + ); + expect(removeNonExistingVersion.ok).toBeFalsy(); + expect(removeNonExistingVersion.content).toEqual( + `Unable to remove ${noPubVer} version of ${NEW_NAME} package.` + ); + // === Can we delete our newest version? // === Here we append an extension to test if the version is selected in the same way. const delLatestVer = await database.removePackageVersion( @@ -307,7 +306,7 @@ describe("Package Lifecycle Tests", () => { ); expect(delLatestVer.ok).toBeTruthy(); expect(delLatestVer.content).toEqual( - `Removed ${v1_0_1.version} of ${NEW_NAME} and ${pack.createPack.metadata.version} is the new latest version.` + `Successfully removed ${v1_0_1.version} version of ${NEW_NAME} package.` ); // === Is our old version the latest again? @@ -318,7 +317,6 @@ describe("Package Lifecycle Tests", () => { expect(newLatestVer.content.versions[0].semver).toEqual( pack.createPack.metadata.version ); - expect(newLatestVer.content.versions[0].status).toEqual("latest"); // === Can we reinsert a previous deleted version? // This is intentionally unsupported because we want a new package to be always @@ -355,16 +353,6 @@ describe("Package Lifecycle Tests", () => { `Successfully removed ${pack.createPack.metadata.version} version of ${NEW_NAME} package.` ); - // === Can we remove a non-existing version? - const removeNonExistingVersion = await database.removePackageVersion( - NEW_NAME, - pack.createPack.metadata.version - ); - expect(removeNonExistingVersion.ok).toBeFalsy(); - expect(removeNonExistingVersion.content).toEqual( - `There's no version ${pack.createPack.metadata.version} to remove for ${NEW_NAME} package` - ); - // === Can we add an odd yet valid semver? const oddVer = pack.addVersion("1.2.3-beta.0"); const oddNewVer = await database.insertNewPackageVersion( @@ -400,7 +388,7 @@ describe("Package Lifecycle Tests", () => { expect(ghostPack.short).toEqual("Not Found"); // === Is the name of the deleted package available? - const deletedNameAvailable = await database.packageNameAvailability("package-a-lifetime"); + const deletedNameAvailable = await database.packageNameAvailability(pack.createPack.name); expect(deletedNameAvailable.ok).toBeFalsy(); }); test("User A Lifecycle Test", async () => { diff --git a/src/utils.js b/src/utils.js index d3d7586b..93cd795a 100644 --- a/src/utils.js +++ b/src/utils.js @@ -62,10 +62,10 @@ async function constructPackageObjectFull(pack) { }; // database.getPackageByName() sorts the JSON array versions in descending order, - // so no need to find the latest, it's the first one. - const latestVer = pack.versions.length !== 0 ? pack.versions[0] : []; + // so no need to find the latest metadata, it's the first one. + const latestVer = pack.versions.length !== 0 ? pack.versions[0] : {}; - let newPack = pack.data; + let newPack = latestVer; newPack.name = pack.name; newPack.downloads = pack.downloads; newPack.stargazers_count = pack.stargazers_count; From d67bedc8543704cf95e6512434d4eb2dcbc39e85 Mon Sep 17 00:00:00 2001 From: Digitalone Date: Tue, 24 Jan 2023 14:17:30 +0100 Subject: [PATCH 4/6] deprecate package.data + use structuredClone and fix tests --- src/database.js | 45 ++++++++++++++++++++------------------------- src/git.js | 7 ++++++- src/utils.js | 12 ++++++------ 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/database.js b/src/database.js index 50a7a69f..eb5f56ba 100644 --- a/src/database.js +++ b/src/database.js @@ -119,12 +119,6 @@ async function insertNewPackage(pack) { // All data is committed into the database only if no errors occur. return await sqlStorage .begin(async (sqlTrans) => { - const packData = { - name: pack.name, - repository: pack.repository, - readme: pack.readme, - metadata: pack.metadata, - }; const packageType = typeof pack.metadata.themes === "string" && pack.metadata.themes.match(/^(?:themes|ui)$/i) !== null @@ -139,7 +133,7 @@ async function insertNewPackage(pack) { // 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}, ${packData}, ${packageType}) + VALUES (${pack.name}, ${pack.creation_method}, ${pack}, ${packageType}) RETURNING pointer; `; } catch (e) { @@ -487,9 +481,9 @@ async function getPackageByName(name, user = false) { ) ORDER BY v.semver_v1 DESC, v.semver_v2 DESC, v.semver_v3 DESC, v.created DESC ) AS versions - FROM packages p - INNER JOIN names n ON (p.pointer = n.pointer AND n.name = ${name}) - INNER JOIN versions v ON (p.pointer = v.package AND v.deleted IS FALSE) + FROM packages AS p + INNER JOIN names AS n ON (p.pointer = n.pointer AND n.name = ${name}) + INNER JOIN versions AS v ON (p.pointer = v.package AND v.deleted IS FALSE) GROUP BY p.pointer; `; @@ -558,9 +552,9 @@ async function getPackageVersionByNameAndVersion(name, version) { const command = await sqlStorage` SELECT v.semver, v.license, v.engine, v.meta - FROM packages p - INNER JOIN names n ON (p.pointer = n.pointer AND n.name = ${name}) - INNER JOIN versions v ON (p.pointer = v.package AND v.semver = ${version} AND v.deleted IS FALSE); + FROM packages AS p + INNER JOIN names AS n ON (p.pointer = n.pointer AND n.name = ${name}) + INNER JOIN versions AS v ON (p.pointer = v.package AND v.semver = ${version} AND v.deleted IS FALSE); `; return command.count !== 0 @@ -598,11 +592,11 @@ async function getPackageCollectionByName(packArray) { const command = await sqlStorage` SELECT DISTINCT ON (p.name) p.name, v.semver, p.downloads, (p.stargazers_count + p.original_stargazers) AS stargazers_count, v.meta AS data - FROM packages p - INNER JOIN names n ON (p.pointer = n.pointer AND n.name IN ${sqlStorage( + FROM packages AS p + INNER JOIN names AS n ON (p.pointer = n.pointer AND n.name IN ${sqlStorage( packArray )}) - INNER JOIN versions v ON (p.pointer = v.package AND v.deleted IS FALSE) + INNER JOIN versions AS v ON (p.pointer = v.package AND v.deleted IS FALSE) ORDER BY p.name, v.semver_v1 DESC, v.semver_v2 DESC, v.semver_v3 DESC, v.created DESC; `; @@ -631,8 +625,9 @@ async function getPackageCollectionByID(packArray) { sqlStorage ??= setupSQL(); const command = await sqlStorage` - SELECT DISTINCT ON (p.name) p.name, v.meta - FROM packages p + SELECT DISTINCT ON (p.name) p.name, v.semver, p.downloads, + (p.stargazers_count + p.original_stargazers) AS stargazers_count, v.meta AS data + FROM packages AS p INNER JOIN versions AS v ON (p.pointer = v.package AND v.deleted IS FALSE) WHERE pointer IN ${sqlStorage(packArray)} ORDER BY p.name, v.semver_v1 DESC, v.semver_v2 DESC, v.semver_v3 DESC, v.created DESC; @@ -717,9 +712,9 @@ async function updatePackageIncrementDownloadByName(name) { sqlStorage ??= setupSQL(); const command = await sqlStorage` - UPDATE packages p + UPDATE packages AS p SET downloads = p.downloads + 1 - FROM names n + FROM names AS n WHERE n.pointer = p.pointer AND n.name = ${name} RETURNING p.name, p.downloads; `; @@ -753,9 +748,9 @@ async function updatePackageDecrementDownloadByName(name) { sqlStorage ??= setupSQL(); const command = await sqlStorage` - UPDATE packages p + UPDATE packages AS p SET downloads = GREATEST(p.downloads - 1, 0) - FROM names n + FROM names AS n WHERE n.pointer = p.pointer AND n.name = ${name} RETURNING p.name, p.downloads; `; @@ -1352,8 +1347,8 @@ async function simpleSearch(term, page, dir, sort, themes = false) { SELECT DISTINCT ON (p.name) p.name, v.meta AS data, p.downloads, (p.stargazers_count + p.original_stargazers) AS stargazers_count, v.semver, p.created, v.updated - FROM packages p - INNER JOIN names n ON (p.pointer = n.pointer AND n.name LIKE ${ + FROM packages AS p + INNER JOIN names AS n ON (p.pointer = n.pointer AND n.name LIKE ${ "%" + lcterm + "%" } ${ @@ -1472,7 +1467,7 @@ async function getSortedPackages(page, dir, method, themes = false) { SELECT DISTINCT ON (p.name) p.name, v.meta AS data, p.downloads, (p.stargazers_count + p.original_stargazers) AS stargazers_count, v.semver, p.created, v.updated - FROM packages p + FROM packages AS p INNER JOIN versions AS v ON (p.pointer = v.package AND v.deleted IS FALSE ${ themes === true diff --git a/src/git.js b/src/git.js index 3c571655..c7375798 100644 --- a/src/git.js +++ b/src/git.js @@ -261,7 +261,12 @@ async function createPackage(repo, user) { } // They match tag and version, stuff the data into the package. - const versionMetadata = await metadataAppendTarballInfo(pack, tag, user); + // Copy pack so we avoid to append tarball info to the same object + const versionMetadata = await metadataAppendTarballInfo( + structuredClone(pack), + tag, + user + ); // TODO:: // Its worthy to note that the function above assigns the current package.json file within the repo // as the version tag. Now this in most cases during a publish should be fine. diff --git a/src/utils.js b/src/utils.js index 93cd795a..d9e19115 100644 --- a/src/utils.js +++ b/src/utils.js @@ -61,16 +61,16 @@ async function constructPackageObjectFull(pack) { return retVer; }; - // database.getPackageByName() sorts the JSON array versions in descending order, - // so no need to find the latest metadata, it's the first one. - const latestVer = pack.versions.length !== 0 ? pack.versions[0] : {}; - - let newPack = latestVer; + // We need to copy the metadata of the latest version in order to avoid an + // auto-reference in the versions array that leads to a freeze in JSON stringify stage. + let newPack = structuredClone(pack?.versions[0]?.meta ?? {}); newPack.name = pack.name; newPack.downloads = pack.downloads; newPack.stargazers_count = pack.stargazers_count; newPack.versions = parseVersions(pack.versions); - newPack.releases = { latest: latestVer }; + // database.getPackageByName() sorts the JSON array versions in descending order, + // so no need to find the latest semver, it's the first one (index 0). + newPack.releases = { latest: pack?.versions[0]?.semver ?? "" }; logger.generic(6, "Built Package Object Full without Error"); From c89e0d96d8d2d696b7cc753e60920c0c55e58560 Mon Sep 17 00:00:00 2001 From: Digitalone Date: Tue, 24 Jan 2023 14:54:52 +0100 Subject: [PATCH 5/6] remove unused const as suggested by CodeQL --- src/database.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/database.js b/src/database.js index eb5f56ba..4e9106d8 100644 --- a/src/database.js +++ b/src/database.js @@ -7,7 +7,6 @@ const fs = require("fs"); const postgres = require("postgres"); const storage = require("./storage.js"); -const utils = require("./utils.js"); const logger = require("./logger.js"); const { DB_HOST, @@ -163,10 +162,6 @@ async function insertNewPackage(pack) { throw `Cannot insert ${pack.name} in names table`; } - // git.createPackage() executed before this function ensures - // the latest version is correctly selected. - const latest = pack.releases.latest; - // Populate versions table let versionCount = 0; const pv = pack.versions; From 341464031fced6c1d1681fa5d29fdec161ba3bcf Mon Sep 17 00:00:00 2001 From: Digitalone Date: Wed, 25 Jan 2023 22:32:21 +0100 Subject: [PATCH 6/6] struturedClone not supported for NodeJS v16 --- .github/workflows/ci-tests-pull.yml | 2 +- .github/workflows/ci-tests.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci-tests-pull.yml b/.github/workflows/ci-tests-pull.yml index 1588f083..bbd8b464 100644 --- a/.github/workflows/ci-tests-pull.yml +++ b/.github/workflows/ci-tests-pull.yml @@ -39,7 +39,7 @@ jobs: strategy: matrix: - node-version: [16.x, 18.x] + node-version: [18.x] steps: - name: Checkout the latest code diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index e9b392df..ecb26d1c 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -42,7 +42,7 @@ jobs: strategy: matrix: - node-version: [16.x, 18.x] + node-version: [18.x] steps: - name: Checkout the latest code