Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Packagist] dependency version #8371

Merged
merged 31 commits into from
Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
4448d3f
happy path is done: packagist depenency version is returned for depen…
PaulaBarszcz Aug 22, 2022
7db6ba2
Merge branch 'badges:master' into packagist-dependency-version
PaulaBarszcz Aug 22, 2022
411ec19
Merge branch 'badges:master' into packagist-dependency-version
PaulaBarszcz Aug 25, 2022
2b97033
changes in error handling in packagist dependency version service
PaulaBarszcz Aug 25, 2022
77717e4
Move to packagist-base common parts of packagist-php and packagist-de…
PaulaBarszcz Aug 26, 2022
94474a9
Merge branch 'badges:master' into packagist-dependency-version
PaulaBarszcz Sep 1, 2022
623b787
the label now shows the name of the dependency
PaulaBarszcz Sep 1, 2022
6de21c3
add comments,slightly modify if statement
PaulaBarszcz Sep 1, 2022
905667c
fix unit tests in services/packagist/packagist-php-version.spec.js
PaulaBarszcz Sep 2, 2022
6fe762e
add unit tests for services/packagist/packagist-dependency-version.js
PaulaBarszcz Sep 2, 2022
55f50ce
service tests for packagist-dependency-version.service.js are in prog…
PaulaBarszcz Sep 2, 2022
623b442
Add additional service tests for packagist-dependency-version.tester.js
PaulaBarszcz Sep 2, 2022
fd96f9e
remove toLowerCase()
PaulaBarszcz Sep 2, 2022
de9aee1
resolve from lgtm: 1 for Wrong use of 'this' for static method
PaulaBarszcz Sep 2, 2022
9b0e9cc
DRY determining the name of the dependency (depVen/depRepo)
PaulaBarszcz Sep 3, 2022
1602710
Merge branch 'master' into packagist-dependency-version
PaulaBarszcz Sep 4, 2022
55c36df
Merge branch 'master' into packagist-dependency-version
PaulaBarszcz Sep 7, 2022
c1a7786
url change in strategy is in progress
PaulaBarszcz Sep 7, 2022
b3c8014
basic examples of packagist/dependency-v work with new url; packagist…
PaulaBarszcz Sep 7, 2022
46bc9b0
service tests should be green
PaulaBarszcz Sep 8, 2022
8eb8437
Merge branch 'master' into packagist-dependency-version
PaulaBarszcz Sep 8, 2022
ed15ffb
unit tests for packagist should be fixed
PaulaBarszcz Sep 8, 2022
a0a99ad
add toLowerCase()
PaulaBarszcz Sep 8, 2022
af35f84
part of suggestions from the PR is implemented
PaulaBarszcz Sep 12, 2022
e356b1b
updated tests for packagist dependency version
PaulaBarszcz Sep 12, 2022
d78c83d
Merge branch 'master' into packagist-dependency-version
PaulaBarszcz Sep 12, 2022
4071359
fix packagist dependency version spec
PaulaBarszcz Sep 12, 2022
3a2fce2
Merge branch 'master' into packagist-dependency-version
PaulaBarszcz Sep 14, 2022
a6fd477
add missing unit tests
PaulaBarszcz Sep 14, 2022
126eb42
Update services/packagist/packagist-dependency-version.spec.js
PaulaBarszcz Sep 14, 2022
becfc0e
Update services/packagist/packagist-dependency-version.spec.js
PaulaBarszcz Sep 14, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 3 additions & 6 deletions services/packagist/packagist-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ const packageSchema = Joi.array().items(
Joi.object({
version: Joi.string().required(),
require: Joi.alternatives(
Joi.object({
php: Joi.string(),
}).required(),
Joi.object().pattern(Joi.string(), Joi.string()).required(),
PaulaBarszcz marked this conversation as resolved.
Show resolved Hide resolved
Joi.string().valid('__unset')
),
})
Expand All @@ -23,7 +21,7 @@ class BasePackagistService extends BaseJsonService {
/**
* Default fetch method.
*
* This method utilize composer metadata API which
* This method utilizes composer metadata API which
* "... is the preferred way to access the data as it is always up to date,
* and dumped to static files so it is very efficient on our end." (comment from official documentation).
* For more information please refer to https://packagist.org/apidoc#get-package-data.
Expand All @@ -47,7 +45,7 @@ class BasePackagistService extends BaseJsonService {
/**
* Fetch dev releases method.
*
* This method utilize composer metadata API which
* This method utilizes composer metadata API which
* "... is the preferred way to access the data as it is always up to date,
* and dumped to static files so it is very efficient on our end." (comment from official documentation).
* For more information please refer to https://packagist.org/apidoc#get-package-data.
Expand Down Expand Up @@ -166,7 +164,6 @@ class BasePackagistService extends BaseJsonService {
return versions.filter(version => version.version === release)[0]
}
}

const customServerDocumentationFragment = `
<p>
Note that only network-accessible packagist.org and other self-hosted Packagist instances are supported.
Expand Down
204 changes: 204 additions & 0 deletions services/packagist/packagist-dependency-version.service.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
import Joi from 'joi'
import { optionalUrl } from '../validators.js'
import { NotFound } from '../index.js'
import {
allVersionsSchema,
BasePackagistService,
customServerDocumentationFragment,
} from './packagist-base.js'

const queryParamSchema = Joi.object({
server: optionalUrl,
version: Joi.string(),
}).required()

export default class PackagistDependencyVersion extends BasePackagistService {
static category = 'platform-support'

static route = {
base: 'packagist/dependency-v',
pattern: ':user/:repo/:dependency+',
queryParamSchema,
}

static examples = [
{
title: 'Packagist Dependency Version Support',
PaulaBarszcz marked this conversation as resolved.
Show resolved Hide resolved
namedParams: {
user: 'symfony',
repo: 'symfony',
dependency: 'twig/twig',
},
staticPreview: this.render({
dependency: 'twig/twig',
dependencyVersion: '2.13|^3.0.4',
}),
},
{
title: 'Packagist Dependency Version Support (specify version)',
namedParams: {
user: 'symfony',
repo: 'symfony',
dependency: 'twig/twig',
},
queryParams: {
version: 'v2.8.0',
},
staticPreview: this.render({
dependency: 'twig/twig',
dependencyVersion: '1.12',
}),
},
{
title: 'Packagist Dependency Version Support (custom server)',
namedParams: {
user: 'symfony',
repo: 'symfony',
dependency: 'twig/twig',
},
queryParams: {
server: 'https://packagist.org',
},
staticPreview: this.render({
dependency: 'twig/twig',
dependencyVersion: '2.13|^3.0.4',
}),
documentation: customServerDocumentationFragment,
},
{
title: 'Packagist PHP Version Support (custom server)',
namedParams: {
user: 'symfony',
repo: 'symfony',
dependency: 'php',
},
queryParams: {
server: 'https://packagist.org',
version: 'v2.8.0',
},
PaulaBarszcz marked this conversation as resolved.
Show resolved Hide resolved
staticPreview: this.render({
dependency: 'php',
dependencyVersion: '^7.1.3',
}),
documentation: customServerDocumentationFragment,
},
]

static defaultBadgeData = {
PaulaBarszcz marked this conversation as resolved.
Show resolved Hide resolved
label: 'dependency version',
color: 'blue',
}

static render({ dependency, dependencyVersion }) {
return {
label: dependency,
message: dependencyVersion,
}
}

async getDependencyVersion({
json,
user,
repo,
dependency,
version = '',
server,
}) {
let packageVersion
const versions = BasePackagistService.expandPackageVersions(
json,
this.getPackageName(user, repo)
)

if (version === '') {
packageVersion = this.findLatestRelease(versions)
} else {
try {
packageVersion = await this.findSpecifiedVersion(
versions,
user,
repo,
version,
server
)
} catch (e) {
packageVersion = null
}
}

if (!dependency) {
throw new NotFound({
prettyMessage: 'dependency vendor or repo not specified',
})
}
PaulaBarszcz marked this conversation as resolved.
Show resolved Hide resolved

if (!packageVersion) {
throw new NotFound({ prettyMessage: 'invalid version' })
}

if (!packageVersion.require) {
throw new NotFound({ prettyMessage: 'version requirement not found' })
}

const depLowerCase = dependency.toLowerCase()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure no packagist vendor name, package name or extension name can contain an upper-case character?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question - I don’t think we can be sure of that.
Should i remove the above line?
It would then have issues with querries containing capital letters
(#8371 (comment))

We can do one of the following:

  1. first check if there is a position in the response in ‚require’ section that contains directly the dependecy’s name (with capital letters)
    and if not, transform the queried dependency’s name to lowercase and repeat the comparison
  2. transform every dependency’s name from the reeponse to lowercase - and compare with lowercase depemdency’s name from the query.

What would be your preference?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified it, now it looks like this - is it fine?

    // All dependencies' names in the 'require' section from the response should be lowercase,
    // so that we can compare lowercase name of the dependency given via url by the user.
    Object.keys(packageVersion.require).forEach(dependency => {
      packageVersion.require[dependency.toLowerCase()] =
        packageVersion.require[dependency]
    })

    const depLowerCase = dependency.toLowerCase()

    if (!packageVersion.require[depLowerCase]) {
      throw new NotFound({ prettyMessage: 'version requirement not found' })
    }

    return { dependencyVersion: packageVersion.require[depLowerCase] }


if (!packageVersion.require[depLowerCase]) {
throw new NotFound({ prettyMessage: 'version requirement not found' })
}

return { dependencyVersion: packageVersion.require[depLowerCase] }
}

async handle({ user, repo, dependency }, { server, version = '' }) {
const allData = await this.fetch({
user,
repo,
schema: allVersionsSchema,
server,
})

const { dependencyVersion } = await this.getDependencyVersion({
json: allData,
user,
repo,
dependency,
version,
server,
})

return this.constructor.render({
dependency,
dependencyVersion,
})
}

findVersionIndex(json, version) {
return json.findIndex(v => v.version === version)
}

async findSpecifiedVersion(json, user, repo, version, server) {
let release

if ((release = json[this.findVersionIndex(json, version)])) {
return release
} else {
try {
const allData = await this.fetchDev({
user,
repo,
schema: allVersionsSchema,
server,
})

const versions = BasePackagistService.expandPackageVersions(
allData,
this.getPackageName(user, repo)
)

return versions[this.findVersionIndex(versions, version)]
} catch (e) {
return release
}
}
}
}
113 changes: 113 additions & 0 deletions services/packagist/packagist-dependency-version.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import chai from 'chai'
import chaiAsPromised from 'chai-as-promised'
import PackagistDependencyVersion from './packagist-dependency-version.service.js'
const { expect } = chai
chai.use(chaiAsPromised)

describe('PackagistDependencyVersion', function () {
const fullPackagistJson = {
packages: {
'frodo/the-one-package': [
{
version: 'v3.0.0',
require: { php: '^7.4 || 8', 'twig/twig': '~1.28|~2.0' },
},
{
version: 'v2.0.0',
require: { php: '^7.2', 'twig/twig': '~1.20|~1.30' },
},
{
version: 'v1.0.0',
require: { php: '^5.6 || ^7', 'twig/twig': '~1.10|~1.0' },
},
],
},
}

it('should throw NotFound when package version is missing', async function () {
await expect(
PackagistDependencyVersion.prototype.getDependencyVersion({
json: fullPackagistJson,
user: 'frodo',
repo: 'the-one-package',
version: 'v4.0.0',
})
).to.be.rejectedWith('dependency vendor or repo not specified')
})

it('should throw NotFound when dependency not specified (when using default release)', async function () {
await expect(
PackagistDependencyVersion.prototype.getDependencyVersion({
json: fullPackagistJson,
user: 'frodo',
repo: 'the-one-package',
})
).to.be.rejectedWith('dependency vendor or repo not specified')
})

it('should throw NotFound when dependency not specified (when using specified release)', async function () {
const fullPackagistJson = {
packages: {
'frodo/the-one-package': [
{
version: 'v3.0.0',
require: { php: '^7.4 || 8' },
},
{
version: 'v2.0.0',
require: { php: '^7.2' },
},
{
version: 'v1.0.0',
require: '__unset',
},
],
},
}
await expect(
PackagistDependencyVersion.prototype.getDependencyVersion({
json: fullPackagistJson,
user: 'frodo',
repo: 'the-one-package',
version: 'v1.0.0',
})
).to.be.rejectedWith('dependency vendor or repo not specified')
})

it('should throw NotFound if dependency was not specified', async function () {
await expect(
PackagistDependencyVersion.prototype.getDependencyVersion({
json: fullPackagistJson,
user: 'frodo',
repo: 'the-one-package',
})
).to.be.rejectedWith('dependency vendor or repo not specified')
})

it('should return dependency version for the default release', async function () {
expect(
await PackagistDependencyVersion.prototype.getDependencyVersion({
json: fullPackagistJson,
user: 'frodo',
repo: 'the-one-package',
dependency: 'twig/twig',
})
)
.to.have.property('dependencyVersion')
.that.equals('~1.28|~2.0')
})

it('should return dependency version for the specified release', async function () {
expect(
await PackagistDependencyVersion.prototype.getDependencyVersion({
json: fullPackagistJson,
user: 'frodo',
repo: 'the-one-package',
version: 'v2.0.0',
dependency: 'twig/twig',
})
)
.to.have.property('dependencyVersion')
.that.equals('~1.20|~1.30')
})
})