Hi ! finally fixed the OSGi failing test, which was about fixing the Control Factories registration. We have ow two lists, one for requests controls and another for response control.
API tests are passing green. I started testing the server with the updated API, and hit a first wall, due to the LdapMessageContainer not being cleaned up between decoding. That was not catched by API tests, but obviously the server was hit by this mistake almost immediately (as soon as a secod search request is processed, it was 'reusing' the temporary storage in the container which was not cleaned up when the preovious search request was fully decoded). It was easy to fix as soon as I understod the reason for this failure. I'm now facing many errors in server-integ - which means core-integ are passing green ! -. Most of the errors are likely due to the fact that encoding is done using a different order for elements like Attributes or Controls, and test are probably expecting a certain order. I'l fix those failures ASAP. All in all, there is some more work to be done before I can push the modifications (and as of today, we are talking about a 70 000 lines commit ;-) I'll be off for a week starting this saturday, so I'm not sure Il'l be able to close this big refactoring this week, but I will try. Thnaks ! On 2018/10/23 02:10:07, Emmanuel Lécharny <[email protected]> wrote: > 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> > > -- Emmanuel Lecharny Symas.com directory.apache.org
pEpkey.asc
Description: application/pgp-keys
