Details

    • Type: Improvement
    • Status: Done
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 6.0.0
    • Fix Version/s: 7.0.0
    • Component/s: core apis, tech-debt
    • Labels:
      None

      Description

      Usage of LdapSocket during implementation of LdapServer and LdapClient showed some limitations / bad behaviors of the current API.

      TL;DR:

      Adding the following two methods would reduce significantly the complexity of LdapClient implementations and remove the writer()'s request(Long.MAX_VALUE) hack:

      • Completable onDisconnect()
      • Completable write(LdapMessage message)

      Disconnection management

      Problem

      The current API forces to perform the disconnection management in a Subscriber subscribed to the reader().
      When the ldap socket is disconnected, the reader() gets notified in two different ways:

      • onComplete() is invoked if the connection has been closed remotely,
      • onError() is invoked if the connection has been closed locally.

      Problem is that onComplete() will notify the subscriber after all pending messages have been processed. On the other hand, onError() might be processed immediately.
      This gives two different scenarios that the developer have to keep in mind when using the LdapSocket API. This adds unnecessary complexity.

      Another problem is on the writer() side. Today, when disconnected, LdapSocket invokes request(Long.MAX_VALUE on its publisher. This is semantically wrong: when the connection is closed, the writer should invoke cancel() on its Subscription.

      Solution proposal

      To solve that, a proposal is to extract the disconnection handling from the reader() by adding a new Completable onDisconnect() method to LdapSocket API.

      The Subscribers of this Completable will be notified first (before reader() and writer()) of any disconnection, either by onComplete if the disconnection has been done remotely, or by onError if the disconnection has been done locally.

      On local and remove disconnection, the reader() will always receive an onComplete() notification while the writer() Subscription will be cancel() ed. The order on which these events occur is not defined. But it is guaranteed that these will happen after the notification sent by the Completable returned by onDisconnect().

      Unitary write of message

      Problem

      The current API only offers a writer() method returning a Subscriber. This Subscriber can only be subscribed to one Publisher.

      This works well when the messages to be written are streamed (i.e: proxy and server mode) but it is more challenging to use when the use case is to write message unitary (i.e: client mode).

      The current API forces the developer to create a message queue which will acts as a Publisher and then push the messages needed to be written to the message queue.
      The need for this message queue is the reason why the current writer() does not invoke cancel. Because once cancel() ed, the message queue will silently drop all messages it received. Which would make state management of this message queue event more complex.

      Solution proposal

      The proposal is to add a new Completable write(LdapMessage message) method to the LdapSocket API. It must:

      Allow multiple subscriber on reader()

      Problem: possible to use one or both write mechanism at the same time. Note that given the signature of this method, the back-pressure management for this new method has to be handled externally.

      The returned Completable will be notified once the message has been written, either successfully (onComplete()) or not (onError()).

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                ylecaillez Yannick Lecaillez
                Reporter:
                ylecaillez Yannick Lecaillez
                Dev Assignee:
                Yannick Lecaillez
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: