Hi Jody, To follow up on the github discussion regarding the resources delete API. As I understand we have three methods to delete resources:
1. Resource.delete() - non recursive (until last PR), returns false if file doesn't exist 2. ResourceStore.remove (String path) - recursive, returns true if file does exist 3. Resources.delete(Resource res) - recursive, returns false if file doesn't exist I understand that the idea of creating (3) is that it would factor out the recursive algorithm, leaving to implementers of resource stores only the need to implement the deletion of one resource / empty directory. But then (2) is still recursive, which enforces an implementation of the recursion in the resource store anyway. I understand the reasoning, but recursive delete can be made much more efficiently for the JDBCStore than the implementation in (3) with less queries sent to the database. I think that resource stores should be allowed to have their own implementation for that reason. Perhaps one future implementation of resource store could delete a full directory in one go, without the need for recursion. So yes, I'd agree to get rid of the (3) method all together and change the contract. Another thing I'd like to see changed is that I'd like the rule on whether to return true or false on a non-existing file should be the same for (1) and (2). I don't care which is chosen, but since method (2) is hardly (if anywhere at all) used, it would be least risky to change it. Unless there was/is a conscious reason to do it this way, of course. It should be better documented then though, because at this moment the behaviour towards non-existing files for (1) is not documented, which may lead people to believe that it will the same as (2), which is explicitly documented. Regards Niels ------------------------------------------------------------------------------ _______________________________________________ Geoserver-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geoserver-devel
