JavaScript based REST interface client #1209

s-ludwig · 2015-08-03T20:26:39Z

No description provided.

Geod24 · 2015-08-03T22:44:07Z

Any ETA for when you want this ? I'll review by the end of this week. Though at first sight it looks like a there's a fair amount of duplication ?

s-ludwig · 2015-08-04T12:56:15Z

I have actually started to work on the duplication issue (which is already present even without this addition), but the current RestInterface struct intended for this is still not ready for CT generation, because it currently requires the RestInterfaceSettings for a lot of things. I'll work on that later.

I'll probably create a 0.7.24 branch during the next couple of days and start merging your remaining changes in vibe.web.rest. Then I'd do a larger refactoring, moving things out of the vibe.web.rest module and switching over to use RestInterface there, too.

s-ludwig · 2015-08-05T05:30:21Z

Cleaned up the commit history for review.

Geod24 · 2015-08-06T10:49:34Z

source/vibe/web/internal/rest/common.d

+	header,      // req.header[]
+	attributed,  // @before
+	internal     // req.params[]
+}


IIUC, internal is for path attribute, isn't it ? Maybe that'll be a more descriptive name. Also body parameters are not meant to stay JSON forever. On that topic: any way to check the server setting ? It's not a big deal, but if someone disable parseQueryString for example, there's no way to detect it.

It's for path attributes or any other attributes that are passed using req.params. Since there are valid uses outside of path placeholders, I decided against using that as the name. With the "JSON" comment I just wanted to describe the current state, since I'm not sure yet how to best handle multiple formats in terms of the internal architecture.

Currently there is no way to check the server settings. Really the only possibility that I see would be to throw an exception from inside HTTPServerRequest if the .json field is accessed without the parseJson setting. Another, probably better, path would be to switch the whole JSON body parsing to happen lazily and optionally strongly typed.

Geod24 · 2015-08-06T11:49:42Z

