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

Add [dynamic-yaml] badge #1623

Merged
merged 11 commits into from
Apr 8, 2018
1 change: 1 addition & 0 deletions frontend/components/dynamic-badge-maker.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export default class DynamicBadgeMaker extends React.Component {
onChange={event => this.setState({ datatype: event.target.value })}>
<option value="" disabled>data type</option>
<option value="json">json</option>
<option value="yaml">yaml</option>
Copy link
Member

Choose a reason for hiding this comment

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

Can we sort them in an alphabetical order (json, xml, yaml)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See other comment about order.

<option value="xml">xml</option>
</select> {}
<input
Expand Down
3 changes: 3 additions & 0 deletions frontend/components/usage.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ export default class Usage extends React.PureComponent {
<p>
<code>/badge/dynamic/json.svg?url=&lt;URL&gt;&amp;label=&lt;LABEL&gt;&amp;query=&lt;<a href="https://www.npmjs.com/package/jsonpath" target="_BLANK" title="JSONdata syntax">$.DATA.SUBDATA</a>&gt;&amp;colorB=&lt;COLOR&gt;&amp;prefix=&lt;PREFIX&gt;&amp;suffix=&lt;SUFFIX&gt;</code>
</p>
<p>
<code>/badge/dynamic/yaml.svg?url=&lt;URL&gt;&amp;label=&lt;LABEL&gt;&amp;query=&lt;<a href="https://www.npmjs.com/package/jsonpath" target="_BLANK" title="JSONdata syntax">$.DATA.SUBDATA</a>&gt;&amp;colorB=&lt;COLOR&gt;&amp;prefix=&lt;PREFIX&gt;&amp;suffix=&lt;SUFFIX&gt;</code>
Copy link
Member

Choose a reason for hiding this comment

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

Can we sort them in an alphabetical order (json, xml, yaml)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes sure. I went with this order because the usage of json and yaml are the same and so thought I would keep them together.

</p>
Copy link
Member

Choose a reason for hiding this comment

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

Could you also add yaml to the list in the dynamic badge maker?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

<p>
<code>/badge/dynamic/xml.svg?url=&lt;URL&gt;&amp;label=&lt;LABEL&gt;&amp;query=&lt;<a href="https://www.npmjs.com/package/xpath" target="_BLANK" title="XPath syntax">//data/subdata</a>&gt;&amp;colorB=&lt;COLOR&gt;&amp;prefix=&lt;PREFIX&gt;&amp;suffix=&lt;SUFFIX&gt;</code>
</p>
Expand Down
22 changes: 22 additions & 0 deletions frontend/lib/badge-url.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,28 @@ describe('Badge URL functions', function() {
].join(''))
});

test(dynamicBadgeUrl, () => {
const yamlUrl = 'http://example.com/foo.yaml';
const query = '$.bar';
const prefix = 'value: ';
Copy link
Member

Choose a reason for hiding this comment

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

Aren't these three tests of dynamicBadgeUrl testing exactly the same thing? Maybe we should strip out two of them.

Copy link
Member

Choose a reason for hiding this comment

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

Good point. Let's remove two of them.


given(
'http://img.example.com',
'yaml',
'foo',
yamlUrl,
query,
{ prefix, style: 'plastic' }
).expect([
'http://img.example.com/badge/dynamic/yaml.svg',
'?label=foo',
`&url=${encodeURIComponent(yamlUrl)}`,
`&query=${encodeURIComponent(query)}`,
`&prefix=${encodeURIComponent(prefix)}`,
'&style=plastic',
].join(''))
});

test(dynamicBadgeUrl, () => {
const xmlUrl = 'http://example.com/foo.xml';
const query = '//bar';
Expand Down
50 changes: 28 additions & 22 deletions package-lock.json

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

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"escape-string-regexp": "^1.0.5",
"glob": "^7.1.1",
"gm": "^1.23.0",
"js-yaml": "^3.11.0",
"json-autosave": "~1.1.2",
"jsonpath": "~1.0.0",
"lodash.countby": "^4.6.0",
Expand Down
18 changes: 17 additions & 1 deletion server.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const queryString = require('query-string');
const semver = require('semver');
const xml2js = require('xml2js');
const xpath = require('xpath');
const yaml = require('js-yaml');
const Raven = require('raven');

const serverSecrets = require('./lib/server-secrets');
Expand Down Expand Up @@ -7466,7 +7467,7 @@ camp.route(/^\/maven-metadata\/v\/(https?)\/(.+\.xml)\.(svg|png|gif|jpg|json)$/,
}));

