duct-framework / duct

Server-side application framework for Clojure
MIT License
1.13k stars 51 forks source link

Add +rest hint for json rest endpoints #11

Closed pjagielski closed 8 years ago

pjagielski commented 9 years ago

I had a little trouble with converting example endpoint to make it json compatible, so I thought about adding such option to duct. I wonder is it possible to have both :example and :rest in system.clj? For now I just replaced :app [:example] with :app [:rest] but it may be nice to have both.

weavejester commented 9 years ago

Thanks for the patch.

With Duct I want to be careful about adding more profiles, because I want to be reasonably certain that a profile demonstrates the right way to do things before adding it to the template. In this case, I'm not certain whether I should advocate Ring-JSON, or something like Liberator, or something entirely new, so until I come to a decision on that, I'm afraid this pull request needs to be put on hold.

I do have a few comments on the code, however. First, I think that there's no point to having another example endpoint. We can just add a JSON route to the existing example endpoint. That's its purpose after all; to provide examples of use.

You also need to be careful about getting the indentation right. This is a template, after all, so it matters more than in regular code. On the last line of your example endpoint you indent several spaces too far.

pjagielski commented 9 years ago

Thanks for review. I merged new endpoint into existing example and fixed indentation.

I recently evaluated a few approaches to json support with duct and ring-json seems most lightweight. Let me know if there is something I can do with this PR.

dadair-ca commented 9 years ago

Alternatively, should the json-body & json-response be automatically applied to base duct templates? The base template already provides api-defaults to wrap-defaults, and (it seems, at least) that most API-centric apps would wants JSON output/input, to conform to current industry standards.

json-body and json-response could be left out if the +site profile is set.

Then, a section of the README (or wiki) could be put towards changing your input/output data format (say to EDN, XML, etc), and would basically just point to the json-middleware and offer examples to change?

weavejester commented 9 years ago

I don't think they should be added by default. While JSON is commonly used, there are plenty of scenarios that don't require JSON. For instance, if you have a single-page ClojureScript app backed up a web service, then the web service is more likely to use edn or Transit than JSON.

weavejester commented 8 years ago

I'm closing this PR because after thinking about it, I don't want to use Ring-JSON as the canonical example of how to build a RESTful web service.