On Wed, Oct 3, 2018 at 7:42 PM Emmanuel Lécharny <[email protected]> wrote:
> Hi ! > > a quick mail as a follow up of my last night insomnias... > > > Last week-end I wa scompleting my rewrite of the Message encoding part. > The gain is clear : > - simpler code (way simpler !!!) > - faster code (way faster, too, 2.5x average) > > At the end of this refactoring, I faced some issues with the Controls. > > Controls are handled in a bit specific way : > - we may have 'unknown' controls, which have to be accepted by the API > - we use a factory to create them > - they have a value that itself may need to be decoded and encoded. > > All in all, some inconsistencies pointed their nose, and some of the > tests were simply failing (ClassCastException and such things...) > > I tried hard to draw the global Message hierarchy, same for Controls, > but at the end of the day, the Decorator additions makes a full mess of it. > > I remember Alex reaction when he discovered those Decorator additions, > which was kind of "what the HECK is THAT ??? Ok, your choice, I'm not > going to touch it with a 10 feet pole..." (kind of. He may have been > less polite...) > > 6 or 7 years later (I don't exactly remember), the whole stuff seems to > me an insanity. > > > Let's see why we added it (mainly following my lead) > - At this time Pierre-Arnaud was working on implementing DSML > - There are heavy simularity, and it sounded like a 'good idea' (read : > 'really bad idea') to add a decorator to hide the encoding/decoding parts > - There was no reason to expose the codec logic in LdapMessage > - We wanted to decouple the encoding/decoding from the LdapMessage > implementation, so that it was possible to encode in DSML or BER or > anything (JSON anyone ?). > > The last point was quite appealing. > Yeah, but we aren't writing Spring framework :) > > The problem is that the implementation was really a nightmare (and still > is). Anyone who wants to add a new extendedOperation or a nex Control > has to go through many classes and is likely to get lost (I experienced > it last month while implementing transactions). > and I had gone through the same while writing an extendedoperation for certificate signing. > > Anyway, if you look at the LdapMessage current hierarchy (50 interfaces, > 16 abstract classes, 13 final classes, 107 classes…), it simply makes no > sense. > > > So I'm currenly trying to get rid of all those Decorator non-sense. > > The idea is to have the message contain the method to get the encoded > value at first. The decoding is still delegated to the codec package and > for Controls, we use their factory for that purpose. > > Later on, I will move the encode() method out of the Ldap Message > inmplementation to the LdapEncoder class, as encoding just need to have > access to the messages data, which is exposed by the message interface. > That would decouple encoding from the implementation (this will also > allow the encoding in DSML or whatever, we will just need a DsmlEncoder, > etc). > > > At this point, it's still an experiment, but I'm pleased by the result > so far. I'll keep going up to the point I have something that passes > tests green for teh API *and* the server. > > Studio should not be impacted, nor should Fortress. > > Expect this work to take a couple of weeks ! > > Thanks for taking care of it Emmanuel. > Thanks ! > > Kiran
