Skip to content

Commit

Permalink
Integrate new path-to-regexp with trailing optionals (#2644)
Browse files Browse the repository at this point in the history
  • Loading branch information
paulmelnikow authored Jan 8, 2019
1 parent 4efe883 commit d1c5378
Show file tree
Hide file tree
Showing 21 changed files with 41 additions and 75 deletions.
13 changes: 2 additions & 11 deletions doc/rewriting-services.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,7 @@ tests are passing, though.
the route using a `pattern`. A `pattern` (e.g. `pattern: ':param1/:param2'`) is
the simplest way to declare the route, also the most readable, and will be
useful for displaying a badge builder with fields in the front end and
generating badge URLs programmatically. One limitation to keep in mind is that,
at present, the trailing parameter of a pattern can't be optional. If the last
part of a route is optional, like a branch, you will need to use a `format`
regex string (e.g. `format: '([^/]+/[^/]+)'`).
generating badge URLs programmatically.

3. When creating the initial route, you can stub out the service. A minimal
service extends BaseJsonService (or BaseService, or one of the others), and
Expand Down Expand Up @@ -264,13 +261,7 @@ In either case, the service should throw e.g

## Convert the examples

1. Convert all the examples to `pattern`, `namedParams`, and
`staticPreview`. In some cases you can use the `pattern` inherited
from `route`, though in other cases you may need to specify a pattern
in the example. For example, when showing download badges for several
periods, you may want to render the example with an explicit `dt`
instead of `:which`. You will also need to specify a pattern for badges
that use a `format` regex in the route.
1. Convert all the examples to `pattern`, `namedParams`, and `staticExample`. In some cases you can use the `pattern` inherited from `route`, though in other cases you may need to specify a pattern in the example. For example, when showing download badges for several periods, you may want to render the example with an explicit `dt` instead of `:which`. You will also need to specify a pattern for badges that use a `format` regex in the route.

2. Open the frontend and check that the static preview badges look good.
Remember, none of them are live.
Expand Down
5 changes: 2 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"lodash.times": "^4.3.2",
"moment": "^2.23.0",
"node-env-flag": "^0.1.0",
"path-to-regexp": "^2.4.0",
"path-to-regexp": "pillarjs/path-to-regexp#a33e7b7a5225ff7d301961a89439e9874674cbfa",
"pretty-bytes": "^5.0.0",
"priorityqueuejs": "^1.0.0",
"prom-client": "^11.2.1",
Expand Down
7 changes: 3 additions & 4 deletions services/appveyor/appveyor-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ module.exports = class AppVeyorBase extends BaseJsonService {
return 'build'
}

async fetch({ repo, branch }) {
let url = `https://ci.appveyor.com/api/projects/${repo}`
async fetch({ user, repo, branch }) {
let url = `https://ci.appveyor.com/api/projects/${user}/${repo}`
if (branch != null) {
url += `/branch/${branch}`
}
Expand All @@ -37,8 +37,7 @@ module.exports = class AppVeyorBase extends BaseJsonService {
static buildRoute(base) {
return {
base,
format: '([^/]+/[^/]+)(?:/(.+))?',
capture: ['repo', 'branch'],
pattern: ':user/:repo/:branch*',
}
}
}
4 changes: 2 additions & 2 deletions services/appveyor/appveyor-ci.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ module.exports = class AppVeyorCi extends AppVeyorBase {
}
}

async handle({ repo, branch }) {
const data = await this.fetch({ repo, branch })
async handle({ user, repo, branch }) {
const data = await this.fetch({ user, repo, branch })
if (!data.hasOwnProperty('build')) {
// this project exists but no builds have been run on it yet
return this.constructor.render({ status: 'no builds found' })
Expand Down
4 changes: 2 additions & 2 deletions services/appveyor/appveyor-tests.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ module.exports = class AppVeyorTests extends AppVeyorBase {
}

async handle(
{ repo, branch },
{ user, repo, branch },
{
compact_message: compactMessage,
passed_label: passedLabel,
Expand All @@ -101,7 +101,7 @@ module.exports = class AppVeyorTests extends AppVeyorBase {
}
) {
const isCompact = compactMessage !== undefined
const data = await this.fetch({ repo, branch })
const data = await this.fetch({ user, repo, branch })

if (!data.hasOwnProperty('build')) {
return { message: 'no builds found' }
Expand Down
5 changes: 2 additions & 3 deletions services/azure-devops/azure-devops-coverage.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ module.exports = class AzureDevOpsCoverage extends AzureDevOpsBase {
},
{
title: 'Azure DevOps coverage (branch)',
pattern: ':organization/:project/:definitionId/:branch',
pattern: ':organization/:project/:definitionId/:branch*',
namedParams: {
organization: 'swellaby',
project: 'opensource',
Expand All @@ -96,8 +96,7 @@ module.exports = class AzureDevOpsCoverage extends AzureDevOpsBase {
static get route() {
return {
base: 'azure-devops/coverage',
format: '([^/]+)/([^/]+)/([^/]+)(?:/(.+))?',
capture: ['organization', 'project', 'definitionId', 'branch'],
pattern: ':organization/:project/:definitionId/:branch*',
}
}

Expand Down
5 changes: 2 additions & 3 deletions services/azure-devops/azure-devops-tests.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ module.exports = class AzureDevOpsTests extends AzureDevOpsBase {
},
{
title: 'Azure DevOps tests (branch)',
pattern: ':organization/:project/:definitionId/:branch',
pattern: ':organization/:project/:definitionId/:branch*',
namedParams: {
organization: 'azuredevops-powershell',
project: 'azuredevops-powershell',
Expand Down Expand Up @@ -176,8 +176,7 @@ module.exports = class AzureDevOpsTests extends AzureDevOpsBase {
static get route() {
return {
base: 'azure-devops/tests',
format: '([^/]+)/([^/]+)/([^/]+)(?:/(.+))?',
capture: ['organization', 'project', 'definitionId', 'branch'],
pattern: ':organization/:project/:definitionId/:branch*',
queryParams: [
'compact_message',
'passed_label',
Expand Down
3 changes: 1 addition & 2 deletions services/date/date.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ module.exports = class Date extends BaseService {
static get route() {
return {
base: 'date',
format: '([0-9]+)',
capture: ['timestamp'],
pattern: ':timestamp([0-9]+)',
}
}

Expand Down
5 changes: 2 additions & 3 deletions services/github/github-manifest.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ class GithubManifestVersion extends ConditionalGithubAuthService {
static get route() {
return {
base: 'github/manifest-json/v',
format: '([^/]+)/([^/]+)/?([^/]+)?',
capture: ['user', 'repo', 'branch'],
pattern: ':user/:repo/:branch*',
}
}

Expand All @@ -44,7 +43,7 @@ class GithubManifestVersion extends ConditionalGithubAuthService {
},
{
title: 'GitHub manifest version',
pattern: ':user/:repo/:branch',
pattern: ':user/:repo/:branch*',
namedParams: {
user: 'RedSparr0w',
repo: 'IndieGala-Helper',
Expand Down
9 changes: 4 additions & 5 deletions services/github/github-package-json.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ class GithubPackageJsonVersion extends ConditionalGithubAuthService {
static get route() {
return {
base: 'github/package-json/v',
format: '([^/]+)/([^/]+)/?([^/]+)?',
capture: ['user', 'repo', 'branch'],
pattern: ':user/:repo/:branch*',
}
}

Expand All @@ -38,8 +37,8 @@ class GithubPackageJsonVersion extends ConditionalGithubAuthService {
documentation,
},
{
title: 'GitHub package.json version',
pattern: ':user/:repo/:branch',
title: 'GitHub package.json version (branch)',
pattern: ':user/:repo/:branch*',
namedParams: {
user: 'IcedFrisby',
repo: 'IcedFrisby',
Expand Down Expand Up @@ -102,7 +101,7 @@ class DynamicGithubPackageJson extends ConditionalGithubAuthService {
},
{
title: 'GitHub package.json dynamic',
pattern: ':key/:user/:repo/:branch',
pattern: ':key/:user/:repo/:branch*',
namedParams: {
key: 'keywords',
user: 'developit',
Expand Down
8 changes: 2 additions & 6 deletions services/gitlab/gitlab-pipeline-status.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,7 @@ module.exports = class GitlabPipelineStatus extends BaseSvgScrapingService {
static get route() {
return {
base: 'gitlab/pipeline',
format: '([^/]+)/([^/]+)(?:/([^/]+))?',
capture: ['user', 'repo', 'branch'],
// Trailing optional parameters don't work. The issue relates to the `.`
// separator before the extension.
// pattern: ':user/:repo/:branch?',
pattern: ':user/:repo/:branch*',
queryParams: ['gitlab_url'],
}
}
Expand All @@ -43,7 +39,7 @@ module.exports = class GitlabPipelineStatus extends BaseSvgScrapingService {
},
{
title: 'Gitlab pipeline status (branch)',
pattern: ':user/:repo/:branch',
pattern: ':user/:repo/:branch*',
namedParams: {
user: 'gitlab-org',
repo: 'gitlab-ce',
Expand Down
5 changes: 2 additions & 3 deletions services/jenkins/jenkins-plugin-installs.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ class JenkinsPluginInstalls extends BaseJsonService {
static get route() {
return {
base: 'jenkins/plugin/i',
format: '([^/]+)/?([^/]+)?',
capture: ['plugin', 'version'],
pattern: ':plugin/:version?',
}
}

Expand All @@ -117,7 +116,7 @@ class JenkinsPluginInstalls extends BaseJsonService {
}),
},
{
title: 'Jenkins Plugin installs',
title: 'Jenkins Plugin installs (version)',
pattern: ':plugin/:version',
namedParams: {
plugin: 'view-job-filters',
Expand Down
3 changes: 1 addition & 2 deletions services/matrix/matrix.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,7 @@ module.exports = class Matrix extends BaseJsonService {
static get route() {
return {
base: 'matrix',
format: '([^/]+)',
capture: ['roomAlias'],
pattern: ':roomAlias',
queryParams: ['server_fqdn'],
}
}
Expand Down
4 changes: 1 addition & 3 deletions services/npm/npm-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@ module.exports = class NpmBase extends BaseJsonService {
if (withTag) {
return {
base,
// The trailing optional means this has to be a regex.
format: '(?:(@[^/]+)/)?([^/]*)/?([^/]*)',
capture: ['scope', 'packageName', 'tag'],
pattern: ':scope(@[^/]+)?/:packageName/:tag?',
queryParams: ['registry_uri'],
}
} else {
Expand Down
3 changes: 1 addition & 2 deletions services/readthedocs/readthedocs.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ module.exports = class ReadTheDocs extends BaseSvgScrapingService {
static get route() {
return {
base: 'readthedocs',
format: '([^/]+)(?:/(.+))?',
capture: ['project', 'version'],
pattern: ':project/:version?',
}
}

Expand Down
11 changes: 5 additions & 6 deletions services/requires/requires.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,21 @@ module.exports = class RequiresIo extends BaseJsonService {
static get route() {
return {
base: 'requires',
format: '([^/]+)/([^/]+/[^/]+)(?:/(.+))?',
capture: ['service', 'repo', 'branch'],
pattern: ':service/:user/:repo/:branch*',
}
}

static get defaultBadgeData() {
return { label: 'requirements' }
}

async handle({ service, repo, branch }) {
const { status } = await this.fetch({ service, repo, branch })
async handle({ service, user, repo, branch }) {
const { status } = await this.fetch({ service, user, repo, branch })
return this.constructor.render({ status })
}

async fetch({ service, repo, branch }) {
const url = `https://requires.io/api/v1/status/${service}/${repo}`
async fetch({ service, user, repo, branch }) {
const url = `https://requires.io/api/v1/status/${service}/${user}/${repo}`
return this._requestJson({
url,
schema: statusSchema,
Expand Down
3 changes: 1 addition & 2 deletions services/shippable/shippable.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ module.exports = class Shippable extends BaseJsonService {
static get route() {
return {
base: 'shippable',
format: '([^/]+)(?:/(.+))?',
capture: ['projectId', 'branch'],
pattern: ':projectId/:branch*',
}
}

Expand Down
3 changes: 1 addition & 2 deletions services/snyk/snyk-vulnerability-github.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ module.exports = class SnykVulnerabilityGitHub extends SynkVulnerabilityBase {
static get route() {
return {
base: 'snyk/vulnerabilities/github',
format: '([^/]+)/([^/]+)(?:/(.+))?',
capture: ['user', 'repo', 'manifestFilePath'],
pattern: ':user/:repo/:manifestFilePath*',
}
}

Expand Down
8 changes: 2 additions & 6 deletions services/wordpress/wordpress-downloads.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,14 @@ function InstallsForExtensionType(extensionType) {
static get route() {
return {
base: `wordpress/${extensionType}/installs`,
format: '(.+)',
capture: ['slug'],
pattern: ':slug',
}
}

static get examples() {
return [
{
title: `Wordpress ${capt} Active Installs`,
pattern: ':slug',
namedParams: { slug: exampleSlug },
staticPreview: this.render({ response: { active_installs: 300000 } }),
},
Expand Down Expand Up @@ -143,16 +141,14 @@ function DownloadsForInterval(interval) {
static get route() {
return {
base,
format: '(.*)',
capture: ['slug'],
pattern: ':slug',
}
}

static get examples() {
return [
{
title: 'WordPress Plugin Downloads',
pattern: ':slug',
namedParams: { slug: 'bbpress' },
staticPreview: this.render({ downloads: 30000 }),
},
Expand Down
6 changes: 2 additions & 4 deletions services/wordpress/wordpress-platform.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,7 @@ class WordpressPluginTestedVersion extends BaseWordpressPlatform {
static get route() {
return {
base: `wordpress/plugin/tested`,
format: '(.+)',
capture: ['slug'],
pattern: ':slug',
}
}

Expand All @@ -136,8 +135,7 @@ class WordpressPluginTestedVersionAlias extends WordpressPluginTestedVersion {
static get route() {
return {
base: `wordpress/v`,
format: '(.+)',
capture: ['slug'],
pattern: ':slug',
}
}

Expand Down

0 comments on commit d1c5378

Please sign in to comment.