Initial work to support CRUD through REST#26
Conversation
| import java.util.Date; | ||
| import java.util.Map; | ||
|
|
||
| public class DataFilterDefaultResponse { |
There was a problem hiding this comment.
Please build atop the REST web services module’s API to maintain uniformity from a client perspective.
| import java.util.Map; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| @Component |
There was a problem hiding this comment.
Like I said earlier, build on top of the REST API instead of creating this logic from scratch.
| if (basis == null) { | ||
| return new ResponseEntity<>("Data filter Basis is required!", HttpStatus.BAD_REQUEST); | ||
| } | ||
| dataFilterService.grantAccess(entity, basis); |
There was a problem hiding this comment.
Methods like grantAccess are not protected in the Java API, this will be a security hole because anyone can call this endpoint to grant themselves access to forbidden resources.
|
|
||
| @RequestMapping(method = RequestMethod.POST, value = "search") | ||
| @ResponseBody | ||
| public List<DataFilterDefaultResponse> searchEntityBasisMap(@Valid @RequestBody EntityBasisMapSearchRequest searchQuery) |
There was a problem hiding this comment.
This should be a protected resource that you don't want every user to view.
| @@ -0,0 +1,93 @@ | |||
| -- create a mapping: | |||
There was a problem hiding this comment.
We would need to return resources based on the web services module's API, let's not create 'deviant' resources.
wluyima
left a comment
There was a problem hiding this comment.
I have added some comments, let's have a ticket and discuss before implementing this.
|
Thanks, @wluyima. Comments well noted |
|
I agree with @wluyima's points. At a slight step back, I wonder if it might make sense to move the Location-based filtering and the REST API here into a separate, optional JAR? The issue is that the REST API here is only useful with location-based filter stuff, but that isn't the only thing that this module is able to filter. Basically, I'd like some kind of "data filter core" with a "location-based limit" addon for that core, if that makes sense? |
No description provided.