Rewrite staticBrowserREST.js

[rrthomas]

This rewrite is in two parts. The first can be accepted without the second.

1. Factor out the request logic into two functions. This gives considerable simplification.
2. However, once this simplification has been achieved, it seems that using obtainJS is overkill for this module, so the second part removes it, reducing the size of the module considerably.

[rrthomas]

Note that as I was preparing part 2 I noticed quite a few mistakes in part 1. I suggest therefore that if only part 1 is accepted, I should fix it (I have a list of fixes); and if part 2 is accepted, then the two commits should be squashed (that would save my having to fix part 1).

[rrthomas]

Some comments on the impact of this commit:

1. It restricts the API of the module to a subset of the "normal" obtainJS API, but I think it's perfectly adequate to be the "official" API of the IO module.
2. It requires Promises. I don't see this as a problem, since this code only runs in the browser. If it were a problem, it could be translated with regenerator.

And some further thoughts:

1. The same techique could be used on the Node back-end. This would also give a good indication of how general the technique is, i.e., whether it's worth considering a lighterweight version of ObtainJS more generally. My feeling is that it may well be, but it only works for collections of API which are "the same shape", so they can be collected into a single generic synchronous and asynchronous method.
2. With ES7's await and async, or a similar solution (e.g. wait.for, which has ES5 and ES6 versions), it would be possible to use a single generic method (because one can then turn an async method into a synchronous method).

[graphicore]

1.) I don't get why you suggest reimplementing an API incompletely . Some days ago you argued that it would be better to have a module than to reimplement a four-line pattern over and over because of different reasons. (I can look these up on my other computer, if you can't remember, it was about the memoize module.) What holds you back from writing a _obtainify method? You get the obtain interface for free. Having the same interface for all these methods with async/sync execution models is a good thing. Did you see the last example that I sent you on Friday night via the chat? It had a unified handler for the async and the sync case, so errors like if(request.status < 200 || request.status > 209) would only happen at one place, speaking of DRY.

2.) Just use the Promise implementation of obtainJS it works, it's ES6 compatible, we already use it, it's a third party package, so we don't have to care, it uses the original Promise if available.

3.) This is partly beginning to be obfuscation, I don't see the value in trying to write the shortest possible implementation. e.g. _p.ensureDir = _dirify(_p._writeFile); this is only short but not very helpful when I try to understand the semantics. Also, how does it not throw a generic IOError for the 405 Method Not Allowed code if the dir already exists?

4.) You do generalizations that are not acceptable when we want to use HTTP strictly. E.g:

if(request.status < 200 || request.status > 204)
    error = _errorFromRequest(request);

If a server answers 201 Created on a GET request, we shouldn't accept this, because the Server behaves not like we expect it. Also 202 Accepted has implications that are important to consider, so if we are going to use that status code, it will have another meaning than 200. I think it's important to be able to define what a response means in respect to the request that was performed. The other occasion if(request.status < 200 || request.status > 209) is probably an error, so I'm not asking about it.

5.) A lighterweight version of ObtainJS: Ok, but let's keep the API in sync. We could just extend ObtainJS to provide a lightweight implementation for simple cases.

6.) If using ES 6/7 now means we need more time to get the code running it's a bad idea. Almost everything we want to do can be done in ES 5 already. Until ES 7 features are reasonable easy to use, we shouldn't do it. I think, if we are still working on this project when ES 7 is around the corner, we may want to clean up anyway. Either by removing all the shims we needed to make the code look like ES 7 or by rewriting portions of it to make it more current.

[rrthomas]

1. I argued a few days ago it was better to have a module than reimplement a 4-line pattern. Here, I argue that it is better not to use a module, because it saves 200 lines of code. In both cases, I am trying to reduce the amount of code.

Your _obtainify idea sounds like a good one. I wish I had thought of it!

1. OK.
2. Agreed.
3. It seems then that there should be a list of acceptable response statuses for each method, with a default of [200].
4. If I can write an _obtainify that keeps the module as short as at present, I'm done. No need to simplify ObtainJS.
5. I was not suggesting we do this now.

It sounds as though the best thing is to write _obtainify and fix the problems with status codes that you mentioned.

[graphicore]

it is quite possible that we are not doing everything correct, regarding the response codes at the moment, so it should be possible to fix these things later easily.

[rrthomas]

I've "obtainified" the code in another commit. If it's accepted, I'll squash it before merging.

[graphicore]

I think in the _dirify the actual path-decorator is not called but instead itself used as the new path:

https://github.com/rrthomas/ufoJS/blob/master/lib/tools/io/staticBrowserREST.js#L52

 // We signal a directory to the REST endpoint by adding a / suffix
var _dirify = function (f) {
return function (async, path, data) {
return f(async, function(path) { return uri + (uri.slice(-1) !== '/' ? '/' : ''); }, data);
}
};

[rrthomas]

About _dirify, it just goes to show how little of the code needs to be working to start the red-pill. Fixed.

[graphicore]

I rewrote this PR, it's in https://github.com/graphicore/ufoJS/tree/rewrite-io-static-browser-rest. We can make a PR from that or you pull it into this branch.

About:

it works for both sync and async cases

you need to run the testsuite in the browser, it covers far more cases than starting metapolator red-pill:

• go to your ufojs base directory
• run $ ./serve.sh
• goto http://localhost:8080 in your browser
• click Testsuite
• see tests fail because of different reasons (results in timeouts etc.)

[graphicore]

I just rebased https://github.com/graphicore/ufoJS/tree/rewrite-io-static-browser-rest on top of this branch, so you should easily be able to merge it