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/01/05 17:35:05 UTC

Multiple Set Methods on Models

There are a couple instances where models have multiple set methods for the same field. 

UserAdminRole has...

    public void setOsP( String osP )
    public void setOsP( Set<String> osPs )

    public void setOsU( Set<String> osUs )
    public void setOsU( String osU )

User has...

    public void setRole( String roleName )    
    public void setRole( UserRole role )

    public void setAdminRole( UserAdminRole role )
    public void setAdminRole( String roleName )

This is causing some issues down the line with jackson. I have a workaround but wanted to see if there was any possibility of changing one of the method names.

Thanks,

~Chris

Re: Multiple Set Methods on Models

Posted by Shawn McKinney <sm...@apache.org>.
> On Jan 6, 2016, at 10:43 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> I already had that set to openldap, might try to reprovision my VM and start fresh.

For openldap testing, take a look at this:
https://github.com/apache/directory-fortress-core/blob/master/README-QUICKSTART-SLAPD.md

The new slapd.properties.example file being the key takeaway.  It captures the settings from the old openldap quickstarts and contains the openldap specific parameters to use with the latest fortress core.  Not many changes from what you had before.  Take care to make sure your setup includes these params:

########################################################################
# 2. BEGIN OPENLDAP SERVER CONFIGURATION SECTION: (Ignore if using HTTP or ApacheDS):
####################################################################################

# Used for slapd logger connection pool.  Leave zeros when using apacheds:
min.log.conn=1
max.log.conn=3

#These are passwords used for LDAP audit log service accounts:
# Audit Pool:
log.admin.user=cn=Manager,${log.suffix}
log.admin.pw=secret

# Use if ldap.server.type=openldap.  (Default is false):
#disable.audit=false

# This OpenLDAP slapd logger password is bound for slapd.conf and was encrypted using 'slappasswd' command:
log.root.pw={SSHA}pSOV2TpCxj2NMACijkcMko4fGrFopctU

# More Audit Config:
log.suffix=cn=log
log.ops=logops bind writes compare

# don’t enable accelerator (yet):
rbac.accelerator=false

Re: Multiple Set Methods on Models

Posted by Chris Pike <cl...@psu.edu>.
I already had that set to openldap, might try to reprovision my VM and start fresh.


----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Wednesday, January 6, 2016 10:39:11 AM
Subject: Re: Multiple Set Methods on Models

> On Jan 6, 2016, at 9:32 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> Adding the disable.audit=false didn't fix the issue. Does it matter if I'm using openldap or apacheds?

Yes it does.  If you are using apachds add this flag:

ldap.server.type=apacheds

For openldap this one:

ldap.server.type=openldap

When the server type is apacheds, the junit tests won’t run the pw policy or audit tests.

Shawn

Re: Multiple Set Methods on Models

Posted by Shawn McKinney <sm...@apache.org>.
> On Jan 6, 2016, at 9:32 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> Adding the disable.audit=false didn't fix the issue. Does it matter if I'm using openldap or apacheds?

Yes it does.  If you are using apachds add this flag:

ldap.server.type=apacheds

For openldap this one:

ldap.server.type=openldap

When the server type is apacheds, the junit tests won’t run the pw policy or audit tests.

Shawn


Re: Multiple Set Methods on Models

Posted by Chris Pike <cl...@psu.edu>.
Adding the disable.audit=false didn't fix the issue. Does it matter if I'm using openldap or apacheds?


----- Original Message -----
From: "Chris Pike" <cl...@psu.edu>
To: fortress@directory.apache.org
Sent: Wednesday, January 6, 2016 10:18:33 AM
Subject: Re: Multiple Set Methods on Models

----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Wednesday, January 6, 2016 9:58:24 AM
Subject: Re: Multiple Set Methods on Models

> On Jan 6, 2016, at 7:33 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> I made the change and submitted a pull request on github, however I got an error when trying to run the junit tests. I reverted the code and still get the same error when running the test so it is not an issue with something I changed. I put the error in the github comment.

Yes I saw this comment on the pull request:

