Laying the groundwork for the git refactor

[confused-Techie]

Requirements

• Filling out the template is required.

• All new code requires tests to ensure against regressions.

  • However, if your PR contains zero code changes, feel free to select the checkmark below to indicate so.

• Have you ran tests against this code?

• This PR contains zero code changes.

Description of the Change

Alright, to get some things out of the way.

This PR is essentially a Minimum Viable Product. There are still many things that we should to to improve this, as well as many changes from #74 that I would still like to implement via Cherry-Picks.

But this PR has now been up for quite some time, and has seen much much activity. I think it's a good time to get these changes in and work with smaller more manageable changes for other needed improvements.

So while there are still things that we need and want to do, this should fully function as of now, even if not perfect.

With that out of the way onto the actual PR description:

---

This PR refactors our GitHub interaction. Namely by removing git.js entirely. This adds a VCS Service system, that will ideally allow us to seamlessly support as many Git providers as we want in the future with a few small hurdles to overcome to do so.

This PR also resolves the complexity that arose out of having to handle internal functions to git.js within package_handler.js to accomplish everything we needed to.

Additionally this PR covers the host of issues we have seen while creating our interactions with the package, and as seen on #78 does resolve some issues actively affecting package maintainers.

Which as a note, as far as I can tell currently no package maintainers are able to publish new changes to the backend. So this should be merged as soon as we can.

Otherwise this PR does increase the complexity within our testing framework quite a bit, but only because it is supporting a new testing style I would like to move other interactions to.

The new testing style within VCS services allows us to easily mock the underlying API calls for git related tests so that we are total in control of all interactions. This is amazing because it allows us to eventually remove the internal ExpressJS server that is propped up for testing the git.js module. While also letting us 100% test the module as we still can use the actual API calls for testing.
Although that isn't totally what's happening due to making sure our VCS exported functions are still compliant with the old git.js testing style.

So with that to say, all changes from main should still be here, #74 should still be left open to grab the last changes we want from it. And this should be merged as soon as we can to resolve public facing issues, and allow other PRs that rely on these changes to move forward. Then lastly any improvements or smaller changes that should be made to this PR can be made in a much more manageable state.

---

A huge thanks to everyone that has helped on this PR, it takes a village, and I appreciate all of you!

[Digitalone1]

So this generally would have a structure like the following?

switch (platform.toLowerCase) {
  case "gitlab":
    return getGitlabAuth(user, ...);
  case "bitbucket":
    return getBBAuth(user, ...);
  case "sourceforge":
    return getSFAuth(user, ...);
  default:
    return getGitHubAuth(user, ...);
}

On PHP I would make a class/interface and inherit for each platform, but is this doable on Javascript?

[Digitalone1]

@confused-Techie see if this could be useful: https://javascript.info/class-inheritance

class Github {
  constructor() {
    // Github platform
  }
  auth() {
    // GH auth
  }
  owner() {
    // GH owner
  }
  // ...
}

class Gitlab extends Github {
  constructor() {
    // Gitlab platform
  }
  auth() {
    // GL auth overrides GH
  }
  owner() {
    // GL owner overrides GH
  }
  // ...
}

[sertonix]

Gitlab extending Github doesn't make sense. Maybe create a generic Git class.

[Digitalone1]

Gitlab extending Github doesn't make sense. Maybe create a generic Git class.

If there were interfaces, it's a good idea, but there aren't in JS, so you have to define a default and we use Github at the moment.

Otherwise we could define a base git, then extend to Github, but git methods what do they do? Should they be empty and return nothing?

[sertonix]

If the Git class would be empty inhertens is usless and could cause confusion

[Digitalone1]

If the Git class would be empty inhertens is usless and could cause confusion

So you would define a base class Git which works for Github? If yes, I'd say call it Github directly. :)

[Daeraxa]

What is the list of forges you want to support?
For example I think you have:

• GitHub
• GitLab
• BitBucket
• SourceForge

One major service I would include is CodeBerg (https://codeberg.org/) (Gitea/Forejo based).

What about self hosted repos for GitLab, Gitea, Gogs, Forgejo? (Again just laying options out there, not that we have to support everything).

[confused-Techie]

Appreciate you guys chiming in, was not expected so much feedback here.

But @Daeraxa that list is 100% correct for what I was planning to try and support, we could add Codeberg as well. The big thing to see is if we will be able to, but if we want to we should start trying to identify their repos, but support can always also be added later.

And to @Digitalone1 my original idea was to essentially have something like package_handler.js interact with vcs.ownership() passing full user objects and package objects, from there, the VCS or Version Control System module would see which platform it needs and make the relevant ownership call to vcs_services/github.js with .ownership() or something.

But you idea of class inheritance could be interesting, suppose it depends on how much shared code there would be per implementation, but I'll look further into it, appreciate the idea!

[confused-Techie]

@Daeraxa as for self hosted support, keep in mind that currently all this section is able to do is detect what you are on, which is really just the precursor to supporting these services.

I'd love to support self-hosted solutions, but would have to identify a way to auto-detect their usage, which likely could come at the cost of hitting the server and hopefully there are some headers there describing the service, which may have the name there.

So to find out if this is possible, maybe do you know of any self-hosted instances of these services that I would be able to send a few web requests out to?

[Daeraxa]

Just a small typo (I assume).

[confused-Techie]

On a slightly bit of a separate note, you can see here my work on creating the PackageJSON class. The idea being that having something like this would be able to handle a lot of the nitty gritty parts of interacting with the package.json data for us.
Thing is, for it's scope and purpose having every single method return a Server Status Object is obviously a bit impossible, and you can even see in my notes while writing it I was realizing how helpful this would be over on package-frontend, so do you think it's important to try and push this out into it's own separate module sooner rather than later? Or does having the slightly different style within our repo bother you all that much? I know not super important just wanted to get your opinion there

If you think it's useful, do it sooner rather than later.

I'm giving a look at it more deeply. I wonder if serializing that class in JSON will give some issues since it contains also internal methods. Never tried to serialize an instance of a class.

Maybe adding a public method getData returing only the needed properties would be helpful?

And also validateName could be made private since it seems used only by internal methods.

Yeah essentially the eventually goal if it would be something like this.

The class PackageJSON can be provided a package.json where you can define several options such as BugMode, AuthorMode that sort of thing, you could then call PackageJSON.parse() and that will return your santized, healthy JSON file. Which can ensure everything is correct, but even for new Packages being published, we could call PackageJSON.validateName("pulsar") and that will automatically check for us if it's a valid name for the backend.

Plus then if we want to repository or some other value, we don't have to look for it, it's just PackageJSON.name and this will do all the work for us.

But we could do something like that, for now I'm leaving it a bit agnostic in implementation details to see if I did want to move it out for us elsewhere, But the PackageObjectFull class could then extend or just internally use PackageJSON and have implementation details much more closely tied in with our purposes. Which is exactly the idea.

Have that contain functions were we can say add these versions, rather than having to it ourselves in our Package Publish and Version Publish. But I'm hoping to have some time today to work on this.

---

As for you last comment, feel free to address those concerns as I'll be at work for the next few hours, otherwise will try and get to it once home.

[Digitalone1]

As for you last comment, feel free to address those concerns as I'll be at work for the next few hours, otherwise will try and get to it once home.

Did in #61

[confused-Techie]

This PR is now good to review, like I've explained above, while there are still areas to improve, it's been long enough, and theres enough changes waiting on it. I didn't mean for this PR to take as much time and effort as it has. But here we go

[confused-Techie]

Alright, as the changes here have been substantially discussed and worked on together, I'll go ahead and merge these changes.

I may not have the time to update production with the new code today, but if we would like to get any other changes in that build off of this we can do so before we bump the backend version and update prod.

Thanks again to all the fantastic help that has been received on this PR.