You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by Shawn McKinney <sm...@apache.org> on 2015/06/01 16:40:19 UTC

Re: [Bulk] Re: [Fortress] Sonar...

> On May 31, 2015, at 5:46 PM, Shawn McKinney <sm...@apache.org> wrote:
> 
>> 
>> On May 31, 2015, at 5:16 PM, Emmanuel Lécharny <el...@gmail.com> wrote:
>> 
>> Can you provide a quick description of the changes ?
> 
> 1. move all of the fortress entities (user, role, permission, etc) from rbac to model package
> 2. move (some of the) common utility classes to under the util package
> 
> Since clients only use the manager and entity objects, their exposure to change will be limited.

My proposal to clean up package cycles breaks backward compatibility and has met some (mild) opposition.  In the interest of not taking a controversial step unilaterally, what is proper course of action to take - vote?

Shawn
smckinney@apache.org

Re: [Bulk] Re: [Fortress] Sonar...

Posted by Shawn McKinney <sm...@apache.org>.
> On Jun 1, 2015, at 10:07 AM, Emmanuel Lécharny <el...@gmail.com> wrote:
> 
> Do be clear : do what you want to do in a branch, and explain what
> exactly you want to do :
> 
> - I'll move entities (ie, User, Role, etc) classes into a
> o.a.d.fortress.core.model package
> - I'll move blah blah...
> 
> That would be perfect for those who need to discuss what you are
> suggesting !

I created a branch (1.0-RC41) and performed the following:

1. moved all entities classes from package org.apache.directory.fortress.core.rbac to org.apache.directory.fortress.core.model

2. moved all implementation classes from package org.apache.directory.fortress.core.rbac to org.apache.directory.fortress.core.impl

Note - this change should have no impact to clients if they are using the Manager interfaces and factories in base package - org.apache.directory.fortress.core

3. eliminated unnecessary utilities like isNotNull.  refactored code to use commons for these functions.

4. split utilities up by package usage.  some utilities now reside in model package.

5. added a couple interfaces to utilities used by model package

Now the Directory Tangle index has reduced to about 11%.  Most of the issues found by SonarQube have been resolved.  The impact to existing clients is minimal.  

Next steps are to notify the fortress users list about the change, merge into trunk.

Instructions to upgrade to fortress 1.0.0:

1. Change package name of fortress entities. 
search and replace: import org.apache.directory.fortress.core.rbac
with: import org.apache.directory.fortress.core.model

2. Change to use Apache Commons StringUtils (instead of fortress utility) to check for null or empty Strings in code. 
replace import org.apache.directory.fortress.core.util.attr.VUtil; with import org.apache.commons.collections.CollectionUtils; 
search and replace: VUtil.isNotNullOrEmpty, with: StringUtils.isNotNull

3. Change to use Apache Commons CollectionUtils (instead of fortress utility) to check for null or empty Collections in code. 
replace import org.apache.directory.fortress.core.util.attr.VUtil; with import org.apache.commons.collections.CollectionUtils;
search and replace: VUtil.isNotNullOrEmpty, with: CollectionUtils.isNotEmpty

4. Always use factories for Manager construction.  Do not construct manually

Don’t do this: AdminMgr adminMgr = new org.apache.directory.fortress.core.impl.AdminMgrImpl();
Do this: AdminMgr adminMgr = AdminMgrFactory.createInstance();

Shawn
smckinney@apache.org

Re: Fortress-Core modularization was Re: [Bulk] Re: [Fortress] Sonar...

Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 01/06/15 17:37, Shawn McKinney a écrit :
>> On Jun 1, 2015, at 10:14 AM, Emmanuel Lécharny <el...@gmail.com> wrote:
>>
>>
>>
>> I would also suggest another approach here. We *could* also split
>> fortress-core in modules (à la Maven) :
>> - an API module that contains the interfaces
>> - a model module
>> - a ldap module
>> - a rest module
>> etc
>>
>> wdyt ?
> Would this require multiple jar files for the core?  

