Skip to content

Introducing a SidFactory #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

ctapobep
Copy link

@ctapobep ctapobep commented Jul 7, 2012

  • I made some research on different forums and it appeared I'm not the only one who was suffering because of hardcoded sids being created. In my case I needed Groups of Users and Users themselves and for this I needed different Sid classes, so I had to create a SidFactory and replace all the places where Sids were created with this factory.
    I made changes so that nothing differs in the logic of Spring ACL, but if user wants to create custom Sids, he is welcome to introduce and inject his own factory.
  • Another problem was that Sids don't have a method 'getSidId()' and thus we're forced always to cast them to particular implementation which is also ugly.

Please, consider this patch to be included into Spring ACL sources and I'm really willing to hear the feedback about suggested changes.

ctapobep added 3 commits July 7, 2012 19:41
…classes that were creating sids now create them via that factory
…classes that were creating sids now create them via that factory
@ctapobep
Copy link
Author

@ctapobep
Copy link
Author

ctapobep commented Mar 3, 2014

Does this go anyway? The pull request is hanging for 2 years already.. There are not that many of changes as for me.

@rwinch
Copy link
Member

rwinch commented Mar 18, 2014

NOTE: Please refer to the referenced JIRA for further discussion.

@ctapobep
Copy link
Author

ctapobep commented Aug 8, 2014

Closing this PR in favour of #115

@ctapobep ctapobep closed this Aug 8, 2014
@rwinch rwinch removed the status: waiting-for-feedback We need additional information before we can continue label Feb 4, 2015
bclozel added a commit to bclozel/spring-security that referenced this pull request Jan 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants