javascript json API client - is network layer abstraction good? Why isn't response.text right?
up vote
0
down vote
favorite
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 coffee-script
- webpack, have a distribution
- Use ES6 classes but hopefully, through the dist, make it work for people who aren't using ES6.
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, which are also included below for the sake of permanence:
Original http and- My current version
My current version is, in principle, similar to what I found in the previous version... 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(response, done) {
// should call done with 'error, body-as-json, response headers'
if(response.status.ok || response.status < 400) {
done(null, response.text(), 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())
Most of this works. I believe thethough I'm also struggling with how to load this from the webpacked file, so it's a bit of a mess.
I have the following "Live" test, which actually hits an example Collection+JSON API. It shows that the code doesn't work. I'm fairly sure that it's because response.text() in done(null, response.text(), response.headers)
shows a promise or something other than the response text I'd expect.
Here's the 'live' test:
import Client from '../src/client.js'
describe.skip("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 Coffee Script:
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
//=====================================================================
// in another file, lib/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"
I typically prefer TDD. But since I'm sort of out on a limb here, I'm struggling to just... figure out what I even what it to look like, so it's more "guess-driven development" more than anything.
- Is this a good way to separate these two concerns
- Why isn't response.text() returning ... text?
- 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. If I can get this code away from coffee-script it will be used. At least by one team, but I suspect by many more. Thank you for your time.
javascript json collections client
add a comment |
up vote
0
down vote
favorite
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 coffee-script
- webpack, have a distribution
- Use ES6 classes but hopefully, through the dist, make it work for people who aren't using ES6.
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, which are also included below for the sake of permanence:
Original http and- My current version
My current version is, in principle, similar to what I found in the previous version... 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(response, done) {
// should call done with 'error, body-as-json, response headers'
if(response.status.ok || response.status < 400) {
done(null, response.text(), 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())
Most of this works. I believe thethough I'm also struggling with how to load this from the webpacked file, so it's a bit of a mess.
I have the following "Live" test, which actually hits an example Collection+JSON API. It shows that the code doesn't work. I'm fairly sure that it's because response.text() in done(null, response.text(), response.headers)
shows a promise or something other than the response text I'd expect.
Here's the 'live' test:
import Client from '../src/client.js'
describe.skip("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 Coffee Script:
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
//=====================================================================
// in another file, lib/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"
I typically prefer TDD. But since I'm sort of out on a limb here, I'm struggling to just... figure out what I even what it to look like, so it's more "guess-driven development" more than anything.
- Is this a good way to separate these two concerns
- Why isn't response.text() returning ... text?
- 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. If I can get this code away from coffee-script it will be used. At least by one team, but I suspect by many more. Thank you for your time.
javascript json collections client
add a comment |
up vote
0
down vote
favorite
up vote
0
down vote
favorite
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 coffee-script
- webpack, have a distribution
- Use ES6 classes but hopefully, through the dist, make it work for people who aren't using ES6.
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, which are also included below for the sake of permanence:
Original http and- My current version
My current version is, in principle, similar to what I found in the previous version... 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(response, done) {
// should call done with 'error, body-as-json, response headers'
if(response.status.ok || response.status < 400) {
done(null, response.text(), 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())
Most of this works. I believe thethough I'm also struggling with how to load this from the webpacked file, so it's a bit of a mess.
I have the following "Live" test, which actually hits an example Collection+JSON API. It shows that the code doesn't work. I'm fairly sure that it's because response.text() in done(null, response.text(), response.headers)
shows a promise or something other than the response text I'd expect.
Here's the 'live' test:
import Client from '../src/client.js'
describe.skip("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 Coffee Script:
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
//=====================================================================
// in another file, lib/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"
I typically prefer TDD. But since I'm sort of out on a limb here, I'm struggling to just... figure out what I even what it to look like, so it's more "guess-driven development" more than anything.
- Is this a good way to separate these two concerns
- Why isn't response.text() returning ... text?
- 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. If I can get this code away from coffee-script it will be used. At least by one team, but I suspect by many more. Thank you for your time.
javascript json collections client
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 coffee-script
- webpack, have a distribution
- Use ES6 classes but hopefully, through the dist, make it work for people who aren't using ES6.
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, which are also included below for the sake of permanence:
Original http and- My current version
My current version is, in principle, similar to what I found in the previous version... 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(response, done) {
// should call done with 'error, body-as-json, response headers'
if(response.status.ok || response.status < 400) {
done(null, response.text(), 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())
Most of this works. I believe thethough I'm also struggling with how to load this from the webpacked file, so it's a bit of a mess.
I have the following "Live" test, which actually hits an example Collection+JSON API. It shows that the code doesn't work. I'm fairly sure that it's because response.text() in done(null, response.text(), response.headers)
shows a promise or something other than the response text I'd expect.
Here's the 'live' test:
import Client from '../src/client.js'
describe.skip("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 Coffee Script:
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
//=====================================================================
// in another file, lib/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"
I typically prefer TDD. But since I'm sort of out on a limb here, I'm struggling to just... figure out what I even what it to look like, so it's more "guess-driven development" more than anything.
- Is this a good way to separate these two concerns
- Why isn't response.text() returning ... text?
- 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. If I can get this code away from coffee-script it will be used. At least by one team, but I suspect by many more. Thank you for your time.
import fetch from 'node-fetch';
let content_type = "application/vnd.collection+json";
class HttpImplementation {
http_callback(response, done) {
// should call done with 'error, body-as-json, response headers'
if(response.status.ok || response.status < 400) {
done(null, response.text(), 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())
import fetch from 'node-fetch';
let content_type = "application/vnd.collection+json";
class HttpImplementation {
http_callback(response, done) {
// should call done with 'error, body-as-json, response headers'
if(response.status.ok || response.status < 400) {
done(null, response.text(), 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())
import Client from '../src/client.js'
describe.skip("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();
});
})
})
import Client from '../src/client.js'
describe.skip("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();
});
})
})
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
//=====================================================================
// in another file, lib/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"
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
//=====================================================================
// in another file, lib/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"
javascript json collections client
javascript json collections client
asked 4 mins ago
MustModify
16315
16315
add a comment |
add a comment |
active
oldest
votes
active
oldest
votes
active
oldest
votes
active
oldest
votes
active
oldest
votes
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f209409%2fjavascript-json-api-client-is-network-layer-abstraction-good-why-isnt-respon%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown