Hi Dinuka, Went through the code and please find the points for improvement below:
- Like in other endpoints, you can select the encoder, decoder dynamically based on the content type and accept headers without hard coding the JSON encoder and JSON decoder. - In an improved version, bulk requests needs to be processed in a generic manner and distinguish different operations based on "method" and "path" in each operation. BulkRequestProcessor, BulkRequestData and BulkResponseData in your patch could be improved accordingly. - SCIMClient API could be improved to generate the payload in a generic manner and use that in the sample provided in this patch. - When improving this, we can avoid hard coded values as much as possible. For eg: 200 OK in response in BulkResourceEndpoint - Please use constants to refer to different values in the code. Refer SCIMConstants and its usages. - Please use the wso2-coding styles found at https://sites.google.com/a/wso2.com/engineering/Home/masteringintellijidea. It is good that you have done the design diagrams too. We can add them under documentations [1]. - You can use UML notations to show the relationships between the classes in the class diagram [1] https://svn.wso2.org/repos/wso2/trunk/commons/charon/documentation/ Overall It is a very good job. Hope above comments would help you to further improve. Thanks, Hasini. On Tue, Jan 8, 2013 at 12:42 PM, Hasini Gunasinghe <[email protected]> wrote: > Great job Dinuka..! > Appreciate a lot all your voluntarily efforts on adding bulk endpoint to > Charon.. I think we can try to ship this with IS 4.1.0, if time permits. > Will review it and get back to you with the feedback.. And once it is > completed, we also can have a code review session when you have some free > time.. > > Thanks, > Hasini, > > > On Mon, Jan 7, 2013 at 11:52 AM, Dinuka Malalanayake <[email protected]>wrote: > >> Hi All, >> >> I have complete the basic development of SCIM bulk endpoint. This is >> created according to the specification [1]. Appreciate if some one can do >> the code review and give me some feedback. >> >> Sequence diagram and class association diagram attached here with >> >> As well as I have attached the patch file as a improvement in Commons >> project [2] >> >> >> [1] >> http://www.simplecloud.info/specs/draft-scim-api-01.html#delete-resource >> >> [2] https://wso2.org/jira/browse/COMMONS-90 >> >> -- >> Thanks, >> Dinuka Malalanayake >> *Software Engineer*, WSO2, Inc.; http://www.wso2.com, >> *Linked In* : http://lk.linkedin.com/pub/dinuka-malalanayake/24/438/169 >> *Blog* : http://malalanayake.wordpress.com/ >> *Contact* : 0772508354 , [email protected] *Skype *: >> dinuka_malalanayake >> > >
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
