Hi Francesco, > -----Original Message----- > From: Francesco Chicchiriccò [mailto:ilgro...@apache.org] > Sent: Freitag, 25. Januar 2013 17:08 > To: dev@syncope.apache.org > Subject: Re: Discuss: Rest interface for UserRequestService > > On 25/01/2013 16:56, Christian Schneider wrote: > > When doing the CXF version of the UserRequestService I found that our > > current UserRequestController does not really follow the rest ideas. > > > > The problem is that it does follow the idea of doing CRUD operations > > on resources. Some methods work with userId others with requestId. So > > for example the are create, update and delete operations but they do > > not mean the UserRequest but the underlying user. > > > > So I worked out a new interface that purely works on UserRequest and > > only supports POST, READ and DELETE. So basically if you want to > > create or delete or update a user you will POST a UserRequestTO in all > cases. > > Problem: you've already made all these changes *without* discussing this > before,
IMHO I think it is OK to commit new features without discussing method signature in advanced. Most time it is not possible to think of best design in advanced, but only while implementing a feature it becomes clear how to optimize method signatures. Of course if I change something that effects other code also, this should be discusses previously. Do you expect all committers to start a discussion before they introduce a new method? Since Core REST interface is an important topic for whole project we already agreed to discuss each REST interface one by one. But since new REST API will not be used in next syncope release I was expecting that it is more important to get a first (nice) implementation done and then start discussion and refactoring afterwards. As mentioned earlier, it is not easy to make a "perfect" design at first and then start a discussion. WDYT? > once again not taking into any account the domain requirements I don't understand this, can you please elaborate? Domain specific requirements are completely mapped. There is no functionality lost by making a service RESTful. IMHO I think it is best to preserve all current functionality while optimizing request behavior to best practices of used technology. Which domain requirement cannot be covered by using proper RESTful design? > nor the compatibility with existing components, but only the technology > aspects. I also don't understand this. Could you please explain some more? We previously agreed, that new REST API does not need to be identically with current REST URLs, only functional compatibility was required. That's why I started [1] to document how REST API will change from Spring to CXF migration. So people using their own Console client will have to migrate in any case. And if this is the case, why not using underlying technology according to best practices? This way we will avoid refactoring API at a later state, when we run into issues, that could have been avoided by directly applying best practices of used technology. Since we are using JAX-B and JAX-RS annotation in new interfaces, writing a custom client will become much easier, since client code can be generated based on annotations. Best regards. Jan [1] https://cwiki.apache.org/confluence/display/SYNCOPE/REST+API+upgrade#RESTAPIupgrade-RoleService > We must fix this ASAP. > > Regards. > > > I added constructors to the UserRequestTO to make the use cases simpler. > > > > request to create a user: userRequestService.create(new > > UserRequestTO(userTO)); request to update a user: > > userRequestService.create(new UserRequestTO(userMod)); request to > > delete a user: userRequestService.create(new > > UserRequestTO(userTO.getId())); > > > > So what do you think about this? > > > > The only thing I really struggled with is the old isCreateAllowed > > method. I implemented this using @Options and return the boolean in a > > custom header. I am not sure if this is really the best way to do this > > but it follows the rest principles quite closely. The alternative > > would be to do a get on a magic path like in the current spring > > service. > > > > Best regards > > > > Christian > > > > ------------------------------ > > Current Interface: > > @RequestMapping(method = RequestMethod.POST, value = "/create") > public > > UserRequestTO create(@RequestBody final UserTO userTO); > > > > @RequestMapping(method = RequestMethod.POST, value = "/update") > public > > UserRequestTO update(@RequestBody final UserMod userMod); > > > > @RequestMapping(method = RequestMethod.GET, value = > > "/delete/{userId}") public UserRequestTO > > delete(@PathVariable("userId") final Long userId) > > > > @RequestMapping(method = RequestMethod.GET, value = "/list") public > > List<UserRequestTO> list(); > > > > @RequestMapping(method = RequestMethod.GET, value = > > "/read/{requestId}") public UserRequestTO > > read(@PathVariable("requestId") final Long requestId); > > > > @RequestMapping(method = RequestMethod.GET, value = > > "/deleteRequest/{requestId}") > > public UserRequestTO deleteRequest(@PathVariable("requestId") final > > Long requestId); > > > > -------------------- > > New interface for CXF Service: > > @Path("requests/user") > > public interface UserRequestService { > > public static final String SYNCOPE_CREATE_ALLOWED = > > "Syncope-Create-Allowed"; > > > > @OPTIONS > > Response getOptions(); > > > > @POST > > Response create(UserRequestTO userRequestTO); > > > > @GET > > List<UserRequestTO> list(); > > > > @GET > > @Path("{requestId}") > > UserRequestTO read(@PathParam("requestId") Long requestId); > > > > @DELETE > > @Path("{requestId}") > > void delete(@PathParam("requestId") Long requestId); } > > -- > Francesco Chicchiriccò > > ASF Member, Apache Syncope PMC chair, Apache Cocoon PMC Member > http://people.apache.org/~ilgrosso/
