You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by Stefan Seelmann <ma...@stefan-seelmann.de> on 2015/05/27 07:55:48 UTC
Re: svn commit: r1681745 - /directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
On 05/26/2015 02:10 PM, semancik@apache.org wrote:
> Author: semancik
> Date: Tue May 26 12:10:25 2015
> New Revision: 1681745
>
> URL: http://svn.apache.org/r1681745
> Log:
> Check if we really need to specify newSuperior in moveAndRename(...)
> newSuperior is optional [RFC4511, section 4.9]. Some servers (e.g. OpenDJ 2.6) require a special privilege if
> newSuperior is specified even if it is the same as the old one. Therefore let's not specify it if we do not need it.
> This is better interoperability.
>
> Modified:
> directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
>
> Modified: directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
> URL: http://svn.apache.org/viewvc/directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java?rev=1681745&r1=1681744&r2=1681745&view=diff
> ==============================================================================
> --- directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java (original)
> +++ directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java Tue May 26 12:10:25 2015
> @@ -2625,7 +2625,18 @@ public class LdapNetworkConnection exten
> ModifyDnRequest modDnRequest = new ModifyDnRequestImpl();
> modDnRequest.setName( entryDn );
> modDnRequest.setNewRdn( newDn.getRdn() );
> - modDnRequest.setNewSuperior( newDn.getParent() );
> +
> + // Check if we really need to specify newSuperior.
> + // newSuperior is optional [RFC4511, section 4.9]
> + // Some servers (e.g. OpenDJ 2.6) require a special privilege if
> + // newSuperior is specified even if it is the same as the old one. Therefore let's not
> + // specify it if we do not need it. This is better interoperability.
> + Dn newDnParent = entryDn.getParent();
I think this should be
Dn newDnParent = newDn.getParent();
> + if ( newDnParent != null && !newDnParent.equals( newDn.getParent() ) )
And here:
if ( newDnParent != null && !newDnParent.equals( entryDn.getParent() ) )
> + {
> + modDnRequest.setNewSuperior( newDnParent );
> + }
> +
Otherwise the old parent is used. See also test failures on Jenkins:
https://builds.apache.org/job/dir-apacheds-ubuntu-deploy/1718/
> modDnRequest.setDeleteOldRdn( deleteOldRdn );
>
> ModifyDnResponse modifyDnResponse = modifyDn( modDnRequest );
>
>
Re: svn commit: r1681745 - /directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 27/05/15 23:01, Stefan Seelmann a écrit :
> On 05/27/2015 11:44 AM, Kiran Ayyagari wrote:
>> On Wed, May 27, 2015 at 5:36 PM, Radovan Semancik <
>>> Well, it looks like it is covered by some tests as they have failed in
>>> Jenkins:
>>> https://builds.apache.org/job/dir-apacheds-ubuntu-deploy/1718/org.apache.directory.server$ldap-client-test/
>>>
>>> So, my question is how can I run these tests on my machine before a commit?
>>>
>>> I'm still new here so I'm trying to learn the development process that you
>>> are usually following. What tests are you usually running before commit? Do
>>> you run something else than "mvn clean install"? Or do you just run "mvn
>>> clean install", commit and then check Jenkins after the commit?
>>>
>> just mvn clean install
>>
>> otoh, I see why it wasn't failed when you ran the tests, the API
>> integration tests are in ldap-client-test module
>> you should run mvn clean install there as well after modifying client.
>>
>> A general rule we all follow is to build the whole server after any change,
>> that way the whole integration
>> test suite gets executed
> Yes, it is a bit unfortune that the integration tests of the LDAP API
> resist in the ApacheDS project. We discussed that multiple times before,
> but always came to the conclusion to keep them there.
We could have created a separate projects for those tests, but that
would have meant some more complexity. And we didn't want to have them
being part of the LDAP API, because then that would mean the LDAP API
would depend on ApacheDS, which depends on the API...
Re: svn commit: r1681745 - /directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 28/05/15 10:11, Radovan Semancik a écrit :
> On 05/27/2015 11:01 PM, Stefan Seelmann wrote:
>> Yes, it is a bit unfortune that the integration tests of the LDAP API
>> resist in the ApacheDS project. We discussed that multiple times
>> before, but always came to the conclusion to keep them there. Kind
>> Regards, Stefan
>
> I don't see that as a problem. Now I can even see the reasons for that
> ... especially as we have a very similar approach in midPoint and also
> for some ConnId connectors :-)
Ideally, we could create a sub projects for the client API test, that
uses ApacheDS as a dependency, because launching teh ApacheDS test is
quite expensive (30 mins).
>
> Maybe it just needs to be mentioned in the Devel Guide on the site.
> Maybe we should add a section "Before You Commit" or something like
> that. I can draft it. But I have no idea how to modify the site.
That's quite simple : everything is stored in Markdown format. Let me
write a page on the site that explains how to update the site, that
would be more helpful than explaining it in this mail !
i'll be back later with a link.
>
> I was also thinking about updating the API documentation. Is there any
> documentation describing how to update the documentation? :-)
Same thing than my previous response...
Tahnks radovan.
Re: svn commit: r1681745 - /directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 28/05/15 10:11, Radovan Semancik a écrit :I can draft it. But I have
no idea how to modify the site.
>
> I was also thinking about updating the API documentation. Is there any
> documentation describing how to update the documentation? :-)
http://directory.staging.apache.org/sources.html, look at the end of the
page (Web Site)
Re: svn commit: r1681745 - /directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
Posted by Radovan Semancik <ra...@evolveum.com>.
On 05/27/2015 11:01 PM, Stefan Seelmann wrote:
> Yes, it is a bit unfortune that the integration tests of the LDAP API
> resist in the ApacheDS project. We discussed that multiple times
> before, but always came to the conclusion to keep them there. Kind
> Regards, Stefan
I don't see that as a problem. Now I can even see the reasons for that
... especially as we have a very similar approach in midPoint and also
for some ConnId connectors :-)
Maybe it just needs to be mentioned in the Devel Guide on the site.
Maybe we should add a section "Before You Commit" or something like
that. I can draft it. But I have no idea how to modify the site.
I was also thinking about updating the API documentation. Is there any
documentation describing how to update the documentation? :-)
--
Radovan Semancik
Software Architect
evolveum.com
Re: svn commit: r1681745 - /directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
Posted by Stefan Seelmann <ma...@stefan-seelmann.de>.
On 05/27/2015 11:44 AM, Kiran Ayyagari wrote:
> On Wed, May 27, 2015 at 5:36 PM, Radovan Semancik <
>> Well, it looks like it is covered by some tests as they have failed in
>> Jenkins:
>> https://builds.apache.org/job/dir-apacheds-ubuntu-deploy/1718/org.apache.directory.server$ldap-client-test/
>>
>> So, my question is how can I run these tests on my machine before a commit?
>>
>> I'm still new here so I'm trying to learn the development process that you
>> are usually following. What tests are you usually running before commit? Do
>> you run something else than "mvn clean install"? Or do you just run "mvn
>> clean install", commit and then check Jenkins after the commit?
>>
> just mvn clean install
>
> otoh, I see why it wasn't failed when you ran the tests, the API
> integration tests are in ldap-client-test module
> you should run mvn clean install there as well after modifying client.
>
> A general rule we all follow is to build the whole server after any change,
> that way the whole integration
> test suite gets executed
Yes, it is a bit unfortune that the integration tests of the LDAP API
resist in the ApacheDS project. We discussed that multiple times before,
but always came to the conclusion to keep them there.
Kind Regards,
Stefan
Re: svn commit: r1681745 - /directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
Posted by Kiran Ayyagari <ka...@apache.org>.
On Wed, May 27, 2015 at 5:36 PM, Radovan Semancik <
radovan.semancik@evolveum.com> wrote:
> On 05/27/2015 11:19 AM, Kiran Ayyagari wrote:
>
>
>> The strange thing is that I have run all the API tests before committing.
>> I'm sure about this. And the tests haven't failed. I had run "mvn clean
>> install" with the trunk of "shared" before the fix. And again no failure.
>
> my guess is that this case is not covered by the existing tests,
> probably due to identical input
> for newDn and entryDn
>
>
> Well, it looks like it is covered by some tests as they have failed in
> Jenkins:
> https://builds.apache.org/job/dir-apacheds-ubuntu-deploy/1718/org.apache.directory.server$ldap-client-test/
>
> So, my question is how can I run these tests on my machine before a commit?
>
> I'm still new here so I'm trying to learn the development process that you
> are usually following. What tests are you usually running before commit? Do
> you run something else than "mvn clean install"? Or do you just run "mvn
> clean install", commit and then check Jenkins after the commit?
>
just mvn clean install
otoh, I see why it wasn't failed when you ran the tests, the API
integration tests are in ldap-client-test module
you should run mvn clean install there as well after modifying client.
A general rule we all follow is to build the whole server after any change,
that way the whole integration
test suite gets executed
> --
> Radovan Semancik
> Software Architectevolveum.com
>
>
--
Kiran Ayyagari
http://keydap.com
Re: svn commit: r1681745 - /directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
Posted by Radovan Semancik <ra...@evolveum.com>.
On 05/27/2015 11:19 AM, Kiran Ayyagari wrote:
>
>
> The strange thing is that I have run all the API tests before
> committing. I'm sure about this. And the tests haven't failed. I
> had run "mvn clean install" with the trunk of "shared" before the
> fix. And again no failure.
>
> my guess is that this case is not covered by the existing tests,
> probably due to identical input
> for newDn and entryDn
Well, it looks like it is covered by some tests as they have failed in
Jenkins:
https://builds.apache.org/job/dir-apacheds-ubuntu-deploy/1718/org.apache.directory.server$ldap-client-test/
So, my question is how can I run these tests on my machine before a commit?
I'm still new here so I'm trying to learn the development process that
you are usually following. What tests are you usually running before
commit? Do you run something else than "mvn clean install"? Or do you
just run "mvn clean install", commit and then check Jenkins after the
commit?
--
Radovan Semancik
Software Architect
evolveum.com
Re: svn commit: r1681745 - /directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
Posted by Kiran Ayyagari <ka...@apache.org>.
On Wed, May 27, 2015 at 5:15 PM, Radovan Semancik <
radovan.semancik@evolveum.com> wrote:
> Hi,
>
> Oh yes. You are right. Sorry for that. Thanks for pointing this out. Fixed
> in rev.1681938.
>
> The strange thing is that I have run all the API tests before committing.
> I'm sure about this. And the tests haven't failed. I had run "mvn clean
> install" with the trunk of "shared" before the fix. And again no failure.
my guess is that this case is not covered by the existing tests, probably
due to identical input
for newDn and entryDn
> Some tests are obviously executed during "clean install". Should I somehow
> run other tests explicitly? (other project? or a different profile?)
>
the best is to add a new test or pickup a related existing test and modify
to cover this case
>
> --
>
> Radovan Semancik
> Software Architect
> evolveum.com
>
>
>
> On 05/27/2015 07:55 AM, Stefan Seelmann wrote:
>
>> On 05/26/2015 02:10 PM, semancik@apache.org wrote:
>>
>>> Author: semancik
>>> Date: Tue May 26 12:10:25 2015
>>> New Revision: 1681745
>>>
>>> URL: http://svn.apache.org/r1681745
>>> Log:
>>> Check if we really need to specify newSuperior in moveAndRename(...)
>>> newSuperior is optional [RFC4511, section 4.9]. Some servers (e.g.
>>> OpenDJ 2.6) require a special privilege if
>>> newSuperior is specified even if it is the same as the old one.
>>> Therefore let's not specify it if we do not need it.
>>> This is better interoperability.
>>>
>>> Modified:
>>>
>>> directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
>>>
>>> Modified:
>>> directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
>>> URL:
>>> http://svn.apache.org/viewvc/directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java?rev=1681745&r1=1681744&r2=1681745&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
>>> (original)
>>> +++
>>> directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
>>> Tue May 26 12:10:25 2015
>>> @@ -2625,7 +2625,18 @@ public class LdapNetworkConnection exten
>>> ModifyDnRequest modDnRequest = new ModifyDnRequestImpl();
>>> modDnRequest.setName( entryDn );
>>> modDnRequest.setNewRdn( newDn.getRdn() );
>>> - modDnRequest.setNewSuperior( newDn.getParent() );
>>> +
>>> + // Check if we really need to specify newSuperior.
>>> + // newSuperior is optional [RFC4511, section 4.9]
>>> + // Some servers (e.g. OpenDJ 2.6) require a special privilege if
>>> + // newSuperior is specified even if it is the same as the old
>>> one. Therefore let's not
>>> + // specify it if we do not need it. This is better
>>> interoperability.
>>> + Dn newDnParent = entryDn.getParent();
>>>
>> I think this should be
>>
>> Dn newDnParent = newDn.getParent();
>>
>> + if ( newDnParent != null && !newDnParent.equals(
>>> newDn.getParent() ) )
>>>
>> And here:
>>
>> if ( newDnParent != null && !newDnParent.equals( entryDn.getParent()
>> ) )
>>
>> + {
>>> + modDnRequest.setNewSuperior( newDnParent );
>>> + }
>>> +
>>>
>> Otherwise the old parent is used. See also test failures on Jenkins:
>> https://builds.apache.org/job/dir-apacheds-ubuntu-deploy/1718/
>>
>> modDnRequest.setDeleteOldRdn( deleteOldRdn );
>>> ModifyDnResponse modifyDnResponse = modifyDn( modDnRequest );
>>>
>>>
>>>
>
>
--
Kiran Ayyagari
http://keydap.com
Re: svn commit: r1681745 - /directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
Posted by Radovan Semancik <ra...@evolveum.com>.
Hi,
Oh yes. You are right. Sorry for that. Thanks for pointing this out.
Fixed in rev.1681938.
The strange thing is that I have run all the API tests before
committing. I'm sure about this. And the tests haven't failed. I had run
"mvn clean install" with the trunk of "shared" before the fix. And again
no failure. Some tests are obviously executed during "clean install".
Should I somehow run other tests explicitly? (other project? or a
different profile?)
--
Radovan Semancik
Software Architect
evolveum.com
On 05/27/2015 07:55 AM, Stefan Seelmann wrote:
> On 05/26/2015 02:10 PM, semancik@apache.org wrote:
>> Author: semancik
>> Date: Tue May 26 12:10:25 2015
>> New Revision: 1681745
>>
>> URL: http://svn.apache.org/r1681745
>> Log:
>> Check if we really need to specify newSuperior in moveAndRename(...)
>> newSuperior is optional [RFC4511, section 4.9]. Some servers (e.g. OpenDJ 2.6) require a special privilege if
>> newSuperior is specified even if it is the same as the old one. Therefore let's not specify it if we do not need it.
>> This is better interoperability.
>>
>> Modified:
>> directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
>>
>> Modified: directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java
>> URL: http://svn.apache.org/viewvc/directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java?rev=1681745&r1=1681744&r2=1681745&view=diff
>> ==============================================================================
>> --- directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java (original)
>> +++ directory/shared/trunk/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java Tue May 26 12:10:25 2015
>> @@ -2625,7 +2625,18 @@ public class LdapNetworkConnection exten
>> ModifyDnRequest modDnRequest = new ModifyDnRequestImpl();
>> modDnRequest.setName( entryDn );
>> modDnRequest.setNewRdn( newDn.getRdn() );
>> - modDnRequest.setNewSuperior( newDn.getParent() );
>> +
>> + // Check if we really need to specify newSuperior.
>> + // newSuperior is optional [RFC4511, section 4.9]
>> + // Some servers (e.g. OpenDJ 2.6) require a special privilege if
>> + // newSuperior is specified even if it is the same as the old one. Therefore let's not
>> + // specify it if we do not need it. This is better interoperability.
>> + Dn newDnParent = entryDn.getParent();
> I think this should be
>
> Dn newDnParent = newDn.getParent();
>
>> + if ( newDnParent != null && !newDnParent.equals( newDn.getParent() ) )
> And here:
>
> if ( newDnParent != null && !newDnParent.equals( entryDn.getParent() ) )
>
>> + {
>> + modDnRequest.setNewSuperior( newDnParent );
>> + }
>> +
> Otherwise the old parent is used. See also test failures on Jenkins:
> https://builds.apache.org/job/dir-apacheds-ubuntu-deploy/1718/
>
>> modDnRequest.setDeleteOldRdn( deleteOldRdn );
>>
>> ModifyDnResponse modifyDnResponse = modifyDn( modDnRequest );
>>
>>