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 2018/12/24 09:45:02 UTC
Re: [directory-server] branch master updated: Fix replication tests
Hi,
there were some test failures in replication tests. I "fixed" them by
registering the replication controls manully, see below. But I'm not
sure if that's the right way, or if the controls should be registered
automatically? Seems before it was not necessary to register them manually.
Kind Regards,
Stefan
On 12/24/18 1:19 AM, seelmann@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> seelmann pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/directory-server.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new a967693 Fix replication tests
> a967693 is described below
>
> commit a967693c3458a1bc5d8bb052cf54499ee9eeb189
> Author: Stefan Seelmann <ma...@stefan-seelmann.de>
> AuthorDate: Mon Dec 24 01:18:53 2018 +0100
>
> Fix replication tests
> ---
> .../server/replication/ClientServerReplicationIT.java | 10 ++++++++++
> .../directory/server/replication/MockSyncReplConsumer.java | 2 +-
> .../directory/server/replication/StaleEventLogDetectionIT.java | 10 ++++++++++
> 3 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/server-integ/src/test/java/org/apache/directory/server/replication/ClientServerReplicationIT.java b/server-integ/src/test/java/org/apache/directory/server/replication/ClientServerReplicationIT.java
> index bedb02c..ebec5bb 100644
> --- a/server-integ/src/test/java/org/apache/directory/server/replication/ClientServerReplicationIT.java
> +++ b/server-integ/src/test/java/org/apache/directory/server/replication/ClientServerReplicationIT.java
> @@ -30,6 +30,10 @@ import java.util.List;
> import java.util.concurrent.CountDownLatch;
> import java.util.concurrent.atomic.AtomicInteger;
>
> +import org.apache.directory.api.ldap.codec.api.LdapApiService;
> +import org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncDoneValueFactory;
> +import org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncRequestValueFactory;
> +import org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncStateValueFactory;
> import org.apache.directory.api.ldap.model.constants.SchemaConstants;
> import org.apache.directory.api.ldap.model.csn.Csn;
> import org.apache.directory.api.ldap.model.cursor.Cursor;
> @@ -558,6 +562,12 @@ public class ClientServerReplicationIT
> {
> DirectoryService provDirService = DSAnnotationProcessor.getDirectoryService();
>
> + // Load the replication controls
> + LdapApiService codec = provDirService.getLdapCodecService();
> + codec.registerRequestControl( new SyncRequestValueFactory( codec ) );
> + codec.registerResponseControl( new SyncDoneValueFactory( codec ) );
> + codec.registerResponseControl( new SyncStateValueFactory( codec ) );
> +
> providerServer = ServerAnnotationProcessor.getLdapServer( provDirService );
> providerServer.setReplicationReqHandler( new SyncReplRequestHandler() );
> providerServer.startReplicationProducer();
> diff --git a/server-integ/src/test/java/org/apache/directory/server/replication/MockSyncReplConsumer.java b/server-integ/src/test/java/org/apache/directory/server/replication/MockSyncReplConsumer.java
> index 5e43043..53253c2 100644
> --- a/server-integ/src/test/java/org/apache/directory/server/replication/MockSyncReplConsumer.java
> +++ b/server-integ/src/test/java/org/apache/directory/server/replication/MockSyncReplConsumer.java
> @@ -222,7 +222,7 @@ public class MockSyncReplConsumer implements ConnectionClosedEventListener, Repl
> if ( connection == null )
> {
> connection = new LdapNetworkConnection( providerHost, port );
> - connection.setTimeOut( -1L );
> + connection.setTimeOut( 10000L );
>
> if ( config.isUseTls() )
> {
> diff --git a/server-integ/src/test/java/org/apache/directory/server/replication/StaleEventLogDetectionIT.java b/server-integ/src/test/java/org/apache/directory/server/replication/StaleEventLogDetectionIT.java
> index 0542225..57dd032 100644
> --- a/server-integ/src/test/java/org/apache/directory/server/replication/StaleEventLogDetectionIT.java
> +++ b/server-integ/src/test/java/org/apache/directory/server/replication/StaleEventLogDetectionIT.java
> @@ -29,6 +29,10 @@ import java.io.IOException;
> import java.util.concurrent.atomic.AtomicInteger;
>
> import org.apache.directory.api.util.FileUtils;
> +import org.apache.directory.api.ldap.codec.api.LdapApiService;
> +import org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncDoneValueFactory;
> +import org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncRequestValueFactory;
> +import org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncStateValueFactory;
> import org.apache.directory.api.ldap.model.entry.DefaultEntry;
> import org.apache.directory.api.ldap.model.entry.Entry;
> import org.apache.directory.api.ldap.model.message.SearchRequest;
> @@ -162,6 +166,12 @@ public class StaleEventLogDetectionIT
>
> providerServer = ServerAnnotationProcessor.getLdapServer( provDirService );
>
> + // Load the replication controls
> + LdapApiService codec = provDirService.getLdapCodecService();
> + codec.registerRequestControl( new SyncRequestValueFactory( codec ) );
> + codec.registerResponseControl( new SyncDoneValueFactory( codec ) );
> + codec.registerResponseControl( new SyncStateValueFactory( codec ) );
> +
> providerServer.setReplicationReqHandler( new SyncReplRequestHandler() );
> providerServer.startReplicationProducer();
>
>
Re: [directory-server] branch master updated: Fix replication tests
Posted by Emmanuel Lécharny <el...@gmail.com>.
I would add that we have 2 implementations of the LdapApiService interface:
- DefaultLdapCodecService
- StandaloneLdapApiService
It has its root back in 2011 (so 8 years ago, not 10).
You can find some rational on those links:
https://directory.markmail.org/search/?q=%5BStudio%5D%20date%3A201102%20#query:%5BStudio%5D%20date%3A201102%20+page:1+mid:eodpidchdtxjxjvx+state:results
https://issues.apache.org/jira/browse/DIRSHARED-91
On 24/12/2018 12:21, Emmanuel Lécharny wrote:
> I will have a look at this.
>
>
> I have made some changes in the way controls/extendedOps are loaded in
> the LdapApiService instance, but it may need more tweaking.
>
> It's a bit of a mess, because we have two 'flavors' of controls/extOp :
>
> - those we consider 'stock' elements (ie that belongs to the default API)
>
> - those we classified as 'extras' (mainly because they are only
> present in the API, and not supported by the server, or simply added
> lately to the API).
>
> This classification does not make a lot of sense.
>
> What should be the correct handling of those elements in a OSGi world
> would be to have each one of them being defined as a standalone
> element, loaded as an OSGi bundle. That would allow dynamic addition
> of them on demand, and extensibility of the API/server. There is
> clearly room for improvement here.
>
> I'll will first complete what I'm doing on the API before proposing a
> new way to deal with those extOps/controls; but I can foresee two
> options:
>
> - Make all the existing controls/extOps being part of the default
> LdapAPIService instance, so that you won't need to load them manually
>
> - make each of those elements separate bundles.
>
> The first approach is simpler, and does not preclude the second one,
> especially for new addition in the future.
>
>
> All in all, this part of the code is mainly 10 years old, and you can
> feel its age when working on it... I'm experiencing it every single
> day atm ;-)
>
>
>
> On 24/12/2018 10:45, Stefan Seelmann wrote:
>> Hi,
>>
>> there were some test failures in replication tests. I "fixed" them by
>> registering the replication controls manully, see below. But I'm not
>> sure if that's the right way, or if the controls should be registered
>> automatically? Seems before it was not necessary to register them
>> manually.
>>
>> Kind Regards,
>> Stefan
>>
>>
>> On 12/24/18 1:19 AM, seelmann@apache.org wrote:
>>> This is an automated email from the ASF dual-hosted git repository.
>>>
>>> seelmann pushed a commit to branch master
>>> in repository https://gitbox.apache.org/repos/asf/directory-server.git
>>>
>>>
>>> The following commit(s) were added to refs/heads/master by this push:
>>> new a967693 Fix replication tests
>>> a967693 is described below
>>>
>>> commit a967693c3458a1bc5d8bb052cf54499ee9eeb189
>>> Author: Stefan Seelmann <ma...@stefan-seelmann.de>
>>> AuthorDate: Mon Dec 24 01:18:53 2018 +0100
>>>
>>> Fix replication tests
>>> ---
>>> .../server/replication/ClientServerReplicationIT.java | 10 ++++++++++
>>> .../directory/server/replication/MockSyncReplConsumer.java | 2 +-
>>> .../directory/server/replication/StaleEventLogDetectionIT.java | 10
>>> ++++++++++
>>> 3 files changed, 21 insertions(+), 1 deletion(-)
>>>
>>> diff --git
>>> a/server-integ/src/test/java/org/apache/directory/server/replication/ClientServerReplicationIT.java
>>> b/server-integ/src/test/java/org/apache/directory/server/replication/ClientServerReplicationIT.java
>>>
>>> index bedb02c..ebec5bb 100644
>>> ---
>>> a/server-integ/src/test/java/org/apache/directory/server/replication/ClientServerReplicationIT.java
>>> +++
>>> b/server-integ/src/test/java/org/apache/directory/server/replication/ClientServerReplicationIT.java
>>> @@ -30,6 +30,10 @@ import java.util.List;
>>> import java.util.concurrent.CountDownLatch;
>>> import java.util.concurrent.atomic.AtomicInteger;
>>> +import org.apache.directory.api.ldap.codec.api.LdapApiService;
>>> +import
>>> org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncDoneValueFactory;
>>> +import
>>> org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncRequestValueFactory;
>>> +import
>>> org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncStateValueFactory;
>>> import org.apache.directory.api.ldap.model.constants.SchemaConstants;
>>> import org.apache.directory.api.ldap.model.csn.Csn;
>>> import org.apache.directory.api.ldap.model.cursor.Cursor;
>>> @@ -558,6 +562,12 @@ public class ClientServerReplicationIT
>>> {
>>> DirectoryService provDirService =
>>> DSAnnotationProcessor.getDirectoryService();
>>> + // Load the replication controls
>>> + LdapApiService codec = provDirService.getLdapCodecService();
>>> + codec.registerRequestControl( new SyncRequestValueFactory(
>>> codec ) );
>>> + codec.registerResponseControl( new SyncDoneValueFactory(
>>> codec ) );
>>> + codec.registerResponseControl( new SyncStateValueFactory(
>>> codec ) );
>>> +
>>> providerServer = ServerAnnotationProcessor.getLdapServer(
>>> provDirService );
>>> providerServer.setReplicationReqHandler( new
>>> SyncReplRequestHandler() );
>>> providerServer.startReplicationProducer();
>>> diff --git
>>> a/server-integ/src/test/java/org/apache/directory/server/replication/MockSyncReplConsumer.java
>>> b/server-integ/src/test/java/org/apache/directory/server/replication/MockSyncReplConsumer.java
>>>
>>> index 5e43043..53253c2 100644
>>> ---
>>> a/server-integ/src/test/java/org/apache/directory/server/replication/MockSyncReplConsumer.java
>>> +++
>>> b/server-integ/src/test/java/org/apache/directory/server/replication/MockSyncReplConsumer.java
>>> @@ -222,7 +222,7 @@ public class MockSyncReplConsumer implements
>>> ConnectionClosedEventListener, Repl
>>> if ( connection == null )
>>> {
>>> connection = new LdapNetworkConnection(
>>> providerHost, port );
>>> - connection.setTimeOut( -1L );
>>> + connection.setTimeOut( 10000L );
>>> if ( config.isUseTls() )
>>> {
>>> diff --git
>>> a/server-integ/src/test/java/org/apache/directory/server/replication/StaleEventLogDetectionIT.java
>>> b/server-integ/src/test/java/org/apache/directory/server/replication/StaleEventLogDetectionIT.java
>>>
>>> index 0542225..57dd032 100644
>>> ---
>>> a/server-integ/src/test/java/org/apache/directory/server/replication/StaleEventLogDetectionIT.java
>>> +++
>>> b/server-integ/src/test/java/org/apache/directory/server/replication/StaleEventLogDetectionIT.java
>>> @@ -29,6 +29,10 @@ import java.io.IOException;
>>> import java.util.concurrent.atomic.AtomicInteger;
>>> import org.apache.directory.api.util.FileUtils;
>>> +import org.apache.directory.api.ldap.codec.api.LdapApiService;
>>> +import
>>> org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncDoneValueFactory;
>>> +import
>>> org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncRequestValueFactory;
>>> +import
>>> org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncStateValueFactory;
>>> import org.apache.directory.api.ldap.model.entry.DefaultEntry;
>>> import org.apache.directory.api.ldap.model.entry.Entry;
>>> import org.apache.directory.api.ldap.model.message.SearchRequest;
>>> @@ -162,6 +166,12 @@ public class StaleEventLogDetectionIT
>>> providerServer =
>>> ServerAnnotationProcessor.getLdapServer( provDirService );
>>> + // Load the replication controls
>>> + LdapApiService codec = provDirService.getLdapCodecService();
>>> + codec.registerRequestControl( new SyncRequestValueFactory(
>>> codec ) );
>>> + codec.registerResponseControl( new SyncDoneValueFactory(
>>> codec ) );
>>> + codec.registerResponseControl( new SyncStateValueFactory(
>>> codec ) );
>>> +
>>> providerServer.setReplicationReqHandler( new
>>> SyncReplRequestHandler() );
>>> providerServer.startReplicationProducer();
>>>
Re: [directory-server] branch master updated: Fix replication tests
Posted by Emmanuel Lécharny <el...@gmail.com>.
I will have a look at this.
I have made some changes in the way controls/extendedOps are loaded in
the LdapApiService instance, but it may need more tweaking.
It's a bit of a mess, because we have two 'flavors' of controls/extOp :
- those we consider 'stock' elements (ie that belongs to the default API)
- those we classified as 'extras' (mainly because they are only present
in the API, and not supported by the server, or simply added lately to
the API).
This classification does not make a lot of sense.
What should be the correct handling of those elements in a OSGi world
would be to have each one of them being defined as a standalone element,
loaded as an OSGi bundle. That would allow dynamic addition of them on
demand, and extensibility of the API/server. There is clearly room for
improvement here.
I'll will first complete what I'm doing on the API before proposing a
new way to deal with those extOps/controls; but I can foresee two options:
- Make all the existing controls/extOps being part of the default
LdapAPIService instance, so that you won't need to load them manually
- make each of those elements separate bundles.
The first approach is simpler, and does not preclude the second one,
especially for new addition in the future.
All in all, this part of the code is mainly 10 years old, and you can
feel its age when working on it... I'm experiencing it every single day
atm ;-)
On 24/12/2018 10:45, Stefan Seelmann wrote:
> Hi,
>
> there were some test failures in replication tests. I "fixed" them by
> registering the replication controls manully, see below. But I'm not
> sure if that's the right way, or if the controls should be registered
> automatically? Seems before it was not necessary to register them manually.
>
> Kind Regards,
> Stefan
>
>
> On 12/24/18 1:19 AM, seelmann@apache.org wrote:
>> This is an automated email from the ASF dual-hosted git repository.
>>
>> seelmann pushed a commit to branch master
>> in repository https://gitbox.apache.org/repos/asf/directory-server.git
>>
>>
>> The following commit(s) were added to refs/heads/master by this push:
>> new a967693 Fix replication tests
>> a967693 is described below
>>
>> commit a967693c3458a1bc5d8bb052cf54499ee9eeb189
>> Author: Stefan Seelmann <ma...@stefan-seelmann.de>
>> AuthorDate: Mon Dec 24 01:18:53 2018 +0100
>>
>> Fix replication tests
>> ---
>> .../server/replication/ClientServerReplicationIT.java | 10 ++++++++++
>> .../directory/server/replication/MockSyncReplConsumer.java | 2 +-
>> .../directory/server/replication/StaleEventLogDetectionIT.java | 10 ++++++++++
>> 3 files changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/server-integ/src/test/java/org/apache/directory/server/replication/ClientServerReplicationIT.java b/server-integ/src/test/java/org/apache/directory/server/replication/ClientServerReplicationIT.java
>> index bedb02c..ebec5bb 100644
>> --- a/server-integ/src/test/java/org/apache/directory/server/replication/ClientServerReplicationIT.java
>> +++ b/server-integ/src/test/java/org/apache/directory/server/replication/ClientServerReplicationIT.java
>> @@ -30,6 +30,10 @@ import java.util.List;
>> import java.util.concurrent.CountDownLatch;
>> import java.util.concurrent.atomic.AtomicInteger;
>>
>> +import org.apache.directory.api.ldap.codec.api.LdapApiService;
>> +import org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncDoneValueFactory;
>> +import org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncRequestValueFactory;
>> +import org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncStateValueFactory;
>> import org.apache.directory.api.ldap.model.constants.SchemaConstants;
>> import org.apache.directory.api.ldap.model.csn.Csn;
>> import org.apache.directory.api.ldap.model.cursor.Cursor;
>> @@ -558,6 +562,12 @@ public class ClientServerReplicationIT
>> {
>> DirectoryService provDirService = DSAnnotationProcessor.getDirectoryService();
>>
>> + // Load the replication controls
>> + LdapApiService codec = provDirService.getLdapCodecService();
>> + codec.registerRequestControl( new SyncRequestValueFactory( codec ) );
>> + codec.registerResponseControl( new SyncDoneValueFactory( codec ) );
>> + codec.registerResponseControl( new SyncStateValueFactory( codec ) );
>> +
>> providerServer = ServerAnnotationProcessor.getLdapServer( provDirService );
>> providerServer.setReplicationReqHandler( new SyncReplRequestHandler() );
>> providerServer.startReplicationProducer();
>> diff --git a/server-integ/src/test/java/org/apache/directory/server/replication/MockSyncReplConsumer.java b/server-integ/src/test/java/org/apache/directory/server/replication/MockSyncReplConsumer.java
>> index 5e43043..53253c2 100644
>> --- a/server-integ/src/test/java/org/apache/directory/server/replication/MockSyncReplConsumer.java
>> +++ b/server-integ/src/test/java/org/apache/directory/server/replication/MockSyncReplConsumer.java
>> @@ -222,7 +222,7 @@ public class MockSyncReplConsumer implements ConnectionClosedEventListener, Repl
>> if ( connection == null )
>> {
>> connection = new LdapNetworkConnection( providerHost, port );
>> - connection.setTimeOut( -1L );
>> + connection.setTimeOut( 10000L );
>>
>> if ( config.isUseTls() )
>> {
>> diff --git a/server-integ/src/test/java/org/apache/directory/server/replication/StaleEventLogDetectionIT.java b/server-integ/src/test/java/org/apache/directory/server/replication/StaleEventLogDetectionIT.java
>> index 0542225..57dd032 100644
>> --- a/server-integ/src/test/java/org/apache/directory/server/replication/StaleEventLogDetectionIT.java
>> +++ b/server-integ/src/test/java/org/apache/directory/server/replication/StaleEventLogDetectionIT.java
>> @@ -29,6 +29,10 @@ import java.io.IOException;
>> import java.util.concurrent.atomic.AtomicInteger;
>>
>> import org.apache.directory.api.util.FileUtils;
>> +import org.apache.directory.api.ldap.codec.api.LdapApiService;
>> +import org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncDoneValueFactory;
>> +import org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncRequestValueFactory;
>> +import org.apache.directory.api.ldap.extras.controls.syncrepl_impl.SyncStateValueFactory;
>> import org.apache.directory.api.ldap.model.entry.DefaultEntry;
>> import org.apache.directory.api.ldap.model.entry.Entry;
>> import org.apache.directory.api.ldap.model.message.SearchRequest;
>> @@ -162,6 +166,12 @@ public class StaleEventLogDetectionIT
>>
>> providerServer = ServerAnnotationProcessor.getLdapServer( provDirService );
>>
>> + // Load the replication controls
>> + LdapApiService codec = provDirService.getLdapCodecService();
>> + codec.registerRequestControl( new SyncRequestValueFactory( codec ) );
>> + codec.registerResponseControl( new SyncDoneValueFactory( codec ) );
>> + codec.registerResponseControl( new SyncStateValueFactory( codec ) );
>> +
>> providerServer.setReplicationReqHandler( new SyncReplRequestHandler() );
>> providerServer.startReplicationProducer();
>>
>>