Hi Daniel, Got a chance to review this ? Pradeep.
> Date: Thu, 3 Sep 2015 22:17:38 +0200 > From: [email protected] > To: [email protected] > Subject: Re: Rest Service for FreeMarkerOnline > > I will... tomorrow or at the weekend. > > > Thursday, September 3, 2015, 8:27:15 PM, Pradeep Murugesan wrote: > > > Hi Daniel, > > Kindly review when ever you find time. > > https://github.com/pradeepmurugesan/freemarker-online/commit/4445f45997c1eaf76c6b4e13e8c2054ee163e034https://github.com/pradeepmurugesan/freemarker-online/commit/d8e25be864a25596d29f875fa845695d02168e26 > > Pradeep. > >> From: [email protected] > >> Subject: Re: Rest Service for FreeMarkerOnline > >> Date: Tue, 1 Sep 2015 23:40:38 +0530 > >> To: [email protected] > >> > >> Yup.. Finishing the current task is the priority. > >> > >> Pradeep. > >> > >> > On 01-Sep-2015, at 10:09 pm, "Daniel Dekany" <[email protected]> wrote: > >> > > >> > Tuesday, September 1, 2015, 10:01:31 AM, Pradeep Murugesan wrote: > >> > > >> >> Sure Daniel, > >> >> I will change the initialization @ method level. > >> >> I have a couple of questions. Please reply when you find time. Sorry If > >> >> its too naive. > >> >> 1. I tried to check whether jersey's resource methods are singleton > >> >> but found in the doc that they spawn a thread for each request. And > >> >> we may turn on the singleton thing if required through some explicit > >> >> settings. > >> > > >> > It's Spring bean (@Component), and those are singletons by default. > >> > Even if wasn't a singleton, I see no reason for a response object to > >> > be recycled between requests. It can be the source for hideous bugs, > >> > as you start with a dirty object, etc. > >> > > >> >> I browsed our code to see if we have some setting but > >> >> couldn't find anything. > >> > > >> > The easiest is just trying it with some log messages. > >> > > >> >> In 3.4 of > >> >> https://jersey.java.net/nonav/documentation/latest/user-guide.html#d0e2586 > >> >> we can see they mentioning the above. > >> >> 2. In the mean time I am trying to get hold of how freemarker > >> >> template engine works by trying to fix some low hanging bugs so that > >> >> I can understand the architecture better. I am browsing for issues > >> >> here http://sourceforge.net/p/freemarker/bugs/. Kindly let me know a > >> >> good place or a minor bug to start with. > >> > > >> > Most bugs will be stuff that isn't fixed because it's tricky to fix > >> > them, often because of backward compatibility requirements... But for > >> > example here's this new one: > >> > http://sourceforge.net/p/freemarker/bugs/434/ Could be checked if it's > >> > true, and why it happens. But of course deal what we feel like with. > >> > > >> > (It's also a good idea to polishing what you have started, like > >> > writing the JUnit tests, etc.) > >> > > >> >> Pradeep. > >> >> > >> >>> Date: Mon, 31 Aug 2015 20:49:34 +0200 > >> >>> From: [email protected] > >> >>> To: [email protected] > >> >>> Subject: Re: Rest Service for FreeMarkerOnline > >> >>> > >> >>> OK, thanks! I will be rather busy for a few days, so it might will > >> >>> take a while until I check this out in detail. But I assume you know > >> >>> of things that you wanted to finish/polish anyway. > >> >>> > >> >>> Something I have spotted with this quick look now is that > >> >>> executeResponse shouldn't be a field in > >> >>> FreeMarkerOnlineExecuteResource, as I suppose the resource object is a > >> >>> singleton used by multiple threads. > >> >>> > >> >>> > >> >>> Monday, August 31, 2015, 3:21:09 PM, Pradeep Murugesan wrote: > >> >>> > >> >>>> Hi Daniel, > >> >>>> > >> >>>> https://github.com/pradeepmurugesan/freemarker-online/commit/71e70d6caf9bead735ca0f2b6eb4f81c708a1922 > >> >>>> I have fixed the code review comments and integrated the same with UI. > >> >>>> Please let me know your reviews. > >> >>>> > >> >>>> Pradeep. > >> >>>> > >> >>>>> Date: Mon, 31 Aug 2015 12:22:17 +0200 > >> >>>>> From: [email protected] > >> >>>>> To: [email protected] > >> >>>>> Subject: Re: Rest Service for FreeMarkerOnline > >> >>>>> > >> >>>>> Certainly it's good enough to go ahead with the UI. > >> >>>>> > >> >>>>> Some random stuff I have noticed: > >> >>>>> > >> >>>>> - In "problems", the field names by convention should start with > >> >>>>> lower > >> >>>>> case, and they will have to be extracted to enums (or to static > >> >>>>> final String-s). > >> >>>>> > >> >>>>> - Class names will need some cleanup. Like FreeMarkerPayload and > >> >>>>> FreeMarkerResponse are in fact for the "execute" resource only, not > >> >>>>> for FM-Online in general, so I guess they should be ExecutreRequest > >> >>>>> and ExecuteResponse. FreeMarkerErrorReponse is for the FM *online* > >> >>>>> service, but that's already told by the package so... maybe just > >> >>>>> ErrorResponse. Anyway, these are just Alt+Shift+R things. > >> >>>>> > >> >>>>> -- > >> >>>>> Thanks, > >> >>>>> Daniel Dekany > >> >>>>> > >> >>>>> > >> >>>>> Sunday, August 30, 2015, 4:47:55 PM, Pradeep Murugesan wrote: > >> >>>>> > >> >>>>>> Hi Daniel, > >> >>>>>> https://github.com/pradeepmurugesan/freemarker-online/commit/80c984af1e810d69db2894146d67b52e2449a584 > >> >>>>>> > >> >>>>>> I have made the changes as per your comments below. Please review > >> >>>>>> and let me know if any corrections. > >> >>>>>> Still I didn't do the UI for this new Response structure. Thought > >> >>>>>> we will finalize the API Responses then we will get into the UI. > >> >>>>>> Pradeep. > >> >>>>>> > >> >>>>>>> Date: Sat, 29 Aug 2015 18:30:33 +0200 > >> >>>>>>> From: [email protected] > >> >>>>>>> To: [email protected] > >> >>>>>>> Subject: Re: Rest Service for FreeMarkerOnline > >> >>>>>>> > >> >>>>>>> Saturday, August 29, 2015, 4:12:16 PM, Pradeep Murugesan wrote: > >> >>>>>>> > >> >>>>>>>> okay.. > >> >>>>>>>> So almost all the errors we handle should go under Success right. > >> >>>>>>>> Having a structure like this would help I believe. > >> >>>>>>>> { error: true/false, outputTruncated: true/false, > >> >>>>>>>> failureReason: String output: String} > >> >>>>>>> > >> >>>>>>> That's for the HTTP 200 answers only, I assume. We miss the > >> >>>>>>> information about which field the failureReason applies to. So, I > >> >>>>>>> think, the cleanest and most flexible would be if we remove "error" > >> >>>>>>> (it's confusing and redundant anyway) and "failureReason", and > >> >>>>>>> instead > >> >>>>>>> add a "problems" JSON Object, in which the keys are the field > >> >>>>>>> names, > >> >>>>>>> and the value are the error descriptions. It's like some simple > >> >>>>>>> form > >> >>>>>>> processing answer. > >> >>>>>>> > >> >>>>>>> And then there are the HTTP 5xx errors. There I think we can get > >> >>>>>>> away > >> >>>>>>> with an "errorCode" and an "errorDescription" for now. > >> >>>>>>> > >> >>>>>>> -- > >> >>>>>>> Thanks, > >> >>>>>>> Daniel Dekany > >> >>>>>>> > >> >>>>>>>> Based on the error flag we can decide what to display. Sorry If I > >> >>>>>>>> am taking a longer time to get hold of things. > >> >>>>>>>> Pradeep. > >> >>>>>>>>> Date: Sat, 29 Aug 2015 14:34:58 +0200 > >> >>>>>>>>> From: [email protected] > >> >>>>>>>>> To: [email protected] > >> >>>>>>>>> Subject: Re: Rest Service for FreeMarkerOnline > >> >>>>>>>>> > >> >>>>>>>>> There are some cases whose distinction can be interesting for a > >> >>>>>>>>> UI > >> >>>>>>>>> (without much research, so it might not be accurate): > >> >>>>>>>>> - Successful Web service call results: > >> >>>>>>>>> - Template output, no template or data-model errors > >> >>>>>>>>> - Template output that's cut at a point as it was too long > >> >>>>>>>>> - Failed data model building > >> >>>>>>>>> - Failed template execution (position can be interesting here > >> >>>>>>>>> on the > >> >>>>>>>>> long run) > >> >>>>>>>>> - Service errors: I guess we just need the error message here. > >> >>>>>>>>> - A bit of both: Long running template timeout. Usually it's the > >> >>>>>>>>> user's mistake... usually. > >> >>>>>>>>> > >> >>>>>>>>> The *template* used by the current UI doesn't care about such > >> >>>>>>>>> details, > >> >>>>>>>>> but what it displays was already assembled by code that also > >> >>>>>>>>> belongs > >> >>>>>>>>> to the UI (i.e., to the JavaScript that processes the JSON > >> >>>>>>>>> response). > >> >>>>>>>>> That logic is certainly simplistic currently, but then, UI-s can > >> >>>>>>>>> change any time (and multiple different UI-s can co-exist), > >> >>>>>>>>> while Web > >> >>>>>>>>> service API-s less so. > >> >>>>>>>>> > >> >>>>>>>>> > >> >>>>>>>>> Saturday, August 29, 2015, 1:19:41 PM, Pradeep Murugesan wrote: > >> >>>>>>>>> > >> >>>>>>>>>> Hi Daniel, > >> >>>>>>>>>> The thing I am trying to understand here is the need for all > >> >>>>>>>>>> the 3 > >> >>>>>>>>>> fields of FreeMarkerServiceResponse for the UI. > >> >>>>>>>>>> FreeMarkerOnline view while rendering the template just uses > >> >>>>>>>>>> the 2 parameters. > >> >>>>>>>>>> 1. Is there an error in the result 2. what is the error message > >> >>>>>>>>>> <#if hasResult> <div class="resultContainer"> <label > >> >>>>>>>>>> for="result">Result</label> <textarea id="result" > >> >>>>>>>>>> class="pure-input-1 source-code <#if errorResult> error</#if>" > >> >>>>>>>>>> readonly>${result}</textarea> </div></#if> > >> >>>>>>>>>> > >> >>>>>>>>>> So assuming we too need the same from Ajax requests > >> >>>>>>>>>> I am returning the result & the error is found based on the > >> >>>>>>>>>> status code of the response. > >> >>>>>>>>>> Kindly let me know your thoughts. > >> >>>>>>>>>> Pradeep. > >> >>>>>>>>>> > >> >>>>>>>>>> > >> >>>>>>>>>> > >> >>>>>>>>>> > >> >>>>>>>>>>> Date: Fri, 28 Aug 2015 22:51:52 +0200 > >> >>>>>>>>>>> From: [email protected] > >> >>>>>>>>>>> To: [email protected] > >> >>>>>>>>>>> Subject: Re: Rest Service for FreeMarkerOnline > >> >>>>>>>>>>> > >> >>>>>>>>>>> The response should be JSON because we will need a few > >> >>>>>>>>>>> separate fields > >> >>>>>>>>>>> there. If you look at FreeMarkerServiceResponse, you will see 3 > >> >>>>>>>>>>> candidates, but then you will find more, as the thrown > >> >>>>>>>>>>> exceptions also > >> >>>>>>>>>>> carries information that's needed for the UI, as you can see in > >> >>>>>>>>>>> FreeMarkerService.calculateTemplateOutput. > >> >>>>>>>>>>> > >> >>>>>>>>>>> -- > >> >>>>>>>>>>> Thanks, > >> >>>>>>>>>>> Daniel Dekany > >> >>>>>>>>>>> > >> >>>>>>>>>>> > >> >>>>>>>>>>> Friday, August 28, 2015, 9:57:58 PM, Pradeep Murugesan wrote: > >> >>>>>>>>>>> > >> >>>>>>>>>>>> Sure Daniel, > >> >>>>>>>>>>>> I will change as per your comments. > >> >>>>>>>>>>>> A quick clarification regarding the json response can be like > >> >>>>>>>>>>>> following ? > >> >>>>>>>>>>>> { result: <output> } > >> >>>>>>>>>>>> Pradeep. > >> >>>>>>>>>>>> > >> >>>>>>>>>>>>> Date: Fri, 28 Aug 2015 21:26:58 +0200 > >> >>>>>>>>>>>>> From: [email protected] > >> >>>>>>>>>>>>> To: [email protected] > >> >>>>>>>>>>>>> Subject: Re: Rest Service for FreeMarkerOnline > >> >>>>>>>>>>>>> > >> >>>>>>>>>>>>> Friday, August 28, 2015, 7:53:45 PM, Pradeep Murugesan wrote: > >> >>>>>>>>>>>>> > >> >>>>>>>>>>>>>> Hi Daniel, > >> >>>>>>>>>>>>>> I have made the rest service up @ a new path /compile > >> >>>>>>>>>>>>>> The service takes the following json as input > >> >>>>>>>>>>>>>> { "template": "Hello ${user}", "dataModel": > >> >>>>>>>>>>>>>> "user=pradeep"} > >> >>>>>>>>>>>>>> and then compiles the template and dataModel and returns > >> >>>>>>>>>>>>>> the output. > >> >>>>>>>>>>>>>> https://github.com/pradeepmurugesan/freemarker-online/commit/10de59ac0db0bf0f79ab28214f50c851a5610e20 > >> >>>>>>>>>>>>>> > >> >>>>>>>>>>>>>> Please review the above commit and let me know if its ok. > >> >>>>>>>>>>>>> > >> >>>>>>>>>>>>> The response will have to be JSON as well (not TEXT_PLAIN), > >> >>>>>>>>>>>>> but I > >> >>>>>>>>>>>>> guess that was planned later. > >> >>>>>>>>>>>>> > >> >>>>>>>>>>>>> The usage of the "compile" term is confusing here, as you > >> >>>>>>>>>>>>> actually > >> >>>>>>>>>>>>> parse (aka. compile) and then "process" (aka. execute) here. > >> >>>>>>>>>>>>> The last > >> >>>>>>>>>>>>> naturally implies the first. So I guess it should be, like, > >> >>>>>>>>>>>>> "run" or > >> >>>>>>>>>>>>> "execute". > >> >>>>>>>>>>>>> > >> >>>>>>>>>>>>> Also, all the web service operations should go under /api/, > >> >>>>>>>>>>>>> and the UI > >> >>>>>>>>>>>>> outside it. > >> >>>>>>>>>>>>> > >> >>>>>>>>>>>>>> Will > >> >>>>>>>>>>>>>> Integrate with the UI. I have a questions though > >> >>>>>>>>>>>>>> 1. Should I modify in the same path as "/" or keeping it > >> >>>>>>>>>>>>>> in a > >> >>>>>>>>>>>>>> separate path like "/compile" is fine ? > >> >>>>>>>>>>>>> > >> >>>>>>>>>>>>> The UI addresses should remain /, and the current web > >> >>>>>>>>>>>>> service should > >> >>>>>>>>>>>>> be under /api/run or something, I think. > >> >>>>>>>>>>>>> > >> >>>>>>>>>>>>>> Also I will write unit tests once we finalize the path. > >> >>>>>>>>>>>>>> Pradeep. > >> >>>>>>>>>>>>> > >> >>>>>>>>>>>>> -- > >> >>>>>>>>>>>>> Thanks, > >> >>>>>>>>>>>>> Daniel Dekany > >> >>>>>>>>>>>>> > >> >>>>>>>>>>>> > >> >>>>>>>>>>> > >> >>>>>>>>>> > >> >>>>>>>>> > >> >>>>>>>>> -- > >> >>>>>>>>> Thanks, > >> >>>>>>>>> Daniel Dekany > >> >>>>> > >> >>>> > >> >>> > >> >>> -- > >> >>> Thanks, > >> >>> Daniel Dekany > >> >>> > >> >> > >> > > >> > -- > >> > Thanks, > >> > Daniel Dekany > >> > > > > > -- > Thanks, > Daniel Dekany >
