Skip to content
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

TASK: Defer initialisation of NodePriviliges #1883

Merged
merged 2 commits into from Apr 25, 2018

Conversation

kitsunet
Copy link
Member

@kitsunet kitsunet commented Feb 7, 2018

No description provided.

Copy link
Contributor

@aertmann aertmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks fine, just a small nit pick

*/
protected function buildCacheEntryIdentifier()
{
$this->cacheEntryIdentifier = md5($this->privilegeTarget->getIdentifier() . '__methodPrivilege' . '|' . $this->buildMethodPrivilegeMatcher());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason not to use the existing getPrivilegeTargetIdentifier instead of $this->privilegeTarget->getIdentifier() method here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kitsunet Can you clarify this? Is that needed due to lazy initialisation or something?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by testing look like calling getPrivilegeTargetIdentifier doesn't make any difference.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there were edge cases in which using getPrivilegeTargetIdentifier triggered additional initialisations that were not really needed.

@kdambekalns
Copy link
Member

@kitsunet What about this, re-target to 5.0?

@kitsunet kitsunet changed the base branch from master to 4.0 April 25, 2018 07:40
@kitsunet kitsunet merged commit f38801e into neos:4.0 Apr 25, 2018
@kitsunet kitsunet deleted the task/defer-node-privilege-init branch April 25, 2018 07:40
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.

None yet

4 participants