// User defined sources - JSON response
camp.route(/^\/badge\/dynamic\/(json|xml)\.(svg|png|gif|jpg|json)$/,
camp.route(/^\/badge\/dynamic\/(json|xml|yaml)\.(svg|png|gif|jpg|json)$/,
cache({
queryParams: ['uri', 'url', 'query', 'prefix', 'suffix'],
handler: function(query, match, sendBadge, request) {
Expand Down Expand Up @@ -7503,6 +7504,13 @@ cache({
}
};
break;
case 'yaml':
requestOptions = {
headers: {
Accept: 'application/yaml, text/yaml, text/plain'
Copy link
Member

Choose a reason for hiding this comment

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

From this Quora answer it seems we probably should use 'application/x-yaml, text/x-yaml, text/plain' here. Unless someone has a better source saying something else?

Copy link
Member

Choose a reason for hiding this comment

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

github.com, search in code:

  • "text/x-yaml" content type 39,389 code results
  • "text/yaml" content type 79,485 code results
  • "application/x-yaml" content type 34,264 code results
  • "application/yaml" content type 21,948 code results

Should we add all of them since it's not standardized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that's probably best.

}
};
break;
}

request(url, requestOptions, (err, res, data) => {
Expand Down Expand Up @@ -7533,6 +7541,14 @@ cache({
innerText.push(pathExpression.indexOf('@') + 1 ? i.value : i.firstChild.data);
});
break;
case 'yaml':
data = yaml.safeLoad(data);
data = jp.query(data, pathExpression);
if (!data.length) {
throw 'no result';
}
innerText = data;
break;
}
badgeData.text[1] = (prefix || '') + innerText.join(', ') + (suffix || '');
} catch (e) {
Expand Down
70 changes: 70 additions & 0 deletions services/yaml/yaml.tester.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
'use strict';

const Joi = require('joi');
const ServiceTester = require('../service-tester');
const colorscheme = require('../../lib/colorscheme.json');
const mapValues = require('lodash.mapvalues');

const colorsB = mapValues(colorscheme, 'colorB');

const t = new ServiceTester({ id: 'dynamic-yaml', title: 'User Defined YAML Source Data', pathPrefix: '/badge/dynamic/yaml' });
module.exports = t;

t.create('Connection error')
.get('.json?url=https://github.com/badges/shields/raw/master/package.json&query=$.name&label=Package Name&style=_shields_test')
Copy link
Member

Choose a reason for hiding this comment

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

Could you, just for the sake of a consistency, change an uri value to a yaml resource (from examples below)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops must've missed that one.

.networkOff()
.expectJSON({ name: 'Package Name', value: 'inaccessible', colorB: colorsB.red });

t.create('No URL specified')
.get('.json?query=$.name&label=Package Name&style=_shields_test')
.expectJSON({ name: 'Package Name', value: 'no url specified', colorB: colorsB.red });

t.create('No query specified')
.get('.json?url=https://raw.githubusercontent.com/kubernetes/charts/568291d6e476c39ca8322c30c3f601d0383d4760/stable/coredns/Chart.yaml&label=Package Name&style=_shields_test')
Copy link
Member

Choose a reason for hiding this comment

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

You've used a link to specified commit, which great in my opinion :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 I thought it would be consistent when testing.

.expectJSON({ name: 'Package Name', value: 'no query specified', colorB: colorsB.red });

t.create('YAML from url')
.get('.json?url=https://raw.githubusercontent.com/kubernetes/charts/568291d6e476c39ca8322c30c3f601d0383d4760/stable/coredns/Chart.yaml&query=$.name&style=_shields_test')
.expectJSON({ name: 'custom badge', value: 'coredns', colorB: colorsB.brightgreen });

t.create('YAML from uri (support uri query paramater)')
.get('.json?uri=https://raw.githubusercontent.com/kubernetes/charts/568291d6e476c39ca8322c30c3f601d0383d4760/stable/coredns/Chart.yaml&query=$.name&style=_shields_test')
.expectJSON({ name: 'custom badge', value: 'coredns', colorB: colorsB.brightgreen });

t.create('YAML from url | multiple results')
.get('.json?url=https://raw.githubusercontent.com/kubernetes/charts/568291d6e476c39ca8322c30c3f601d0383d4760/stable/coredns/Chart.yaml&query=$..keywords[0:2:1]')
.expectJSON({ name: 'custom badge', value: 'coredns, dns' });

t.create('YAML from url | caching with new query params')
.get('.json?url=https://raw.githubusercontent.com/kubernetes/charts/568291d6e476c39ca8322c30c3f601d0383d4760/stable/coredns/Chart.yaml&query=$.version')
.expectJSONTypes(Joi.object().keys({
name: 'custom badge',
value: Joi.string().regex(/^\d+(\.\d+)?(\.\d+)?$/)
Copy link
Member

Choose a reason for hiding this comment

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

I think you can use an isSemver validator here:

.expectJSONTypes(Joi.object().keys({
name: 'custom badge',
value: isSemver

Or even better you can change expect to

.expectJSON({ name: 'custom badge', value: '0.8.0' });

since we are using resource from a specific commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just copied the test from the json dynamic badge. Happy to update if you prefer.

}));

t.create('YAML from url | with prefix & suffix & label')
.get('.json?url=https://raw.githubusercontent.com/kubernetes/charts/568291d6e476c39ca8322c30c3f601d0383d4760/stable/coredns/Chart.yaml&query=$.version&prefix=v&suffix= dev&label=Shields')
.expectJSONTypes(Joi.object().keys({
Copy link
Member

Choose a reason for hiding this comment

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

What do you thin about using this approach?

.expectJSON({ name: 'Shields', value: 'v0.8.0 dev' });

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Much neater, again I just copied the other test.

name: 'Shields',
value: Joi.string().regex(/^v\d+(\.\d+)?(\.\d+)?\sdev$/)
}));

t.create('YAML from url | object doesnt exist')
.get('.json?url=https://raw.githubusercontent.com/kubernetes/charts/568291d6e476c39ca8322c30c3f601d0383d4760/stable/coredns/Chart.yaml&query=$.does_not_exist&style=_shields_test')
.expectJSON({ name: 'custom badge', value: 'no result', colorB: colorsB.lightgrey });

t.create('YAML from url | invalid url')
.get('.json?url=https://raw.githubusercontent.com/kubernetes/charts/568291d6e476c39ca8322c30c3f601d0383d4760/stable/coredns/notafile.yaml&query=$.version&style=_shields_test')
.expectJSON({ name: 'custom badge', value: 'resource not found', colorB: colorsB.lightgrey });

t.create('YAML from url | user color overrides default')
.get('.json?url=https://raw.githubusercontent.com/kubernetes/charts/568291d6e476c39ca8322c30c3f601d0383d4760/stable/coredns/Chart.yaml&query=$.name&colorB=10ADED&style=_shields_test')
.expectJSON({ name: 'custom badge', value: 'coredns', colorB: '#10ADED' });

t.create('YAML from url | error color overrides default')
.get('.json?url=https://raw.githubusercontent.com/kubernetes/charts/568291d6e476c39ca8322c30c3f601d0383d4760/stable/coredns/notafile.yaml&query=$.version&style=_shields_test')
.expectJSON({ name: 'custom badge', value: 'resource not found', colorB: colorsB.lightgrey });

t.create('YAML from url | error color overrides user specified')
.get('.json?query=$.version&colorB=10ADED&style=_shields_test')
.expectJSON({ name: 'custom badge', value: 'no url specified', colorB: colorsB.red });