Add support for matchlist-v2.2, remove matchhistory-v2.2, clean up un…

…it tests.
jwalton · Jan 3, 2016 · c8f8758 · c8f8758
1 parent b7e0cb2
commit c8f8758
Show file tree

Hide file tree

Showing 24 changed files with 798 additions and 663 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -0,0 +1,7 @@
+# v2.0.0
+
+* Breaking Change - Remove support for matchhistory-v2.2.
+* Add support for matchlist-v2.2.
+* If you want to use Promises, you no longer need to call the `blahBlahAsync` version of a command - instead callbacks
+  are now optional, and any function which takes a callback will return a Promise if no callback is supplied.
+* Bug fixes.
diff --git a/README.md b/README.md
@@ -19,7 +19,7 @@ Features
   * game-v1.3
   * lol-static-data-v1.2 (partial)
   * match-v2.2
-  * matchhistory-v2.2
+  * matchlist-v2.2
   * summoner-v1.4
   * team-v2.4
   * it's fairly easy to add other APIs - if you need something please raise an issue and I'll
@@ -62,7 +62,7 @@ var lolClient = lol.client({
     defaultRegion: 'na',
     cache: lol.redisCache({host: '127.0.0.1', port: 6379})
 };
-lolClient.getChampionByIdAsync(53, {champData: ['all']})
+lolClient.getChampionById(53, {champData: ['all']})
 .then(function (data) {
     console.log("Found ", data.name);
     lolClient.destroy();

diff --git a/docs/development.md b/docs/development.md
@@ -5,14 +5,13 @@ Language
 ========
 
 lol-js is written in [CoffeeScript](http://coffeescript.org/), using
-the [es6-promise](https://github.com/jakearchibald/es6-promise) promise polyfill.
+the [es6-promise](https://github.com/jakearchibald/es6-promise) Promise polyfill.
 
 Testing lol-js
 ===============
 
-Run `npm test` to run unit tests.  Tests are excuted directly from the CoffeeScript source files,
-as this makes for better stack traces when things go wrong.  Note the first test may take a while
-to run, as compiling streamline files is slow.
+Run `npm test` to run unit tests.  Tests are executed directly from the CoffeeScript source files,
+as this makes for better stack traces when things go wrong.
 
 Design Overview
 ===============
@@ -33,7 +32,7 @@ api = exports.api = {
 }
 
 exports.methods = {
-    getRecentGamesForSummonerAsync: (summonerId, options) ->
+    getRecentGamesForSummoner: pb.break (summonerId, options) ->
         ...
 }
 ```
@@ -49,11 +48,8 @@ The `api` object is also handy for using the `_makeUrl` helper function.
 `methods` is a hash of functions which will be mixed in to the `Client` class's prototype.  These
 methods can call into methods in the core `Client` class or even into methods defined in other APIs.
 
-For any method name that end is `Async`, `Client` will automatically have a method added to it
-without the `Async` suffix that accepts a callback as the last parameter, so in the above example
-the client will end up with a method called `getRecentGamesForSummonerAsync(summonerId, options)`
-which returns a promise, and `getRecentGamesForSummoner(summonerId, options, done)`, which will
-call `done(err, result)` with a result.
+Methods are generally written to return a Promise, and then passed through promise-breaker's `break`
+to make them accept an optional callback.
 
 Writing API Modules
 ===================

diff --git a/src/api/game.coffee b/src/api/game.coffee
@@ -1,6 +1,6 @@
 assert  = require 'assert'
 ld      = require 'lodash'
-{promiseToCb} = require '../utils'
+pb      = require 'promise-breaker'
 matchApi = require './match'
 
 api = exports.api = {
@@ -24,7 +24,7 @@ exports.methods = {
     # Returns a `{games, summonerId}` object.  If `options.asMatches` is specified, returns a
     # `{games, matches, summonerId}` object.
     #
-    getRecentGamesForSummonerAsync: (summonerId, options={}) ->
+    getRecentGamesForSummoner: pb.break (summonerId, options={}) ->
         # Since we're relying on other APIs, we assert here so that if those APIs change, we'll get
         # unit test failures if we don't update this method.
         assert.equal(matchApi.api.version, "v2.2", "match API version has changed.")
@@ -53,15 +53,15 @@ exports.methods = {
             else
                 # Fetch matches in parallel
                 return @Promise.all games.games.map (game) =>
-                    @recentGameToMatchAsync game, summonerId, {
+                    @recentGameToMatch game, summonerId, {
                         region,
                         matchOptions: if options.asMatches is true then null else options.asMatches
                     }
                 .then (matches) ->
                     games.matches = matches
                     games
 
-    # Converts a `game` from `getRecentGamesForSummoner()` into a match (as per `getMatchAsync()`).
+    # Converts a `game` from `getRecentGamesForSummoner()` into a match (as per `getMatch()`).
     #
     # This function may result in multiple calls to the Riot API, to load the match
     # details and to load details of all the summoners in the game.
@@ -72,9 +72,9 @@ exports.methods = {
     # * `game` - a game retrieved via `getRecentGamesForSummoner()`.
     # * `summonerId` - summoner the game was fetched for.
     # * `options.region` - Region where to retrieve the data.
-    # * `options.matchOptions` - options to pass to `getMatchAsync()`.
+    # * `options.matchOptions` - options to pass to `getMatch()`.
     #
-    recentGameToMatchAsync: (game, summonerId, options={}) ->
+    recentGameToMatch: pb.break (game, summonerId, options={}) ->
         matchOptions = if !options.matchOptions?
             {region: options.region}
         else
@@ -87,7 +87,10 @@ exports.methods = {
             summonerId
         }
 
-        return @getMatchAsync game.gameId, matchOptions
-
+        return @getMatch game.gameId, matchOptions
 
 }
+
+# Deprecated `Async` methods
+exports.methods.getRecentGamesForSummonerAsync = exports.methods.getRecentGamesForSummoner
+exports.methods.recentGameToMatchAsync = exports.methods.recentGameToMatch
diff --git a/src/api/lolStaticData.coffee b/src/api/lolStaticData.coffee
@@ -1,5 +1,5 @@
 ld = require 'lodash'
-{promiseToCb} = require '../utils'
+pb = require 'promise-breaker'
 
 api = exports.api = {
     fullname: "lol-static-data-v1.2",
@@ -27,7 +27,7 @@ exports.methods = {
     #   'altimages', 'blurb', 'enemytips', 'image', 'info', 'lore', 'partype', 'passive',
     #   'recommended', 'skins', 'spells', 'stats', 'tags'.
     #
-    getChampionsAsync: (options={}) ->
+    getChampions: pb.break (options={}) ->
         options = ld.defaults {}, options, {
             region: @defaultRegion,
             dataById: false
@@ -54,9 +54,9 @@ exports.methods = {
     # * `id` - the ID of the champion to retrieve.
     # * `options` are the same as for `getChampions()`, except that `dataById` cannot be specified.
     #
-    getChampionByIdAsync: (id, options={}) ->
+    getChampionById: pb.break (id, options={}) ->
         options = ld.extend {}, options, {dataById: true}
-        @getChampionsAsync(options)
+        @getChampions(options)
         .then (champions) -> champions.data[id]
 
     # Retrieve a champion using its key.
@@ -65,14 +65,14 @@ exports.methods = {
     # * `id` - the ID of the champion to retrieve.
     # * `options` are the same as for `getChampions()`, except that `dataById` cannot be specified.
     #
-    getChampionByKeyAsync: (key, options={}) ->
+    getChampionByKey: pb.break (key, options={}) ->
         options = ld.extend {}, options, {dataById: false}
-        @getChampionsAsync(options)
+        @getChampions(options)
         .then (champions) -> champions.data[key]
 
-    getChampionByNameAsync: (name, options={}) ->
+    getChampionByName: pb.break (name, options={}) ->
         options = ld.extend {}, options, {dataById: false}
-        @getChampionsAsync(options)
+        @getChampions(options)
         .then (champions) ->
             # First try the name as a key, because this is the fastest way to do this.
             answer = champions.data[name]
@@ -102,7 +102,7 @@ exports.methods = {
     #    inStore, into, maps, requiredChampion, sanitizedDescription, specialRecipe, stacks, stats,
     #    tags, tree
     #
-    getItemsAsync: (options={}) ->
+    getItems: pb.break (options={}) ->
         options = ld.defaults {}, options, {
             region: @defaultRegion,
             dataById: false
@@ -129,8 +129,8 @@ exports.methods = {
     # * `id` - the ID of the item to retrieve.
     # * `options` are the same as for `getItems()`.
     #
-    getItemByIdAsync: (id, options={}) ->
-        @getItemsAsync(options)
+    getItemById: pb.break (id, options={}) ->
+        @getItems(options)
         .then (objects) ->
             return objects.data[id]
 
@@ -140,7 +140,7 @@ exports.methods = {
     #
     # Parameters:
     # * `options.region` - Region from which to retrieve data.
-    getVersionsAsync: (options={}) ->
+    getVersions: pb.break (options={}) ->
         region = options?.region ? @defaultRegion
 
         requestParams = {
@@ -160,3 +160,12 @@ exports.methods = {
     teamNameToId: (teamName) ->
         if teamName.toLowerCase() is "blue" then 100 else 200
 }
+
+# Deprecated `Async` methods
+exports.methods.getChampionsAsync = exports.methods.getChampions
+exports.methods.getChampionByIdAsync = exports.methods.getChampionById
+exports.methods.getChampionByKeyAsync = exports.methods.getChampionByKey
+exports.methods.getChampionByNameAsync = exports.methods.getChampionByName
+exports.methods.getItemsAsync = exports.methods.getItems
+exports.methods.getItemByIdAsync = exports.methods.getItemById
+exports.methods.getVersionsAsync = exports.methods.getVersions
diff --git a/src/api/match.coffee b/src/api/match.coffee
@@ -1,8 +1,8 @@
 assert           = require 'assert'
 ld               = require 'lodash'
+pb               = require 'promise-breaker'
 summonerApi      = require './summoner'
 lolStaticDataApi = require './lolStaticData'
-{promiseToCb} = require '../utils'
 
 api = exports.api = {
     fullname: "match-v2.2",
@@ -25,7 +25,7 @@ exports.methods = {
     #
     # Returns a promise.
     #
-    getMatchAsync: (matchId, options={}) ->
+    getMatch: pb.break (matchId, options={}) ->
         options = ld.defaults {}, options, {
             region: @defaultRegion
             includeTimeline: false
@@ -49,7 +49,7 @@ exports.methods = {
         if options.players?
             requestOptions.preCache = (match) =>
                 return match if !match?
-                @populateMatchAsync match, options.players, options
+                @populateMatch match, options.players, options
                 .then -> return match
 
         @_riotRequestWithCache requestParams, cacheParams, requestOptions
@@ -75,7 +75,7 @@ exports.methods = {
     #
     # Returns promise - the number of participantIdentities that were filled in, via the callback.
     #
-    populateMatchAsync: (match, players, options={}) ->
+    populateMatch: pb.break (match, players, options={}) ->
         assert(ld.isArray(players), "'players' must be an array!")
 
         # If all participantIdentity objects are populated, we have nothing to do, so check this first.
@@ -85,7 +85,7 @@ exports.methods = {
         # the players are here before we call @_loadPlayers?  This could potentially save us
         # some API calls.
 
-        @_loadPlayersAsync players, options
+        @_loadPlayers players, options
         .then (playerData) ->
             populated = 0
             participantIdentitiesById = ld.indexBy match.participantIdentities, "participantId"
@@ -114,7 +114,8 @@ exports.methods = {
     # objects, return a collection of `{championId, teamId, summoner}` objects.  `summoner` will be
     # a Riot API object.  If there are any objects where the given champion or summoner cannot be
     # loaded, these results will be omitted from the returned data.
-    _loadPlayersAsync: (players, options={}) ->
+    _loadPlayers: (players, options={}) ->
+        client = this
         @Promise.resolve().then =>
             players.forEach (player) ->
                 if !player.summonerId? and !player.summonerName?
@@ -129,14 +130,14 @@ exports.methods = {
             # Fetch summoners by ID if available
             summonerIds = ld(players).filter('summonerId').map('summonerId').value()
             summonersByIdPromise = if summonerIds.length > 0
-                @getSummonersByIdAsync(summonerIds, options)
+                @getSummonersById(summonerIds, options)
             else
                 @Promise.resolve {}
 
             # Only pull data for summoners where we don't have an ID.
             summonerNames = ld(players).reject('summonerId').map("summonerName").value()
             summonersByNamePromise = if summonerNames.length > 0
-                @getSummonersByNameAsync(summonerNames, options)
+                @getSummonersByName(summonerNames, options)
             else
                 @Promise.resolve {}
 
@@ -149,7 +150,7 @@ exports.methods = {
                 return @Promise.all players.map (player) =>
                     summoner = null
 
-                    @Promise.resolve().then ->
+                    @Promise.resolve().then =>
                         summoner = if player.summonerId?
                             summonersById[player.summonerId]
                         else
@@ -160,10 +161,10 @@ exports.methods = {
                         else
                             # Use `getChampionByName()`, because it will always try to get by key first, but
                             # it is much more forgiving than `getChampionByKey()`.
-                            @getChampionByNameAsync(player.championKey, options)
+                            @getChampionByName(player.championKey, options)
                         )
 
-                    .then (champion) ->
+                    .then (champion) =>
                         if summoner? and champion?
                             return {
                                 summoner,
@@ -193,3 +194,7 @@ exports.methods = {
 
         return answer
 }
+
+# Deprecated `Async` methods
+exports.methods.getMatchAsync = exports.methods.getMatch
+exports.methods.populateMatchAsync = exports.methods.populateMatch
diff --git a/src/api/matchhistory.coffee b/src/api/matchhistory.coffee