backend code review - RestCon

[TomasKulhanek]

The class org.cirmmp.spring.controller.RestCon overlaps with functionality at AppController. I recommend to follow the same recomendation as in AppController - extract the user, project, dataset into separated controllers.

[andreagia]

Hi Tomas I have create the AppController like test for the for jsp pages the integration with your frontend is in the RestCon class were

[TomasKulhanek]

This needs to be refactored to follow obvious CRUD mapping into http methods POST (or PUT with new id), GET, PUT, DELETE. In order to prevent kitchen sink antipattern, I'd recommend to extract controller methods related to userService, projectService into separated classes. Therefore I recommend these changes:

• refactor the class - extract methods handling userService to a new controller class

• change all the api endpoints (/list, /newuser, /edit-user,/delete-user) to /user, avoid using verb in endpoints

• distinguish operation by the HTTP methods: GET (without id =current '/list' users, with id=current '/newuser' user details) ,
  PUT (=update),
  POST (=create,or update - if id is presented),
  DELETE (delete)

• refactor the class - extract methods handling projectService operation to a new controller class

• change all the endpoints (/listPro,/listData-{projid}, to /project

• distinguish operations, HTTP methods

• GET on /project = current /listPro

• GET on /project/{id} = project detail = current /list-data-{} with list of data

• POST on /project/{id} = adds new dataset, current /add-dataset-{projectid},

• POST on /project = create new project,

• extract authentication methods to new controller class

• /login

• /logout

• remove Access_Denied - access denied is HTTP 403?

These points for discussion:

• Review: doesn't make sense to have /add-file GET and /add-file POST /new-dataset GET - /new-dataset POST - this is not REST - but simple
• per the analysis the relation between [dataset] and [fileset] is 1:1-
• fileset is either the zip,tar.gz or webdav entrypoint to folder containing the files so dataset=file
• so I suggest to keep this semantics in api endpoints - everything is under /dataset (HTTP methods GET,POST,PUT,DELETE)
• new method /dataset - returns all datasets belonging to user,
• BTW. /project/{id} - from TODO 2. already return project deteails including list of dataset ids belonging to the project.
• "/download-file-{prId}-{fileId}" - refactor to /dataset/{datasetId}, method==RequestMethod.GET - datasetId=fileId?
• "/create-tar-{prId}" - ??? I suggest to remove it, I suggest to create zip on the fly per my email
• refactor @RequestMapping(value = { "/delete-project-{Id}" }, method = RequestMethod.GET)
  to @RequestMapping(value = { "/project/{Id}" }, method = RequestMethod.DELETE)
• refactor @RequestMapping(value = { "/delete-dataset-{proId}-{dataId}" }, method = RequestMethod.GET)
  to @RequestMapping(value = { "/dataset/{dataId}" }, method = RequestMethod.DELETE)
• think of removing "/delete-file-{prId}-{fileId}", file = dataset, if dataset is deleted - then should the file- fileset be deleted???
• refactor or remove other methods with the same semantics as above.

[TomasKulhanek]

RestCon is for REST api, AppController is for JSP UI.