Ok, more about the failing OSGi test. The PasswordPolicy draft (https://tools.ietf.org/html/draft-behera-ldap-password-policy-10#page-24) defines two different controls with the same OID. They are logical distinguished by the fact that those controls are sent with a request or a response.
It's a bit problematic to have used teh same OID, but, well, we have to deal with it. We always know in which context (Request or Response) we are processing controls, so it's easy to know which PasswordPolicy factory to use. The only problem is that control factories are all stored in a single Map. It should be enough to store the controls in two maps, one for requests and another for responses, to get this problem fixed. Of course, we will have to tell in which context we are creating the controlto get the correct factory. I'll get that fix soon. On 2018/10/23 01:48:15, Emmanuel Lécharny <[email protected]> wrote: > Done !> > > All decorators have been removed, and I have the build working (almost) !> > > Ok, I wrote 'almost' because there is a couple of glitches I need to> > address :> > > o first, there is a failing OSGi test.> > > This test, ApiLdapExtrasCodecOsgiTest, tries to get the> > PasswordPolicyRequest control from the LdapApiService instance. the> > problem being I have 2 factories for the same OID: one for the request,> > one for the response. Obviously, that does not fly. The RFC draft is the> > root cause of this issue, as it defines both the request and response> > OID to be the same, but nevertheless, we should be able to fetch the> > correct request or response from the LdapAPIService. One idea would be> > to merge both factories as it was before. All in all, we *know* which of> > the request/response we want when we create them...> > > o second, we have some random failures (NPE) when running the tests.> > This is most certainly due to the concurrency test tools we are using,> > and some initialisation issue (typically, we are doing some things in> > #Before that should be done in #BeforeClass). I'll have a look at that.> > > Finally, many tests have been added for the Message encoding an decoding.> > > It also worths mentionning that the result of this cleanup is that more> > than 17,500 lines of code have been removed...> > > > > Le 07/10/2018 à 16:40, Emmanuel Lécharny a écrit :> > > 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> > > -- Emmanuel Lecharny Symas.com directory.apache.org
pEpkey.asc
Description: application/pgp-keys
