- The goal of this PR is:
- Consume the new service-definition format. (#2397)
- Make the frontend more readable.
- Behavior changes:
- I changed the **Image** field in the markup modal to show only the path.
- I added another click-to-select field below that shows the complete URL.
- This made it easier to suppress the live badge preview while it contains placeholders like `:user` or `:gem`, a minor tweak discussed at https://github.com/badges/shields/issues/2427#issuecomment-442972100.
- The search box now searches all categories, regardless of the current page. (This is an improvement, I would say.)
- I did not deliberately address performance, though I ripped out a bunch of anonymous functions and avoided re-filtering all the examples by category on every render, which I expect will not hurt. I haven't really tested this on a mobile connection and it'd be worth doing that.
- It would be great to have some tests of the components, though getting started with that seemed like a big project and I did not want to make this any larger than it already is.
It's a medium-sized refactor:
1. Replace `BadgeExamples`, `Category` and `Badge` component with a completely rewritten `BadgeExamples` component which renders a table of badges, and `CategoryHeading` and `CategoryHeadings` components.
2. Refactor `ExamplesPage` and `SearchResults` components into a new `Main` component.
3. Rewrite the data flow for `MarkupModal`. Rather than rely on unmounting and remounting the component to copy the badge URL into state, employ the `getDerivedStateFromProps` lifecycle method.
4. Remove `prepareExamples` and `all-badge-examples`.
5. Rewrite the `$suggest` schema to harmonize with the service definition format. It's not backward-compatible which means at deploy time there probably will be 10–20 minutes of downtime on that feature, between the first server deploy and the final gh-pages deploy. 🤷♂️ (We could leave the old version in place if it seems worth it.)
6. Added two new functions in `make-badge-url` with tests. I removed _most_ of the uses of the old functions, but there are some in parts of the frontend I didn't touch like the static and dynamic badge generators, and again I didn't want to make this any larger than it already is.
7. Fix a couple bugs in the service-definition export.
This starts the rewrite of the dynamic badges. I've pulled into BaseService an initial version of the query param validation from #2325.
I've extended from BaseJsonService to avoid duplicating the deserialization logic, though it means there is a bit of duplicated code among the three dynamic services. The way to unravel this would be to move the logic from `_requestJson` and friends from the base classes into functions so DynamicJson can inherit from BaseDynamic. Would that be worth it?
This introduces a regression of #1446 for this badge.
Close#2345
Three main goals:
1. In the front end:
a. Show form fields and automatically assemble badge URLs (#701)
c. Group together examples for the same service
b. Show deprecated services
2. Make it easy to changing the schema of `examples`, thanks to 100% validation. One challenge with frameworks is that when there are typos things fail silently which is pretty unfriendly to developers. The validation should really help with that. (This caught one bug in AUR, though I fixed it in #2405 which landed first.)
3. Produce a service definition export for external tool builders. (#776)
4. Build toward harmony between the front-end data structure and the `examples` key in the service classes. I aliased `staticPreview` to `staticExample` which starts this process.
The old format:
- Lacked a consistent machine-readable representation of the fields.
- Flattened multiple examples for the same service were flattened.
- Excluded deprecated services.
The new format improves a few things, too:
- It cleans up the naming. Since this file evolved over time, the names were a bit muddled (i.e. what was an example vs a preview).
- It duplicated information (like `.svg`). (I can imagine dropping the `.svg` from our badge URLs someday, which would make the URLs easier to read and maintain.)
- For a human reading the YAML file, providing the static example as a deconstructed object is more readable.
Here are a couple snippets:
```yml
- category: build
name: AppVeyorCi
isDeprecated: false
route:
format: '([^/]+/[^/]+)(?:/(.+))?'
queryParams: []
examples:
- title: AppVeyor
example: {path: /appveyor/ci/gruntjs/grunt, queryParams: {}}
preview: {label: build, message: passing, color: brightgreen}
keywords: []
- title: AppVeyor branch
example: {path: /appveyor/ci/gruntjs/grunt/master, queryParams: {}}
preview: {label: build, message: passing, color: brightgreen}
keywords: []
- category: downloads
name: AmoDownloads
isDeprecated: false
examples:
- title: Mozilla Add-on
example: {path: /amo/d/dustman, queryParams: {}}
preview: {path: /amo/d/dustman, queryParams: {}}
keywords: [amo, firefox]
```
There's a lot going on in this PR, though it's all interdependent, so the only way I can see to break it up into smaller pieces would be serially.
1. I completely refactored the functions for managing cache headers. These have been added to `services/cache-headers.js`, and in some ways set the stage for the rest of this PR.
- There are ample higher-level test of the functionality via `request-handler`. Refactoring these tests was deferred. Cache headers were previously dealt with in three places:
- `request-handler.js`, for the dynamic badges. This function now calls `setCacheHeaders`.
- `base-static.js`, for the static badges. This method now calls the wordy `serverHasBeenUpSinceResourceCached` and `setCacheHeadersForStaticResource`.
- The bitFlip badge in `server.js`. 👈 This is what set all this in motion. This badge has been refactored to a new-style service based on a new `NoncachingBaseService` which does not use the Shields in-memory cache that the dynamic badges user.
- I'm open to clearer names for `NoncachingBaseService`, which is kind of terrible. Absent alternatives, I wrote a short essay of clarification in the docstring. 😝
2. In the process of writing `NoncachingBaseService`, I discovered it takes several lines of code to instantiate and invoke a service. These would be duplicated in three or four places in production code, and in lots and lots of tests. I kept the line that goes from regex to namedParams (for reasons) and moved the rest into a static method called `invoke()`, which instantiates and invokes the service. This _replaced_ the instance method `invokeHandler`.
- I gently reworked the unit tests to use `invoke` instead of `invokeHandler`– generally for the better.
- I made a small change to `BaseStatic`. Now it invokes `handle()` async as the dynamic badges do. This way it could use `BaseService.invoke()`.
3. There was logic in `request-handler` for processing environment variables, validating them, and setting defaults. This could have been lifted whole-hog to `services/cache-headers.js`, though I didn't do that. Instead I moved it to `server-config.js`. Ideally `server-config` is the only module that should access `process.env`. This puts the defaults and config validation in one place, decouples the config schema from the entire rest of the application, and significantly simplifies our ability to test different configs, particularly on small units of code. (We were doing this well enough before in `request-handler.spec`, though it required mutating the environment, which was kludgy.) Some of the `request-handler` tests could be rewritten at a higher level, with lower-level data-driven tests directly against `cache-headers`.
These tests clearly should be refactored, though I’d like to get this fix in, in advance of a more involved refactor that shifts most of the responsibility away from this function. Maybe we can even eliminate at least one of these cases in the meantime.
There's a lot of demand for the Gitlab badges (#541) and the PR has been lingering, so I thought I'd start off one of the simple ones as a new-style service. This one is SVG-based, so it shouldn't require the API-token logic which could use some more testing and will require us to create an app and configure it on our server.
We don't have any validation in place for `queryParams`. Probably this should be added to BaseService, though for the time being I extracted a helper function.
Thanks to @LVMBDV for getting this work started in #1838!
This continues the work from #2279, by allowing example badges to be specified using `namedParams`. Using an object makes it possible for us to display these in form fields down the line. (#701)
I've called this the "preferred" way, and labeled the other ways deprecated. I've also added some doc to the `examples` property in BaseService. Then I realized we had some doc in the tutorial, though I think it's fine to have a short version in the tutorial, and the gory detail in BaseService.
I've also added a `pattern` keyword, and made `urlPattern` an alias.
Closes#2050.
The term “url” is overloaded in services, to refer to the Shields route and also the API URL. Calling the Shields URL a “route” is on the whole more descriptive, and makes it clearer and more obvious which one of these we’re talking about. It’s a small thing, though seems like an improvement.
We have a few functions called `buildUrl`. I’ve renamed them to `buildRoute` when they refer to routes, and left them as `buildUrl` when they refer to API URLs.
I included a minor style tweak and some formatting cleanup in `TUTORIAL.md`.
I went down a rabbit hole while trying to untangle the bug in the dockbit and bitrise examples https://github.com/badges/shields/pull/2234#pullrequestreview-169997546.
The URL generation code is spaghetti-like, with functions, many of which I wrote, with opaque names, doing similar but not identical things, and making slightly incompatible assumptions about the way query strings are handled.
I got a bit lost and need to take a step back.
Meanwhile, this is a small piece of work I did that’s worth keeping. It doesn’t scratch the surface of the tangle, but it does remove a bit of duplication.
It also makes a minor stylistic ES6 change in the handling of default arguments.
Ref: #2027
I've rewritten the deprecated services using the `deprecatedService` helper from #1922. I added a test for `Deprecated`, and for `enforceDeprecation`, which isn't being used right now, but is there for future use.
This also makes it possible to write services using BaseService which do not have any named parameters (with a test).
Ref: #1358
This continues a consistency update we’ve been making to standardize on URL based on a recommendation from WHATWG: https://url.spec.whatwg.org/#goals
This also helps with copying and pasting between all-badge-examples and new-style services, where it’s otherwise easy to make a mistake.
Ref: #1322#1341
* allow service classes to define a static example
* define static example for some services
(apm, appveyor, cdnjs, clojars, gem, librariesio, npm, uptimerobot)
* add/update tests
This allows us to show an example without making an API call to a live service for better performance.
We can now specify 3 fields in the example definition:
* urlPattern for the version with placeholders e.g: /npm/dw/:package.svg
* ExampleUrl/Uri for the concrete example e.g: /npm/dw/localeval.svg
* PreviewUrl/Uri for the static (or live) image we will actually show
- Log the response when using `test:services:trace`
- Fully log large nested objects
Since the `badgeData` is in a different format from the JSON response, and also doesn't include the title, including this output is helpful. It makes it clearer what the Joi matchers are trying to match.
Sometimes when there's a deep nested structure, it's helpful or necessary to see the entire thing.
We use arrow functions in most places; this enforces it.
Passing arrow functions to Mocha is discouraged: https://mochajs.org/#arrow-functions
This was a mix of autofixes and hand adjustments.
* InvalidParameter: New error type
* Return inaccessible for 5xx errors from services
* Add test for Inaccessible on 5xx
* Add tests for named error types
When JSON responses come back, they are sometimes not in the format expected by the API. As a result we have a lot of defensive coding (expressions like `(data || {}).someProperty`) to avoid exceptions being thrown in badge processing. Often we rely on the `try` blocks that wrap so much of the badge-processing code, which catch all JavaScript exceptions and return some error result, usually **invalid**. The problem with this is that these `try` blocks catch all sorts of programmer errors too, so when we see **invalid** we don't know whether the API returned something unexpected, or we've made a mistake. We also spend a lot of time writing defensive tests around malformed responses, and creating and maintaining the defensive coding.
A better solution is to validate the API responses using declarative contracts. Here the programmer says exactly what they expect from the API. That way, if the response isn't what we expect we can just say it's an **invalid json response**. And if our code then throws an exception, well that's our mistake; when we catch that we can call it a **shields internal error**. It's also less code and less error-prone. Over time we may be confident enough in the contracts that we won't need so many tests of malformed responses. The contract doesn't need to describe the entire response, only the part that's needed. Unknown keys can simply be dropped, preventing unvalidated parts of the response from creeping into the code. Checking what's in our response before calling values on it also makes our code more secure.
I used Joi here, since we're already using it for testing. There may be another contracts library that's a better fit, though I think we could look at that later.
Those changes are in base.js.
The rest is a rewrite of the remaining NPM badges, including the extraction of an NpmBase class. Inspired by @chris48s's work in #1740, this class splits the service concerns into fetching, validation, transformation, and rendering. This is treated as a design pattern. See the PR discussion for more. There are two URL patterns, one which allows specifying a tag (used by e.g. the version badge `https://img.shields.io/npm/v/npm/next.svg`), and the other which does not accept a tag (e.g. the license badge `https://img.shields.io/npm/l/express.svg`). Subclasses like NpmLicense and NpmTypeDefinitions can specify the URL fragment, examples, the validation schema for the chunk of the package data they use, and a render function. The NpmVersion subclass uses a different endpoint, so it overrides the `handle` implementation from NpmBase.
The remaining services using BaseJsonService are shimmed, so they will keep working after the changes.
Make a clear distinction between programmer errors ("internal errors") and runtime errors, and allow configuring the server to let the programmer errors bubble up in development and unit testing. This saves a huge amount of time because it generates ordinary stack traces when things go wrong. And, if these errors occur in production, we'll catch them, and display **shields | internal error** which is the equivalent of a 500 error.
Instead of centralizing examples, specify them from within a service.
* Avoid duplication in service loading + refactor
* Avoid duplication in URLs, rename uri -> url in BaseService
This merges the `node-8` branch. The heavy lift was by @Daniel15 with refactoring from me and a patch by @RedSparr0w.
* New API for registering services (#963)
* Disable Node 6 tests on node-8 branch (#1423)
* BaseService: Factor out methods _regex and _namedParamsForMatch (#1425)
- Adjust test grouping
- Rename data -> queryParams, text -> message
* BaseService tests: Use Chai (#1450)
* BaseService: make serviceData and badgeData explicit and declarative (#1451)
* fix isValidStyle test (#1544)
* Run tests in Node 9, not Node 6 (#1543)
