directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel LĂ©charny <elecha...@symas.com>
Subject rm -rf *Decorator ;-)
Date Wed, 03 Oct 2018 14:12:46 GMT
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 !


Mime
View raw message