Refactor [Wordpress] services (#3324)

- Prefer inline transforms to take place in `handle()` rather than `render()` - Avoid inversion of control by removing `BaseWordpress#handle()`, passing `extensionType` into `fetch()`, and removing one layer of subclassing - Move “not found” checks into `fetch()` - Cache wordpress versions instead of fetching on each request - Start to convert aliases to redirects (there are more of these which could be tackled in a follow-on) - Replace at least one route `format` with a `pattern` (ref #3329) - Partially reorder: name, category, route, examples, defaultBadgeData, render, fetch, handle Some of this is in line with our established patterns or makes it clearly easier to follow; some of it is arguably stylistic.
badges · Apr 22, 2019 · 5828223 · 5828223
1 parent 05af1f8
commit 5828223
Show file tree

Hide file tree

Showing 9 changed files with 205 additions and 168 deletions.
diff --git a/services/wordpress/wordpress-base.js b/services/wordpress/wordpress-base.js
@@ -21,20 +21,14 @@ const notFoundSchema = Joi.string().allow(null, false)
 const schemas = Joi.alternatives(foundSchema, notFoundSchema)
 
 module.exports = class BaseWordpress extends BaseJsonService {
-  static get extensionType() {
-    throw new Error(`extensionType() function not implemented for ${this.name}`)
-  }
-
-  async fetch({ slug }) {
-    const url = `https://api.wordpress.org/${
-      this.constructor.extensionType
-    }s/info/1.1/`
-    return this._requestJson({
+  async fetch({ extensionType, slug }) {
+    const url = `https://api.wordpress.org/${extensionType}s/info/1.1/`
+    const json = await this._requestJson({
       url,
       schema: schemas,
       options: {
         qs: {
-          action: `${this.constructor.extensionType}_information`,
+          action: `${extensionType}_information`,
           request: {
             slug,
             fields: {
@@ -51,15 +45,9 @@ module.exports = class BaseWordpress extends BaseJsonService {
         },
       },
     })
-  }
-
-  async handle({ slug }) {
-    const json = await this.fetch({ slug })
-
     if (!json) {
       throw new NotFound()
     }
-
-    return this.constructor.render({ response: json })
+    return json
   }
 }
diff --git a/services/wordpress/wordpress-downloads.service.js b/services/wordpress/wordpress-downloads.service.js
@@ -29,13 +29,21 @@ function DownloadsForExtensionType(extensionType) {
       return `Wordpress${capt}Downloads`
     }
 
-    static render({ response }) {
+    static render({ downloads }) {
       return {
-        message: metric(response.downloaded),
-        color: downloadCount(response.downloaded),
+        message: metric(downloads),
+        color: downloadCount(downloads),
       }
     }
 
+    async handle({ slug }) {
+      const { downloaded: downloads } = await this.fetch({
+        extensionType,
+        slug,
+      })
+      return this.constructor.render({ downloads })
+    }
+
     static get category() {
       return 'downloads'
     }
@@ -50,16 +58,13 @@ function DownloadsForExtensionType(extensionType) {
         pattern: ':slug',
       }
     }
-    static get extensionType() {
-      return extensionType
-    }
 
     static get examples() {
       return [
         {
           title: `Wordpress ${capt} Downloads`,
           namedParams: { slug: exampleSlug },
-          staticPreview: this.render({ response: { downloaded: 200000 } }),
+          staticPreview: this.render({ downloads: 200000 }),
         },
       ]
     }
@@ -74,21 +79,25 @@ function InstallsForExtensionType(extensionType) {
       return `Wordpress${capt}Installs`
     }
 
-    static get extensionType() {
-      return extensionType
-    }
-
     static get category() {
       return 'downloads'
     }
 
-    static render({ response }) {
+    static render({ installCount }) {
       return {
-        message: `${metric(response.active_installs)}+`,
-        color: downloadCount(response.active_installs),
+        message: metric(installCount),
+        color: downloadCount(installCount),
       }
     }
 
+    async handle({ slug }) {
+      const { active_installs: installCount } = await this.fetch({
+        extensionType,
+        slug,
+      })
+      return this.constructor.render({ installCount })
+    }
+
     static get defaultBadgeData() {
       return { label: 'active installs' }
     }
@@ -105,7 +114,7 @@ function InstallsForExtensionType(extensionType) {
         {
           title: `Wordpress ${capt} Active Installs`,
           namedParams: { slug: exampleSlug },
-          staticPreview: this.render({ response: { active_installs: 300000 } }),
+          staticPreview: this.render({ installCount: 300000 }),
         },
       ]
     }

diff --git a/services/wordpress/wordpress-platform-redirect.service.js b/services/wordpress/wordpress-platform-redirect.service.js
@@ -0,0 +1,13 @@
+'use strict'
+
+const { redirector } = require('..')
+
+module.exports = redirector({
+  category: 'platform-support',
+  route: {
+    base: 'wordpress/v',
+    pattern: ':slug',
+  },
+  transformPath: ({ slug }) => `/wordpress/plugin/wp-version/${slug}`,
+  dateAdded: new Date('2019-04-17'),
+})
diff --git a/services/wordpress/wordpress-platform-redirect.tester.js b/services/wordpress/wordpress-platform-redirect.tester.js
@@ -0,0 +1,10 @@
+'use strict'
+
+const t = (module.exports = require('../tester').createServiceTester())
+
+t.create('Plugin Tested WP Version (Alias)')
+  .get('/akismet.svg', {
+    followRedirect: false,
+  })
+  .expectStatus(301)
+  .expectHeader('Location', '/wordpress/plugin/wp-version/akismet.svg')
diff --git a/services/wordpress/wordpress-platform.service.js b/services/wordpress/wordpress-platform.service.js
@@ -1,49 +1,15 @@
 'use strict'
 
-const semver = require('semver')
-const Joi = require('joi')
 const { addv } = require('../text-formatters')
 const { version: versionColor } = require('../color-formatters')
-const { NotFound } = require('..')
 const BaseWordpress = require('./wordpress-base')
+const { versionColorForWordpressVersion } = require('./wordpress-version-color')
 
-const coreSchema = Joi.object()
-  .keys({
-    offers: Joi.array()
-      .items(
-        Joi.object()
-          .keys({
-            version: Joi.string()
-              .regex(/^\d+(\.\d+)?(\.\d+)?$/)
-              .required(),
-          })
-          .required()
-      )
-      .required(),
-  })
-  .required()
-
-class BaseWordpressPlatform extends BaseWordpress {
-  static get defaultBadgeData() {
-    return { label: 'wordpress' }
-  }
-  static get extensionType() {
-    return 'plugin'
-  }
-
-  static render({ response }) {
-    return {
-      message: addv(response.requires),
-      color: versionColor(response.requires),
-    }
-  }
-
+class WordpressPluginRequiresVersion extends BaseWordpress {
   static get category() {
     return 'platform-support'
   }
-}
 
-class WordpressPluginRequiresVersion extends BaseWordpressPlatform {
   static get route() {
     return {
       base: `wordpress/plugin/wp-version`,
@@ -56,59 +22,38 @@ class WordpressPluginRequiresVersion extends BaseWordpressPlatform {
       {
         title: 'Wordpress Plugin: Required WP Version',
         namedParams: { slug: 'bbpress' },
-        staticPreview: this.render({ response: { requires: '4.8' } }),
+        staticPreview: this.render({ wordpressVersion: '4.8' }),
       },
     ]
   }
-}
 
-class WordpressPluginTestedVersion extends BaseWordpressPlatform {
-  static render({ version, color }) {
+  static get defaultBadgeData() {
+    return { label: 'wordpress' }
+  }
+
+  static render({ wordpressVersion }) {
     return {
-      message: `${addv(version)} tested`,
-      color,
+      message: addv(wordpressVersion),
+      color: versionColor(wordpressVersion),
     }
   }
 
-  async fetchCore() {
-    const coreURL = 'https://api.wordpress.org/core/version-check/1.7/'
-    return this._requestJson({
-      url: coreURL,
-      schema: coreSchema,
+  async handle({ slug }) {
+    const { requires: wordpressVersion } = await this.fetch({
+      extensionType: 'plugin',
+      slug,
     })
+    return this.constructor.render({ wordpressVersion })
   }
+}
 
-  async handle({ slug }) {
-    const json = await this.fetch({ slug })
-    const core = await this.fetchCore()
-
-    if (!json || !core) {
-      throw new NotFound()
-    }
-
-    //Copy & Paste old color formatting code.
-    const versions = core.offers.map(v => v.version)
-    let testedVersion = json.tested
-    let color = ''
-    const svTestedVersion =
-      testedVersion.split('.').length === 2
-        ? (testedVersion += '.0')
-        : testedVersion
-    const svVersion =
-      versions[0].split('.').length === 2 ? (versions[0] += '.0') : versions[0]
-
-    if (
-      testedVersion === versions[0] ||
-      semver.gtr(svTestedVersion, svVersion)
-    ) {
-      color = 'brightgreen'
-    } else if (versions.indexOf(testedVersion) !== -1) {
-      color = 'orange'
-    } else {
-      color = 'yellow'
-    }
+class WordpressPluginTestedVersion extends BaseWordpress {
+  static get category() {
+    return 'platform-support'
+  }
 
-    return this.constructor.render({ version: testedVersion, color })
+  static get defaultBadgeData() {
+    return { label: 'wordpress' }
   }
 
   static get route() {
@@ -123,29 +68,42 @@ class WordpressPluginTestedVersion extends BaseWordpressPlatform {
       {
         title: 'Wordpress Plugin: Tested WP Version',
         namedParams: { slug: 'bbpress' },
-        staticPreview: this.render({ version: '4.9.8', color: 'brightgreen' }),
-        documentation: `<p>There is an alias for this badge. <code>wordpress/v/:slug.svg</code></p>`,
+        staticPreview: this.renderStaticPreview({
+          testedVersion: '4.9.8',
+        }),
       },
     ]
   }
-}
 
-class WordpressPluginTestedVersionAlias extends WordpressPluginTestedVersion {
-  static get route() {
+  static renderStaticPreview({ testedVersion }) {
+    // Since this badge has an async `render()` function, but `get examples()` has to
+    // be synchronous, this method exists. It should return the same value as the
+    // real `render()`.
     return {
-      base: `wordpress/v`,
-      pattern: ':slug',
+      message: `${addv(testedVersion)} tested`,
+      color: 'brightgreen',
     }
   }
 
-  //The alias is documented in the above class.
-  static get examples() {
-    return []
+  static async render({ testedVersion }) {
+    // Atypically, the `render()` function of this badge is `async` because it needs to pull
+    // data from the server.
+    return {
+      message: `${addv(testedVersion)} tested`,
+      color: await versionColorForWordpressVersion(testedVersion),
+    }
+  }
+
+  async handle({ slug }) {
+    const { tested: testedVersion } = await this.fetch({
+      extensionType: 'plugin',
+      slug,
+    })
+    return this.constructor.render({ testedVersion })
   }
 }
 
 module.exports = {
   WordpressPluginRequiresVersion,
   WordpressPluginTestedVersion,
-  WordpressPluginTestedVersionAlias,
 }
diff --git a/services/wordpress/wordpress-platform.tester.js b/services/wordpress/wordpress-platform.tester.js
@@ -5,8 +5,9 @@ const { ServiceTester } = require('../tester')
 const { isVPlusDottedVersionAtLeastOne } = require('../test-validators')
 
 const t = (module.exports = new ServiceTester({
-  id: 'wordpress',
+  id: 'WordpressPlatform',
   title: 'Wordpress Platform Tests',
+  pathPrefix: '/wordpress',
 }))
 
 t.create('Plugin Required WP Version')
@@ -23,13 +24,6 @@ t.create('Plugin Tested WP Version')
     message: Joi.string().regex(/^v\d+(\.\d+)?(\.\d+)? tested$/),
   })
 
-t.create('Plugin Tested WP Version (Alias)')
-  .get('/v/akismet.json')
-  .expectBadge({
-    label: 'wordpress',
-    message: Joi.string().regex(/^v\d+(\.\d+)?(\.\d+)? tested$/),
-  })
-
 const mockedQuerySelector = {
   action: 'plugin_information',
   request: {