Yes, but they could be bundled into one jar, like what we did with the API.

> Right now I am not be in favor of that.

ATM, not sure that would be necessary, considering the size of
fortress-core.

I'm more in favor in refactoring the existing packages to reflect the
layers we have.


Re: Fortress-Core modularization was Re: [Bulk] Re: [Fortress] Sonar...

Posted by Shawn McKinney <sm...@apache.org>.
> On Jun 1, 2015, at 10:14 AM, Emmanuel Lécharny <el...@gmail.com> wrote:
> 
> 
> 
> I would also suggest another approach here. We *could* also split
> fortress-core in modules (à la Maven) :
> - an API module that contains the interfaces
> - a model module
> - a ldap module
> - a rest module
> etc
> 
> wdyt ?

Would this require multiple jar files for the core?  Right now I am not be in favor of that.

Shawn
smckinney@apache.org

Fortress-Core modularization was Re: [Bulk] Re: [Fortress] Sonar...

Posted by Emmanuel Lécharny <el...@gmail.com>.
I rename this thread for clarity.


I would also suggest another approach here. We *could* also split
fortress-core in modules (à la Maven) :
- an API module that contains the interfaces
- a model module
- a ldap module
- a rest module
etc

wdyt ?

Re: [Bulk] Re: [Fortress] Sonar...

Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 01/06/15 16:56, Emmanuel Lécharny a écrit :
> Le 01/06/15 16:40, Shawn McKinney a écrit :
>>> On May 31, 2015, at 5:46 PM, Shawn McKinney <sm...@apache.org> wrote:
>>>
>>>> On May 31, 2015, at 5:16 PM, Emmanuel Lécharny <el...@gmail.com> wrote:
>>>>
>>>> Can you provide a quick description of the changes ?
>>> 1. move all of the fortress entities (user, role, permission, etc) from rbac to model package
>>> 2. move (some of the) common utility classes to under the util package
>>>
>>> Since clients only use the manager and entity objects, their exposure to change will be limited.
>> My proposal to clean up package cycles breaks backward compatibility and has met some (mild) opposition.  In the interest of not taking a controversial step unilaterally, what is proper course of action to take - vote?
> Build concensus, first. Ie, explain the pros and cons. If the cons are
> just that it breaks backward compatibility, considering we haven't
> reached a GA yet, it's most certainly the last time we can do that
> before the API is frozen.
>
> A vote is a bit strong. I don't think we need it.

Do be clear : do what you want to do in a branch, and explain what
exactly you want to do :

- I'll move entities (ie, User, Role, etc) classes into a
o.a.d.fortress.core.model package
- I'll move blah blah...

That would be perfect for those who need to discuss what you are
suggesting !

Thanks Shawn.


Re: [Bulk] Re: [Fortress] Sonar...

Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 01/06/15 16:40, Shawn McKinney a écrit :
>> On May 31, 2015, at 5:46 PM, Shawn McKinney <sm...@apache.org> wrote:
>>
>>> On May 31, 2015, at 5:16 PM, Emmanuel Lécharny <el...@gmail.com> wrote:
>>>
>>> Can you provide a quick description of the changes ?
>> 1. move all of the fortress entities (user, role, permission, etc) from rbac to model package
>> 2. move (some of the) common utility classes to under the util package
>>
>> Since clients only use the manager and entity objects, their exposure to change will be limited.
> My proposal to clean up package cycles breaks backward compatibility and has met some (mild) opposition.  In the interest of not taking a controversial step unilaterally, what is proper course of action to take - vote?

Build concensus, first. Ie, explain the pros and cons. If the cons are
just that it breaks backward compatibility, considering we haven't
reached a GA yet, it's most certainly the last time we can do that
before the API is frozen.

A vote is a bit strong. I don't think we need it.