directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lécharny <elecha...@gmail.com>
Subject Update : rm -rf *Decorator ;-)
Date Sun, 07 Oct 2018 14:40:04 GMT
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


Mime
View raw message