> On Jan 5, 2016, at 12:47 PM, pike1212 <gi...@git.apache.org> wrote:
> 
>  I renamed the duplicate set methods. My junit tests don't work even before I made the change, so not sure how to propertly test. This is the error I get when running "mvn test -Dtest=FortressJUnitTest"
> 
> 
>    Tests run: 141, Failures: 5, Errors: 0, Skipped: 0, Time elapsed: 228.481 sec <<< FAILURE! - in org.apache.directory.fortress.core.impl.FortressJUnitTest
>    testSearchBinds(org.apache.directory.fortress.core.impl.AuditMgrImplTest)  Time elapsed: 0.022 sec  <<< FAILURE!
>    junit.framework.AssertionFailedError: org.apache.directory.fortress.core.impl.AuditMgrImplTestsearchBinds failed search for successful authentication user [jtsUser1]
>    	at junit.framework.Assert.fail(Assert.java:57)
>    	at junit.framework.Assert.assertTrue(Assert.java:22)
>    	at junit.framework.TestCase.assertTrue(TestCase.java:192)
>    	at org.apache.directory.fortress.core.impl.AuditMgrImplTest.searchBinds(AuditMgrImplTest.java:426)
>    	at org.apache.directory.fortress.core.impl.AuditMgrImplTest.testSearchBinds(AuditMgrImplTest.java:399)

Which prolly means your audit isn’t enabled to log events.  I recently changed the flags in the build.properties to address this issue of audit being inadvertently disabled in the Jenkins build tests.  

This is the old flag:
enable.audit=true

which has been inverted to:
disable.audit=false

And the default for disable.audit is (was intended to be) ‘false’.  Please make sure that it has been added to your build.properties and run the regression tests again. 

Thanks,

Shawn

Re: Multiple Set Methods on Models

Posted by Shawn McKinney <sm...@apache.org>.
> On Jan 6, 2016, at 7:33 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> I made the change and submitted a pull request on github, however I got an error when trying to run the junit tests. I reverted the code and still get the same error when running the test so it is not an issue with something I changed. I put the error in the github comment.

Yes I saw this comment on the pull request:

> On Jan 5, 2016, at 12:47 PM, pike1212 <gi...@git.apache.org> wrote:
> 
>  I renamed the duplicate set methods. My junit tests don't work even before I made the change, so not sure how to propertly test. This is the error I get when running "mvn test -Dtest=FortressJUnitTest"
> 
> 
>    Tests run: 141, Failures: 5, Errors: 0, Skipped: 0, Time elapsed: 228.481 sec <<< FAILURE! - in org.apache.directory.fortress.core.impl.FortressJUnitTest
>    testSearchBinds(org.apache.directory.fortress.core.impl.AuditMgrImplTest)  Time elapsed: 0.022 sec  <<< FAILURE!
>    junit.framework.AssertionFailedError: org.apache.directory.fortress.core.impl.AuditMgrImplTestsearchBinds failed search for successful authentication user [jtsUser1]
>    	at junit.framework.Assert.fail(Assert.java:57)
>    	at junit.framework.Assert.assertTrue(Assert.java:22)
>    	at junit.framework.TestCase.assertTrue(TestCase.java:192)
>    	at org.apache.directory.fortress.core.impl.AuditMgrImplTest.searchBinds(AuditMgrImplTest.java:426)
>    	at org.apache.directory.fortress.core.impl.AuditMgrImplTest.testSearchBinds(AuditMgrImplTest.java:399)

Which prolly means your audit isn’t enabled to log events.  I recently changed the flags in the build.properties to address this issue of audit being inadvertently disabled in the Jenkins build tests.  

This is the old flag:
enable.audit=true

which has been inverted to:
disable.audit=false

And the default for disable.audit is (was intended to be) ‘false’.  Please make sure that it has been added to your build.properties and run the regression tests again. 

Thanks,

Shawn


Re: Multiple Set Methods on Models

Posted by Shawn McKinney <sm...@apache.org>.
> 
> On Jan 8, 2016, at 2:05 PM, Chris Pike <cl...@psu.edu> wrote:
> 
> No need to redo it, github could have automatically merged the pull request.  I rebased our branch, so now the commit history will look like the changes in the pull request came after all of your javadoc cleanup.

OK Good.  I just accepted the pull request.  Another change to method name - used ‘set’ in the name instead of ‘list’ for obvious reasons.  Let me know if you have any problems.

