Re: [xwiki-devs] [ANN] XWiki Summer 2013 Hackathon

Hi Edo, I hope that my comments below will not fall too hard since they arrive after the fact. So, first, I would like to congratulate you for your participation. On Tue, Jul 30, 2013 at 6:50 PM, Edoardo Beutler < edoardo.beutler(a)synventis.com&gt; wrote: You should have asked during the hackathon, it is also a event to team together, and I would have been more than happy to help you understanding the new security authorization module. I have quickly review your implementation, and it have for sure require a deep understanding of the current implementation. This is also what will make your extension more fragile, since it really depends on implementation details that may change over time. I noticed that you have taken care of checking these details to fail gracefully which is nice. We decided to prepare the publication extension for xwiki 5.1. It worked For sure, right caching is not that easy, and it implies a bit of complexity, especially if you expected some access to change simply based of the elapsed time since the decision has been taken. Well, this is true. Maybe I got a bit paranoid on this, but my feeling was that the settler should be the only one with the power of creating and changing a SecurityAccessEntry. This obviously make it difficult for a class from another package to simply extends the existing implementation. I understand why you would like to have a date from the security cache, this is clever idea even if it has not been made for that. Adding the date should not be complex since the cache store SecurityEntries and those are generated by the AuthorizationSettler. But I am afraid that using those dates later is more complex without somewhat "hacking" the AuthorizationManager. Extending the AuthorizationManager is risky. Evicting entries from other places than the SecurityCacheRulesInvalidator is probably a bad idea and could lead to multithreading issues. Moreover, if I have correctly understand your current implementation, it defeat completely the caching of access entries, which is bad for the performance of the security module. At least, you should mention that in the description of your extension, so nobody get surprised. The best alternative I can think about right now, is to extend the SecurityCacheRulesInvalidator in a way that cause entries to be evicted from the cache in due time, when those became invalid. You may for example keep a cached list of cached entries in your own independent cache with their associated publish times. Listening to the security cache events, you may keep that list clean and up to date. Based on that list, it should be possible to program some timer to trigger an extended SecurityCacheRulesInvalidator at appropriate time. Let me know if you would like to discuss that subject further and I will do my best to help. Thanks again for your participation. -- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO