Prepare to migrate to the new versioning system - part 2

[Digitalone1]

Requirements

• Have you ran tests against this code?

Description of the Change

@confused-Techie This is the second part of #64. It contains #62 and #65. Main test is failing on GET package endpoint. The weird thing is that database tests are all passing.

Update: issue fixed.

[confused-Techie]

@Digitalone1 I've found the issue that's causing tests to fail.

It's right here (Line 68-73 of utils.js) on the changes to how constructPackageObjectFull functions.

Right here you assign pack.versions as latestVer to newPack but then below you assign newPack.releases.latest to latestVer

Which ends up (Which using language-css as an example giving an object structure like so.)

 <ref *1> {
      meta: {
        name: 'language-css',
        keywords: [ 'tree-sitter' ],
        description: 'CSS Support in Atom',
        license: 'MIT',
        engine: { atom: '*', node: '*' },
        dist: {
          tarball: 'https://api.pulsar-edit.dev/api/packages/language-css/versions/0.46.0/tarball'
        }
      },
      engine: { atom: '*', node: '*' },
      semver: '0.46.0',
      license: 'MIT',
      name: 'language-css',
      downloads: '400004',
      stargazers_count: '77',
      versions: {
        '0.46.0': {
          name: 'language-css',
          keywords: [Array],
          description: 'CSS Support in Atom',
          license: 'MIT',
          engine: [Object],
          dist: [Object]
        },
        '0.45.7': {
          name: 'language-css',
          keywords: [Array],
          description: 'CSS Support in Atom',
          tarball_url: 'https://github.com/pulsar-edit/language-css',
          license: 'MIT',
          engine: [Object],
          dist: [Object]
        },
        '0.45.0': {
          name: 'language-css',
          keywords: [Array],
          description: 'CSS Support in Atom',
          license: 'MIT',
          engine: [Object],
          dist: [Object]
        }
      },
      releases: { latest: [Circular *1] }
    }

Essentially this creates an object where the head reference to the object is the same as the releases.latest. Looks like ExpressJS tries to parse the file when it's handed to res.status(200).json(pack) and presumably continues parsing down the circular tree trying to turn it into JSON, which can't be done with a Circular Reference inside it, so stalls out until stopped.

I can't tell exactly what the intention was here, but changing the way the package is handled in utils.constructObjectFull() should resolve the failing tests.

[Digitalone1]

Thanks @confused-Techie for the tip.

What you said made me think and I started to check other parts of the code. I figured out that in git.createPackage we pass the same pack reference to metadataAppendTarballInfo and this leads to appending the tarball data to the same object, overwriting what was stored before. So we have to copy the pack object with structuredClone before appending the tarball stuff.

The point of these last changes is to deprecate the use of package.data because we already carry the same information in the meta column of the versions table. The column is still there and is set, but for the moment it's not used, nor selected. In the future we will drop it completely.

Using structuredClone also on the latest version metadata is needed to avoid issues with JSON stringify.

All tests are passing. Switch to ready for review.

[confused-Techie]

@Digitalone1 Looks like our CI is failing, I have a feeling this relates to the use of structuredClone.

The CI tests are using the same version we are in prod with is NodeJS 16.x so the CI itself won't support a Node bump just yet.

If you'd like I can go ahead and author something when I get the chance that bumps Node versions and you can update that here if/when merged.

[Digitalone1]

@confused-Techie CI test passed on v18.

v16 not supported unfortunately.

[confused-Techie]

Overall this looks great. The big changes to tests worried me but I see it was just some reorganizing.

There's a lot in this PR that is hard to manually review, but the changes look great, and the tests are all passing.

I think the only thing I'd really love to see (On this PR or another) could be difinitive objects documented somewhere on what some of the DB functions expect. Since they have strayed far away from just expecting a Package Object Full or Package Object Full, so if maybe some JSON files could be added to the docs/resources section that'd be amazing.

Otherwise I think we should be good to merge this one

[Digitalone1]

I think the only thing I'd really love to see (On this PR or another) could be difinitive objects documented somewhere on what some of the DB functions expect. Since they have strayed far away from just expecting a Package Object Full or Package Object Full, so if maybe some JSON files could be added to the docs/resources section that'd be amazing.

Maybe better to take care of this step in the git refactoring.

[confused-Techie]

Yeah good point @Digitalone1 sounds like a plan to me.

In that case let me give your open PR's a second review through and get them merged. Then check of the checklist and get prod updated.