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

add PrincipalInterface and cleanup docs #31

Merged
merged 3 commits into from Jul 25, 2012
Merged

Conversation

dbu
Copy link
Member

@dbu dbu commented Mar 7, 2012

@sixty-nine and i reviewed this section and we added a "principal" interface because there seems to be nothing fitting in php.

does it make sense like this?

note that for jackalope-jackrabbit, we can't really implement it right now, it first needs to be supported by the jackrabbit davex component. but we need to clean things up so that jsr-333 can ship with valid interfaces.

/cc @bergie, @lsmith77, @chregu

@lsmith77
Copy link
Member

lsmith77 commented Mar 7, 2012

what does this mean for jackalope? will we have a problem updating phpcr?

@dbu
Copy link
Member Author

dbu commented Mar 7, 2012

we don't implement the security chapter at all atm, so no changes in jackalope. the only difference is that now it would be theoretically implementable, because the php interfaces do not contain references to java classes anymore.

i would love to get some feedback on our take on the Principal interface.

management summary is that the repo implementation needs to persist permissions and serializing user objects to do that is not a good approach, so it needs something else to persist that can be compared later. hence the hashCode method.

@dbu
Copy link
Member Author

dbu commented Mar 7, 2012

to better understand about the hashcode, this helped us: http://www.javapractices.com/topic/TopicAction.do?Id=28
we decided to use string instead of number to make it easier to build hashcodes.

@lsmith77
Copy link
Member

lsmith77 commented Mar 7, 2012

ok .. then i of course have no issue with seeing this added.

now the big task for us in general is to finally ensure that jackrabbit exposes everything we need via davex. maybe we need to post to the JSR-333 list that we need this to complete the reference implementation of PHPCR.

* However it should be unique inside your application for different
* principals.
*
* @return string a hashcode for this principal.
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 start to worry if it is a good idea to return string here. the problem is that for the integration with jackrabbit, we will need an integer, and it will be impossible to have the same user in java and php if we convert the string hash to some int hash.

so maybe we should say int here and when we implement the interfaces provide a helper in phpcr-utils similar to this java helper http://www.javapractices.com/topic/TopicAction.do?Id=28

its a bit more painful to use (and also to debug in storage) but would better integrate with jackrabbit/jcr. what do you think?

@dbu
Copy link
Member Author

dbu commented Mar 9, 2012

any inputs on my comment whether hashcode should be string or int?

@dbu
Copy link
Member Author

dbu commented Jul 9, 2012

for cross compatibility e.g. with jackrabbit, int seems the better choice to me, so we should change this.

@lsmith77
Copy link
Member

like i said .. i don't know the spec in this part very well .. but if the spec leaves it open, but it looks like using integers will make jackrabbit compat easier, then lets use integer. maybe also double check with Jukka in regards to Oak.

@dbu
Copy link
Member Author

dbu commented Jul 25, 2012

the principal is a port from the java.security.Principal - it is not even part of JCR. i guess we have no choice here but to go with int. i will update the pull request.

dbu added 2 commits July 25, 2012 11:13
…java, add methods to principal and document a bit
Conflicts:
	doc/JCR_TO_PHPCR.txt
dbu added a commit that referenced this pull request Jul 25, 2012
add PrincipalInterface and cleanup docs
@dbu dbu merged commit d9ff4b3 into master Jul 25, 2012
@travisbot
Copy link

This pull request passes (merged 87009cd into 72388ef).

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

3 participants