Migrate to Express [*****]

[paulmelnikow]

There's surely a bit of cleanup to do here, but thought it worth getting some 👀 on this.

I'd welcome help updating and improving the documentation, though that could also be done in a follow-on PR.

An area I've not yet addressed is query parameters. Scoutcamp used to turn the params into numbers, which I imagine Express does not do. It means we probably can simplify places where we've got Joi.alternatives().try(Joi.string(), Joi.number()) in the query param schemas.

There are surely bugs in this, and edge cases. I removed a few tests in legacy-request-handler.spec.js which we may want to rewrite elsewhere.

We should test this out (maybe even on Fly), in particular things like suggest and token acceptance.

I would be surprised if performance were a problem, though we may want to experiment with this under load to make sure.

Closes #3328 Closes #3368 Closes #4948

[shields-ci]

	Warnings
:warning:	:books: Remember to ensure any changes to config.private in services/github/auth/acceptor.spec.js are reflected in the server secrets documentation
:warning:	:books: Remember to ensure any changes to config.private in services/github/auth/acceptor.js are reflected in the server secrets documentation
:warning:	:books: Remember to ensure any changes to config.private in services/github/github-constellation.js are reflected in the server secrets documentation

	Messages
:book:	Thanks for contributing to our documentation. We :heart: our documentarians!
:book:	:sparkles: Thanks for your contribution to Shields, @paulmelnikow!

Generated by :no_entry_sign: dangerJS against fc68b88a04e41d3243da57dd317eee98d93f2935

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 78b886c01067c3541c6cf8be7679b50aba6a3e6d into 560d26784494a39965fe0bcb0ab658bb37245349 - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[paulmelnikow]

In the first service test run there are some interesting failures.

One issue is that the JSON badge no longer strips leading and trailing whitespace from label and message. That could be added to makeJsonBadge… or maybe we should just fix the services.

[chris48s]

Thanks for taking this on. This aspect of the codebase and the reliance on ScoutCamp is one of the biggest pain points of this codebase so it is really good to be making some progress on this :)

I haven't done a full review of this, but I have checked it out locally and skimmed the diff.

The main issue I hit is if I run npm start on a checkout of this branch, it throws

Failed to load plugin 'graphql' declared in 'BaseConfig': Cannot find module 'ws'

and won't start the frontend, so I've only been able to test the badge server. Any idea what is happening there?

---

There were a few bits I wanted to specifically check as possible edge cases:

• Badges with regex routes e.g:

  • services/travis/travis-build.service.js
  • services/wercker/wercker.service.js
  • services/requires/requires.service.js
  • services/static-badge/static-badge.service.js Those all seem to work fine 👍

• .png redirect to raster proxy. Also works 👍

• /endpoint route conflict - not tested yet as I couldn't start the frontend.

---

I'll try and have a more detailed look at this next week.

[paulmelnikow]

Thanks for the comments. Will nudge this forward when I can!

[PyvesB]

Given that there hasn't been any activity for a while, I'm going to close this pull request. Feel free to ping us or reopen it if you get a chance to look into it again. 😉

If others think that repository level projects would be an option and want to pick this up, you can simply fetch the commits by running git fetch origin pull/7877/head:pr-7877. Change the implementation to target repository level projects and open a new pull request with a co-authored trailer included in the commit message to give attribution to the original author.

[chris48s]

It is still my ambition to pick this up and finish it at some point, but agree that there is limited value in keeping the PR hanging around