Hum, another P.R. blocked by 2.065 :(

Also, it looks like there's a discrepancy between the client and server with the refactoring:

REST route: GET /root/get_check/:param/:param2 []
add route GET /root/get_check/:param/:param2
[...]
no route match: GET /root/getCheck/one/two
No response written for /root/getCheck/one/two
REST call: GET http://127.0.0.1:8000/root/getCheck/one/two -> 404, 404

For the rest, the diff is large, so I'll probably have to try it out on my machine to be sure, but I like where it's heading at. I'm not that familiar with JS, but the new functionality looks cool 👍

s-ludwig · 2015-08-07T21:32:31Z

Fixed the inconsistency (c5d3208) and 2.065 compilation error (ed739a9).

s-ludwig · 2015-08-11T09:37:05Z

BTW, for xref purposes, RestInterface also makes things like #662 and #1070 an easy target.

s-ludwig · 2015-08-11T15:36:29Z

Rebased on master. I've also fixed the handling of optional parameters. Commit ca3def1 needs some re-review, because during rebase it was just a single big conflict with the write-to-headers change. Apart from that, everything except the default service URL comment and the possible merge of ParameterKind and WebParamAttribute.Origin should be addressed.

I'll have a look at the latter now. Regarding the URL, I don't remember exactly anymore, but I think I got some internal assertions due to an empty service URL, which was quite unobvious for the user. We could alternatively use something like "http://localhost/" or just add high-level assertions with a proper error message. The latter sounds like the better way to go...

s-ludwig · 2015-09-11T09:19:10Z

@Geod24, did you see any remaining issues? Otherwise I'd rebase/squash and merge now.

Geod24 · 2015-09-11T09:20:40Z

I'll do a last sweep this evening ;)

Geod24 · 2015-09-11T20:13:00Z

Can't see anything.
All your project build out of the box with the new version ?

s-ludwig · 2015-09-12T06:29:53Z

I haven't tested all of them yet, but I'll do that now.

See also #1209.

…m the generator. The RestInterface struct is suitable to implement all REST interface generators with minimal boilerplate code. It avoids the high level of redundancy that is prevalent among the different implementations today.

s-ludwig · 2015-09-13T08:49:16Z

Okay, when testing my projects I found one bug in the REST client (09ee6a2), but none in the new code. Will merge once Travis goes green.

JavaScript based REST interface client

s-ludwig · 2015-09-13T09:39:17Z

And thanks for reviewing this lengthy change set!

etcimon · 2015-09-13T14:59:20Z

I would have loved to review this but got caught up in the busiest time of the past few years. Sorry!

ColdenCullen · 2015-10-14T06:45:33Z

source/vibe/web/internal/rest/jsclient.d

+		// XHR setup
+		output.put("    var xhr = new XMLHttpRequest();\n");
+		output.formattedWrite("    xhr.open('%s', url, true);\n", route.method.to!string.toUpper);
+		static if (!is(RT == void)) {


Sorry for not catching this sooner, but is RT actually defined here? I think it's causing this block to execute regardless of return type.

Thanks, looks like that... is() and __traits(compiles) are really suboptimal constructs w.r.t. correctness. I'll add a test and fix this.

Err.. or rather just merge your PR and add a test ;)

s-ludwig mentioned this pull request Aug 3, 2015

Add struct type injection to vibe.web.web #1202

Open

s-ludwig force-pushed the restjsclient branch 3 times, most recently from b454c2c to cc0d0a4 Compare August 5, 2015 05:29

s-ludwig force-pushed the restjsclient branch 3 times, most recently from 61c6209 to b1bc236 Compare August 5, 2015 16:52

Geod24 reviewed Aug 6, 2015
View reviewed changes

s-ludwig changed the title ~~[WIP] JavaScript based REST interface client~~ JavaScript based REST interface client Aug 9, 2015

s-ludwig force-pushed the restjsclient branch from ed739a9 to 0d07888 Compare August 11, 2015 15:29

s-ludwig force-pushed the restjsclient branch from 4166944 to 94bf031 Compare August 11, 2015 22:23

This was referenced Aug 15, 2015

Make exception handling for REST servers optional #1084

Closed

REST API default parameter value #1230

Closed

s-ludwig force-pushed the restjsclient branch 2 times, most recently from 6997c4c to eebf993 Compare September 13, 2015 07:44

s-ludwig added a commit that referenced this pull request Sep 13, 2015

Fix REST client to build "id" URLs correctly.

09ee6a2

See also #1209.

s-ludwig added 8 commits September 13, 2015 10:45

Minor refactoring of the REST module + mark some symbols package.

2b4135f

Fix evaluateOutputModifiers and its use in vibe.web.web.

7a632df

Implement a basic JavaScript REST interface client.

c7bb2e1

Add JavaScript REST example.

cd281a3

Switch REST interface server to use RestInterface!T.

16a21ee

Use as much static information about a REST interface class as possible.

d109798

Use ParameterKind instead of WebParamAttribute.Origin.

68d5322

s-ludwig force-pushed the restjsclient branch from eebf993 to 68d5322 Compare September 13, 2015 08:46

s-ludwig added a commit that referenced this pull request Sep 13, 2015

Merge pull request #1209 from rejectedsoftware/restjsclient

897521b

JavaScript based REST interface client

s-ludwig merged commit 897521b into master Sep 13, 2015

s-ludwig deleted the restjsclient branch September 13, 2015 09:40

ColdenCullen reviewed Oct 14, 2015
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JavaScript based REST interface client #1209

JavaScript based REST interface client #1209

s-ludwig commented Aug 3, 2015

Geod24 commented Aug 3, 2015 via email

s-ludwig commented Aug 4, 2015

s-ludwig commented Aug 5, 2015

Geod24 Aug 6, 2015

s-ludwig Aug 6, 2015

Geod24 commented Aug 6, 2015

s-ludwig commented Aug 7, 2015

s-ludwig commented Aug 11, 2015

s-ludwig commented Aug 11, 2015

s-ludwig commented Sep 11, 2015

Geod24 commented Sep 11, 2015

Geod24 commented Sep 11, 2015

s-ludwig commented Sep 12, 2015

s-ludwig commented Sep 13, 2015

s-ludwig commented Sep 13, 2015

etcimon commented Sep 13, 2015

ColdenCullen Oct 14, 2015

s-ludwig Oct 14, 2015

s-ludwig Oct 14, 2015

JavaScript based REST interface client #1209

JavaScript based REST interface client #1209

Conversation

s-ludwig commented Aug 3, 2015

Geod24 commented Aug 3, 2015 via email

s-ludwig commented Aug 4, 2015

s-ludwig commented Aug 5, 2015

Geod24 Aug 6, 2015

Choose a reason for hiding this comment

s-ludwig Aug 6, 2015

Choose a reason for hiding this comment

Geod24 commented Aug 6, 2015

s-ludwig commented Aug 7, 2015

s-ludwig commented Aug 11, 2015

s-ludwig commented Aug 11, 2015

s-ludwig commented Sep 11, 2015

Geod24 commented Sep 11, 2015

Geod24 commented Sep 11, 2015

s-ludwig commented Sep 12, 2015

s-ludwig commented Sep 13, 2015

s-ludwig commented Sep 13, 2015

etcimon commented Sep 13, 2015

ColdenCullen Oct 14, 2015

Choose a reason for hiding this comment

s-ludwig Oct 14, 2015

Choose a reason for hiding this comment

s-ludwig Oct 14, 2015

Choose a reason for hiding this comment