A quick update on this sunny sunday afternoon, while travelling to Tübingen to join the OpenLDAP conference :
- all the LDAP messages decorator have been removed except the IntermediateResponse and the Extended Request/Response - atm, I'm dealing with controls that need to be encoded too. The idea is to delegate this encoding to the control factory. - DSML will need some love Overall, beside the tests that eeded a bit of refactoring, the Decorator removal was quite a breeze, except for the Search Request ne (because of Filters...). We still have the huge performance gain, but also a amazing code reduction : 8000 lines less :-) (I'm talking about SLOCs here, not about blanck lines or comments which are not counted). I think this couldend with 10 000 lines being removed, 5% of the current code base ! So I think we are in good shape. I probably need a few more insomnia and late night to get it completed. I'll keep you updated ! Le 03/10/2018 à 16:12, Emmanuel Lécharny a écrit : > 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. > > 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). > > 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 ! > -- Emmanuel Lecharny Symas.com directory.apache.org
