You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fortress@directory.apache.org by Chris Pike <cl...@psu.edu> on 2016/11/28 14:22:44 UTC
Re: Groups ftProps
That worked! I think the property manager is ready to merge in...
https://github.com/PennState/directory-fortress-core-1/tree/feature/fc197ManageProperties
----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Tuesday, November 22, 2016 2:23:29 PM
Subject: Re: Groups ftProps
a bit more
1. change the schema:
## OC8: LDAP Configuration Group Structural Object Class
objectClass ( ftObId:8
…
MAY ftProps
)
2. change build.properties
# Use Fortress defined LDAP Group objectclass:
group.properties= ftProps
3. give ‘er a whirl and fix what breaks. ;-)
> On Nov 22, 2016, at 1:13 PM, Shawn McKinney <sm...@apache.org> wrote:
>
>
>> On Nov 22, 2016, at 12:52 PM, Chris Pike <cl...@psu.edu> wrote:
>>
>> Not sure I'm following. I created a PropertyMgr / PropertyDAO that can crud properties on objects that support ftProps (Role, AdminRole, PermObj, Permission). Should it handle Group? If so, how?
>
> Short answer no. Longer answer yes but needs a bit of work. We’d alter the object class to use that property and go from there.
Re: Groups ftProps
Posted by Shawn McKinney <sm...@apache.org>.
> On Nov 28, 2016, at 3:14 PM, Chris Pike <cl...@psu.edu> wrote:
>
> You need to pull the latest... but I went ahead and merged into master and pushed
Just now have verified the latest is good with your changes. Tests pass, artifacts look good. More to come as we work these into the rest and web components.
thx,
Shawn
Re: Groups ftProps
Posted by Chris Pike <cl...@psu.edu>.
You need to pull the latest... but I went ahead and merged into master and pushed
----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Monday, November 28, 2016 3:01:04 PM
Subject: Re: Groups ftProps
> On Nov 28, 2016, at 1:09 PM, Chris Pike <cl...@psu.edu> wrote:
>
> I have never touched the LdapGroupSetup file and it looks correct on my branch, perhaps you are looking at old version of file or looking on wrong branch?
>
> I'll make the other fixes and then merge.
The problem is the copy I had checked out with this:
git checkout feature/fc197ManageProperties
doesn’t have this commit:
https://github.com/PennState/directory-fortress-core-1/commit/c2a8852b76a4d20d2e1a27a1c3208189bcaadd94
Rather than jumping into a rabbit hole we’ll have another look once you’ve got the latest code merged into the trunk.
Thanks,
Shawn
Re: Groups ftProps
Posted by Shawn McKinney <sm...@apache.org>.
> On Nov 28, 2016, at 1:09 PM, Chris Pike <cl...@psu.edu> wrote:
>
> I have never touched the LdapGroupSetup file and it looks correct on my branch, perhaps you are looking at old version of file or looking on wrong branch?
>
> I'll make the other fixes and then merge.
The problem is the copy I had checked out with this:
git checkout feature/fc197ManageProperties
doesn’t have this commit:
https://github.com/PennState/directory-fortress-core-1/commit/c2a8852b76a4d20d2e1a27a1c3208189bcaadd94
Rather than jumping into a rabbit hole we’ll have another look once you’ve got the latest code merged into the trunk.
Thanks,
Shawn
Re: Groups ftProps
Posted by Chris Pike <cl...@psu.edu>.
I have never touched the LdapGroupSetup file and it looks correct on my branch, perhaps you are looking at old version of file or looking on wrong branch?
I'll make the other fixes and then merge.
----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Monday, November 28, 2016 11:28:01 AM
Subject: Re: Groups ftProps
> On Nov 28, 2016, at 10:25 AM, Shawn McKinney <sm...@apache.org> wrote:
>
> and a few small probs need attn:
>
> 1. one of the xml load files have been changed to use improper attribute names, should be:
> <group name="test-group-1" propertieswithcsv=“..." memberswithcsv=“…” />
>
> not:
> <group name="test-group-1" properties=“..." members=“…” />
This is the file:
https://github.com/apache/directory-fortress-core/blob/master/ldap/setup/LdapGroupSetup.xml
Re: Groups ftProps
Posted by Shawn McKinney <sm...@apache.org>.
> On Nov 28, 2016, at 10:25 AM, Shawn McKinney <sm...@apache.org> wrote:
>
> and a few small probs need attn:
>
> 1. one of the xml load files have been changed to use improper attribute names, should be:
> <group name="test-group-1" propertieswithcsv=“..." memberswithcsv=“…” />
>
> not:
> <group name="test-group-1" properties=“..." members=“…” />
This is the file:
https://github.com/apache/directory-fortress-core/blob/master/ldap/setup/LdapGroupSetup.xml
Re: Groups ftProps
Posted by Shawn McKinney <sm...@apache.org>.
Looks good Chris. Before you merge please ensure the java project coding guidelines have been met, e.g. block formatting like this:
name/op
{
…
}
not:
op{
}
I see you have this for some but not all. I realize this coding convention is not satisfactory for all but we’d like to be consistent across the project (above all).
Similarly, please ensure all of the public methods have been javadoced. (again I think you have some if not all of them done)
and a few small probs need attn:
1. one of the xml load files have been changed to use improper attribute names, should be:
<group name="test-group-1" propertieswithcsv=“..." memberswithcsv=“…” />
not:
<group name="test-group-1" properties=“..." members=“…” />
2. ensure the schema for the slapd are updated with ftProps on config group OC. For some reason when I checked out branch, your updated fortress.schema wasn’t correct, although I saw it was changed through the github web interface. Strange, maybe something I did wrong?
3. the schema for apacheds needs same change. Let me know if you need help with this.
Other than these minor nits the code looks nice, kudos! Especially the new tests that have been added, makes me very happy. :-)
Thanks,
Shawn
> On Nov 28, 2016, at 8:22 AM, Chris Pike <cl...@psu.edu> wrote:
>
> That worked! I think the property manager is ready to merge in...
>
> https://github.com/PennState/directory-fortress-core-1/tree/feature/fc197ManageProperties
>
>
>
>
> ----- Original Message -----
> From: "Shawn McKinney" <sm...@apache.org>
> To: fortress@directory.apache.org
> Sent: Tuesday, November 22, 2016 2:23:29 PM
> Subject: Re: Groups ftProps
>
> a bit more
>
> 1. change the schema:
> ## OC8: LDAP Configuration Group Structural Object Class
> objectClass ( ftObId:8
> …
> MAY ftProps
> )
>
>
> 2. change build.properties
>
> # Use Fortress defined LDAP Group objectclass:
> group.properties= ftProps
>
> 3. give ‘er a whirl and fix what breaks. ;-)
>
>> On Nov 22, 2016, at 1:13 PM, Shawn McKinney <sm...@apache.org> wrote:
>>
>>
>>> On Nov 22, 2016, at 12:52 PM, Chris Pike <cl...@psu.edu> wrote:
>>>
>>> Not sure I'm following. I created a PropertyMgr / PropertyDAO that can crud properties on objects that support ftProps (Role, AdminRole, PermObj, Permission). Should it handle Group? If so, how?
>>
>> Short answer no. Longer answer yes but needs a bit of work. We’d alter the object class to use that property and go from there.