3
\$\begingroup\$

I'm rewriting collection-json.js, a client for Collection+JSON. (Collection+JSON is a HATEAOS API media type built on top of JSON.)

My Goals:

  • Remove CoffeeScript
  • webpack
  • UMD so it can be used via node and via <script ...> (not there yet.)

I'm more comfortable in Ruby, though I do know JS. Still, I wouldn't say it's my native tongue.

The current challenge is the 'http' file. I have the actual HTTP calls in one module, and then the HTTP library in another. We use different abstraction patters but I believe the original author's intent was to allow the insertion of other network layers into the HTTP client, which would then allow for things like using jquery's methods or, perhaps more importantly, for inserting test-related stubs.

Here's my current code:

import fetch from 'node-fetch';

let content_type = "application/vnd.collection+json";

class HttpImplementation {

  // http_callback is used by each of the verb-methods below.
  // It should call `done` with (error_string_or_null, body_as_text,
  // response_headers)
  //
  http_callback(response, done) {
    if(response.status < 400) {
      // strangely, response.text() returns a promise...
      response.text().then((actual_response) => {
        done(null, actual_response, response.headers)
      })
    }
    else { done(response.status); }
  }

  get(href, options, done) {
    fetch(href).then((response) => {this.http_callback(response, done)}).
      catch(error => done(error));
  };

  post(href, options, done) {
    http_options = Object.assign({}, options,
        {
             method: "POST",        body: JSON.stringify(options.body),
            headers: options.headers,
           redirect: "follow",      mode: "CORS",
        credentials: "same-origin", referrer: "client",
        })

    return(fetch(href, http_options).then(this.http_callback, done));
  };

  put(href, options, done) {
    http_options = Object.assign({}, options,
      {
           method: "PUT",        body: JSON.stringify(options.body),
          headers: headers,
         redirect: "follow",      mode: "CORS",
      credentials: "same-origin", referrer: "client",
      })

    return(fetch(href, http_options).then(this.http_callback, done));
  }

  delete(href, options, done){
    http_options = Object.assign({}, options,
        {
             method: "DELETE",        body: JSON.stringify(options.body),
            headers: headers,
           redirect: "follow",      mode: "CORS",
        credentials: "same-origin", referrer: "client",
        })

    return(fetch(href, http_options).then(http_callback, done));
  };
}

class InterfaceLayer {
  constructor( network_layer )
  {
    if( network_layer == null || network_layer == undefined )
    { this.source = new HttpImplementation(); }
    else
    { this.source = network_layer ; }
  }

  get(href, options, done) {
    if(!options.headers) { options.headers = {} }
    options.headers["accept"] = content_type;

    this.source.get(href, options, function(error, collection, headers) {
      // the previous implementation of this was async, so
      // it might bear reworking once figured out
      // seems like you could add a layer that would try each one
      // and, on failure, try the next.

      if( error || collection )
      {
        // if a collection is returned, the source should handle caching
        // though in the original code...
        // https://github.com/collection-json/collection-json.js/blob/master/lib/http.coffee#L49
        return(done(error, collection, headers));
      }
      else
      {
        return(false);
      }
    })
  }

  post(href, options, done) {
    if (options == null) { options = {}; }
    if (!options.headers) { options.headers = {}; }

    options.headers = Object.assign({}, options.headers, {
      "accept": content_type,
      "content-type": content_type,
      "Content-Type": "application/collection+json; charset=utf-8"
    })

    this.source.post(href, options, function(error, collection, headers)
    {
      if( error || collection )
      {
        // if a collection is returned, the source should handle caching
        // though in the original code...
        // https://github.com/collection-json/collection-json.js/blob/master/lib/http.coffee#L49
        return(done(error, collection, headers));
      }
      else
      {
        return(false);
      }
    })
  }

  put(href, options, done) {
    if (options == null) { options = {}; }
    if (!options.headers) { options.headers = {}; }

    options.headers = Object.assign({}, options.headers, {
      "accept": content_type,
      "content-type": content_type,
      "Content-Type": "application/collection+json; charset=utf-8"
    })

    this.source.put(href, options, function(error, collection, headers)
    {
      if( error || collection )
      {
        // if a collection is returned, the source should handle caching
        // though in the original code...
        // https://github.com/collection-json/collection-json.js/blob/master/lib/http.coffee#L49
        return(done(error, collection, headers));
      }
      else
      {
        return(false);
      }
    })
  }

  delete(href, options, done) {
    if (options == null) { options = {}; }
    if (!options.headers) { options.headers = {}; }

    options.headers = Object.assign({}, options.headers, {
      "accept": content_type,
      "content-type": content_type,
      "Content-Type": "application/collection+json; charset=utf-8"
    })

    this.source.delete(href, options, function(error, collection, headers)
    {
      if( error || collection )
      {
        // if a collection is returned, the source should handle caching
        // though in the original code...
        // https://github.com/collection-json/collection-json.js/blob/master/lib/http.coffee#L49
        return(done(error, collection, headers));
      }
      else
      {
        return(false);
      }
    })
  }
}

export default (new InterfaceLayer())

Sample usage in the form of a 'live test':

import Client from '../src/client.js'

describe("LIVE test from ACTUAL API", () => {
  test("it gets an error when calling an invalid path", () => {
    new Client('http://todo-collection-json.herokuapp.com/invalid',
      (error, collection) => {
        expect(error).toBe(404);
        expect(collection).toBeUndefined();
      });
  })

  test('it works with a real-life-ish example', () => {
    expect.assertions();

    new Client('http://todo-collection-json.herokuapp.com/collection/tasks/',
      (error, collection) => {
        expect(collection).not.toBeDefined();
        console.log(collection);
        expect(error).toBeNull();
      });
  })
})

Original http.coffee:

module.exports = defaults =
  _get: (href, options, done=()->)->
    done new Error "'GET' not implemented"

  _post: (href, options, done=()->)->
    done new Error "'POST' not implemented"

  _put: (href, options, done=()->)->
    done new Error "'PUT' not implemented"

  _del: (href, options, done=()->)->
    done new Error "'DELETE' not implemented"

  cache:
    # This is a fake cache. You should probably add a real one...
    put: (key, value, time, done=()->)->
      if typeof time is 'function'
        done = time
        time = null
      done()
    del: (key, done=()->)->
      done()
    clear: (done=()->)->
      done()
    get: (key, done=()->)->
      done()
    size: (done=()->)->
      done 0
    memsize: (done=()->)->
      done 0
    debug: ()->
      true

module.exports["content-type"] = "application/vnd.collection+json"

module.exports.get = (href, options, done)->
  defaults.cache.get href, (error, collection)->
    # Give them the cached stuff if we have it
    return done error, collection if error or collection

    options.headers ||= {}
    options.headers["accept"] = module.exports["content-type"]

    module.exports.setOptions href, options

    defaults._get href, options, (error, collection, headers)->
      return done error if error
      performCache collection, headers
      done error, collection

module.exports.post = (href, options={}, done)->

  options.headers ||= {}
  options.headers["accept"] = module.exports["content-type"]
  options.headers["content-type"] = module.exports["content-type"]

  module.exports.setOptions href, options

  defaults._post href, options, (error, collection, headers)->
    # Do we cache this?
    done error, collection

# Should be overridden by the client
module.exports.setOptions = (href, options)->

performCache = (collection, headers)->
  # Expires
  # expires = response.headers.expires
  # # TODO convert to time-from-now
  # # Cache-Control
  # # TODO implement
  # defaults.cache.put response.request.href, response.body, new Date(expires).getTime() if expires

Original index.coffee:

request = require "request"
http = require "./http"

http._get = (href, options, done)->
  options.url = href
  request options, (error, response, body)->
    done error, body, response?.headers

http._post = (href, options, done)->
  options.url = href
  options.body = JSON.stringify options.body
  options.method = "POST"
  request options, (error, response, body)->
    done error, body, response?.headers

http._put = (href, options, done)->
  options.url = href
  options.body = JSON.stringify options.body
  request.put options, (error, response)->
    done error, response

http._del = (href, options, done)->
  options.url = href
  request.del options, (error, response)->
    done error, response

module.exports = require "./client"

Questions

  • Is this a good way to separate these two concerns?
  • Is there a much cleaner/simpler way to express all this?

I'm open to input on any part of the code, but especially this http aspect.

\$\endgroup\$
1
  • \$\begingroup\$ We've just taken it off hold. \$\endgroup\$
    – Mast
    Dec 12, 2018 at 18:41

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Browse other questions tagged or ask your own question.