On 07/09/2018 02:54 PM, Emmanuel Lécharny wrote:
I guess most of the errors are produced by the
DefaultSchemaObjectRegistry class.

If so, I wonder if a more simple solution would'nt be to create a
specific Logger for such a thing as AD, and configure the property file
to mute the other loggers. OTOH, that would require to either tune the
property file *before* connecting to an AD server (and that means we
*know* we are talking to such a turd), or that we programatically
configure the logger (which might not be easy considering we use a
facade like slf4j).

Exactly. That's what I thought. And also won't work if you want to connect to nice LDAP and naughty LDAP at the same time. Loggers are essentially singletons. So unless someone invents quantum logger they cannot be both on on off at the same time :-)

Now, using a listener solve both drawbacks, we just have to pay the
price of an indirection (ok, I assume it's not a terrible big deal), and
to have a way to configure this listener.

We have to pay the price only if the listener is configured. Otherwise we will just pay price of "if (listener != null) {}" check. Not a bad price to pay for getting rid of complaints about AD schema errors.

One last solution : pass a flag (like 'MUTE_WARNINGS') to the API while
processing a schema, and have something like :

             if ( LOG.isWarnEnabled() && !muteWarnings )
             {
                 LOG.warn( msg );
             }

We would initiate the flag when instanciating the registries:

     public Registries( boolean muteWarnings )
     {
         globalOidRegistry = new OidRegistry<>();
         attributeTypeRegistry = new DefaultAttributeTypeRegistry(
muteWarnings );
         comparatorRegistry = new DefaultComparatorRegistry( muteWarnings );
         ...
     }

Whatever works, just suggesting an alternative idea, not pretending it's
better, but offering options.

I think the flag is redundant. Setting a listener to non-null value will do the same trick:

if (errorListener == null) {
    LOG.warn( msg );
} else {
    errorListener.accept(msg);
}

I have do dig deeper in the code to figure out how to hide this in some superclass or util class, so we won't repeat the code everywhere. But I think this could work. And in fact, it may even be a compatible API change.

The XXXRegistry classes are not to be used from external callers. The
Registries class can be extended, by adding a parameter, and the
constructor with no parameter will simply works as designed initally.
This would change the API ina  very minimal way (being compatible with
teh current version)

Oh yes. That may be the way. Good idea.

2.0.0-AM1 is pretty much done. I intend to cut a release this week, so I
guess it's a bit late to introduce such a change, except if you think
you can cut it very quickly. To be franck, I've a lot to do on my day
job atm, and the baby is draining the remaining energy I have, so I was
planning to cut this release this week-end, so that leave you some room
for such a change. We just have to be sure that does not break the
server, but I don't think I can't fix any breakage there in more than a
couple of hours. Same for studio.

I'm not sure either. But I'll try on Wednesday or Thursday. If I can't make it I just won't merge the changes to master and it won't be in AM1. It looks like we can make this change compatible with previous API, so probably no problem there.

Bottom line, this is not such a big change, if properly handled - and I
trust your skill here :-) -

Thank you. I'll try to to live up to expectations :-)

If you come to realize it's going to be a bigger breakage than what I
think, then I'd rather call for a 3.0. IMO, 2.0 is almost dne, and I'm
willing to move to a faster numbering release, à la Chrome/Firefox. Ie,
instead of cutting some 2.0.1, 2.1.0 etc, I'd ratcher go for 3.0/4.0/5.0
etc...

Oh good. I like faster versioning. Let's see how the change looks like when I'm done. I'll keep it on a branch. It is not really critical do get it in 2.0 if 3.0 will be released soon enough.

Java 11 is going t be out in 2 months and a half. We already have some
early-access versions (http://jdk.java.net/11/) and I know that The ASF
has already configured Jenkins to support Java 11.

I think I'll be a bit conservative here and wait for Java 11 release. I know that Oracle has made promises. But please forgive me if I'm not entirely ready to believe :-)
I'm quite happy with Java 8 for API 2.0.

--
Radovan Semancik
Software Architect
evolveum.com

Reply via email to