directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kiran Ayyagari <kayyag...@apache.org>
Subject Re: rm -rf *Decorator ;-)
Date Wed, 03 Oct 2018 14:17:21 GMT
On Wed, Oct 3, 2018 at 7:42 PM Emmanuel LĂ©charny <elecharny@symas.com>
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

Mime
View raw message