> 
> On Jan 8, 2016, at 2:05 PM, Chris Pike <cl...@psu.edu> wrote:
> 
> No need to redo it, github could have automatically merged the pull request.  I rebased our branch, so now the commit history will look like the changes in the pull request came after all of your javadoc cleanup.
> 
> I still haven't been able to get the audit manager tests to pass, regardless of how I change the build.properties, the same 5 tests always fail.

We need to get you past this somehow.  I recommend we hop on IRC sometime next week and figure it out.

Thanks for the help,

Shawn

Re: Multiple Set Methods on Models

Posted by Chris Pike <cl...@psu.edu>.
No need to redo it, github could have automatically merged the pull request.  I rebased our branch, so now the commit history will look like the changes in the pull request came after all of your javadoc cleanup.

I still haven't been able to get the audit manager tests to pass, regardless of how I change the build.properties, the same 5 tests always fail.



----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Friday, January 8, 2016 12:31:29 PM
Subject: Re: Multiple Set Methods on Models

> On Jan 6, 2016, at 7:33 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> I made the change and submitted a pull request on github, however I got an error when trying to run the junit tests. I reverted the code and still get the same error when running the test so it is not an issue with something I changed. I put the error in the github comment.

Chris this pull request came in the middle of a javadoc cleanup effort led by Emmanuel.  Rather than trying to merge it back with those changes I propose we redo it.  Let me know if you feel otherwise.

Here is original request:

> On Jan 5, 2016, at 10:35 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> There are a couple instances where models have multiple set methods for the same field. 
> 
> UserAdminRole has...
> 
>    public void setOsP( String osP )
>    public void setOsP( Set<String> osPs )
> 
>    public void setOsU( Set<String> osUs )
>    public void setOsU( String osU )
> 
> User has...
> 
>    public void setRole( String roleName )    
>    public void setRole( UserRole role )
> 
>    public void setAdminRole( UserAdminRole role )
>    public void setAdminRole( String roleName )
> 
> This is causing some issues down the line with jackson. I have a workaround but wanted to see if there was any possibility of changing one of the method names.


Here are proposed changes as they stand now (w/ suggestion on using list in the name):

UserAdminRole has...

   public void setOsP( String osP )
   public void setOsPList( Set<String> osPs )

   public void setOsUList( Set<String> osUs )
   public void setOsU( String osU )

User has...

   public void setRoleName( String roleName )    
   public void setRole( UserRole role )

   public void setAdminRole( UserAdminRole role )
   public void setAdminRoleName( String roleName )

Will this work for you?  If we’re in agreement I can make the change.  

Shawn

Re: Multiple Set Methods on Models

Posted by Shawn McKinney <sm...@apache.org>.
> On Jan 6, 2016, at 7:33 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> I made the change and submitted a pull request on github, however I got an error when trying to run the junit tests. I reverted the code and still get the same error when running the test so it is not an issue with something I changed. I put the error in the github comment.

Chris this pull request came in the middle of a javadoc cleanup effort led by Emmanuel.  Rather than trying to merge it back with those changes I propose we redo it.  Let me know if you feel otherwise.

Here is original request:

> On Jan 5, 2016, at 10:35 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> There are a couple instances where models have multiple set methods for the same field. 
> 
> UserAdminRole has...
> 
>    public void setOsP( String osP )
>    public void setOsP( Set<String> osPs )
> 
>    public void setOsU( Set<String> osUs )
>    public void setOsU( String osU )
> 
> User has...
> 
>    public void setRole( String roleName )    
>    public void setRole( UserRole role )
> 
>    public void setAdminRole( UserAdminRole role )
>    public void setAdminRole( String roleName )
> 
> This is causing some issues down the line with jackson. I have a workaround but wanted to see if there was any possibility of changing one of the method names.


Here are proposed changes as they stand now (w/ suggestion on using list in the name):

UserAdminRole has...

   public void setOsP( String osP )
   public void setOsPList( Set<String> osPs )

   public void setOsUList( Set<String> osUs )
   public void setOsU( String osU )

User has...

   public void setRoleName( String roleName )    
   public void setRole( UserRole role )

   public void setAdminRole( UserAdminRole role )
   public void setAdminRoleName( String roleName )

