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 <[email protected]> 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" <[email protected]>
> To: [email protected]
> 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 <[email protected]> wrote:
>>
>>
>>> On Nov 22, 2016, at 12:52 PM, Chris Pike <[email protected]> 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.