Will this work for you?  If we’re in agreement I can make the change.  

Shawn

Re: Multiple Set Methods on Models

Posted by Chris Pike <cl...@psu.edu>.
I made the change and submitted a pull request on github, however I got an error when trying to run the junit tests. I reverted the code and still get the same error when running the test so it is not an issue with something I changed. I put the error in the github comment.


----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Tuesday, January 5, 2016 2:07:09 PM
Subject: Re: Multiple Set Methods on Models

> On Jan 5, 2016, at 11:31 AM, SHAWN E SMITH <se...@psu.edu> wrote:
> 
> Can I make a small suggestion?  Rather than setOsPs, use something like setOsPList.  I used to work with a visually impaired developer and picking a a trailing "s" was very difficult for him.
> 
> If I'm being too technical let me know ;-)

Works for me!

Re: Multiple Set Methods on Models

Posted by Shawn McKinney <sm...@apache.org>.
> On Jan 5, 2016, at 11:31 AM, SHAWN E SMITH <se...@psu.edu> wrote:
> 
> Can I make a small suggestion?  Rather than setOsPs, use something like setOsPList.  I used to work with a visually impaired developer and picking a a trailing "s" was very difficult for him.
> 
> If I'm being too technical let me know ;-)

Works for me!


Re: Multiple Set Methods on Models

Posted by SHAWN E SMITH <se...@psu.edu>.
Can I make a small suggestion?  Rather than setOsPs, use something like setOsPList.  I used to work with a visually impaired developer and picking a a trailing "s" was very difficult for him.

If I'm being too technical let me know ;-)

"The programmer … works only slightly removed from pure thought-stuff.
He builds his castles in the air, from air, creating by exertion of the imagination."
— Fred Brooks

Shawn Smith
Director of Software Engineering
Administrative Information Services
Penn State University
814-321-5227
ses44@psu.edu

----- Original Message -----
From: "Shawn McKinney" <sm...@apache.org>
To: fortress@directory.apache.org
Sent: Tuesday, January 5, 2016 11:52:13 AM
Subject: Re: Multiple Set Methods on Models

> On Jan 5, 2016, at 10:47 AM, Shawn McKinney <sm...@apache.org> wrote:
> 
> Here are my recommendation for new names:
> 
> User:
> setRoleRaw( String roleName );
> setAdminRoleRaw( String roleName );

Correction.  Use these names:
setRoleName
setAdminRoleName

Shawn

Re: Multiple Set Methods on Models

Posted by Shawn McKinney <sm...@apache.org>.

> On Jan 5, 2016, at 10:47 AM, Shawn McKinney <sm...@apache.org> wrote:
> 
> Here are my recommendation for new names:
> 
> User:
> setRoleRaw( String roleName );
> setAdminRoleRaw( String roleName );

Correction.  Use these names:
setRoleName
setAdminRoleName

Shawn

Re: Multiple Set Methods on Models

Posted by Shawn McKinney <sm...@apache.org>.
> On Jan 5, 2016, at 10:35 AM, Chris Pike <cl...@psu.edu> wrote:
> 
> There are a couple instances where models have multiple set methods for the same field. 
> 
> UserAdminRole has...
> 
>    public void setOsP( String osP )
>    public void setOsP( Set<String> osPs )
> 
>    public void setOsU( Set<String> osUs )
>    public void setOsU( String osU )
> 
> User has...
> 
>    public void setRole( String roleName )    
>    public void setRole( UserRole role )
> 
>    public void setAdminRole( UserAdminRole role )
>    public void setAdminRole( String roleName )
> 
> This is causing some issues down the line with jackson. I have a workaround but wanted to see if there was any possibility of changing one of the method names.

I don’t have any objections.  Before committing it needs to pass tests.  

This will require:
a. run fortress junit tests
b. run fortress junit tests via rest (by setting enable.mgr.impl.rest=true)
c. run fortress web selenium tests

Here are my recommendation for new names:

User:
setRoleRaw( String roleName );
setAdminRoleRaw( String roleName );

UserAdminRole:
setOsPs( Set<String> osPs );
setOsUs( Set<String> osUs );

The other methods can remain as they are.

Shawn