Le 16/09/15 10:46, Radovan Semancik a écrit :
> Hi,
>
> I agree, of course.
>
> Just a bit more information. Currently there is a way how to do this
> ... kind of ...
>
> boolean schemaQuirksMode = ....
> DefaultSchemaLoader schemaLoader = new
> DefaultSchemaLoader(connection, schemaQuirksMode);
> DefaultSchemaManager schemaManager = new
> DefaultSchemaManager(schemaLoader);
> try {
> if (schemaQuirksMode) {
> schemaManager.setRelaxed();
> schemaManager.loadAllEnabledRelaxed();
> } else {
> schemaManager.loadAllEnabled();
> }
> } catch (Exception e) {
> throw new ConnectorIOException(e.getMessage(), e);
> }
> if ( !schemaManager.getErrors().isEmpty() ) {
> if (schemaQuirksMode) {
> LOG.ok("There are {0} schema errors, but we
> are in quirks mode so we are ignoring them",
> defSchemaManager.getErrors().size());
> for (Throwable error:
> schemaManager.getErrors()) {
> LOG.ok("Schema error (ignored): {0}: {1}",
> error.getClass().getName(), error.getMessage());
> }
> } else {
> throw new ConnectorIOException("Errors loading
> schema "+schemaManager.getErrors());
> }
> }
>
>
> This code works even with bad LDAP servers. But .... there are some
> drawbacks:
>
> 1) The schemaManager cannot be attached to the connection. I.e. you
> must not call connection.setSchemaManager(schemaManager). Otherwise
> the LDAP operations will fail on many places as the loaded schema is
> inconsistent. I guess that this is mostly OK. If the schema is not
> consistent then it cannot be expected that it will work normally for
> all the cases. But I believe it should work normally unless a the
> specific inconsistency is encountered. I.e. if there are schema
> inconsistencies in an attribute/syntax that is never used then th API
> should work normally even with inconsistent schema.
The pb you can't do that is that when we try to load a schema from a
remote server, we create a temproraty SM which is not relaxed. This is a
mistake that need to be fixed, thus the proposal to extend teh
LdapConnection class with a loadRelaxed method.
>
> 2) Some errors do not appear in the error list
> (schemaManager.getErrors()). E.g. the OID syntax error that I was
> fixing recently. The error handling in schema processing is a bit
> inconsistent. But I think this can be fixed in an evolutionary fashion.
Probably.
>
>
> 3) This all not very user-friendly. The modifications that Emmanuel
> suggested seem to be a good improvements.
>
> 4) We have quirks mode and relaxed mode. I've realized that quicks
> mode is for parsing and relaxed mode is for schema processing and
> validation. But, do we need to expose both to the API user? Maybe
> relaxed mode should imply quirks mode.
I was thinking about something slightly different : a constant that can
take 4 values :
- STRICT_MODE
- QUIRK_MODE
- RELAXED_MODE
- QUIRK_RELAXED_MODE
we can pass that as an argument. OTOH, yes, relaxed might also imply
quick_mode (and to be frank, I prefer 'relaxed' to 'quirk_mode', it's
more explicit).
So, yes, I think your proposal makes sense.
TBH, I first thought that the quirk_mode was implying the relaxed mode
until I realized it waqs different (too long without playing with that
code ...)
>
> To be more specific, I think we need this (ideally):
>
> connection.loadSchema(mode)
>
> ... and internal implementation will set up quirks/relaxed mode as
> appropriate. The current code is not very clean and I think that we
> will need some "leeway" until we figure out what exactly needs to be
> done here. Therefore we should provide a simple facade to API users
> that will not change even if we change in internal interfaces
> (SchemaManager, SchemaLoader).
Sounds good to me. Should we still have a loadSchema() method that
implies a Strict mode ?
>
> To go one step further I guess we also need to clean up the
> SchemaManager and SchemaLoader a bit. E.g. DefaultSchemaManager has
> setRelaxed(), loadAllEnabled() and loadAllEnabledRelaxed(). One of
> these methods seems to be redundant.
Yep. They are hacks.
> I would expect either to use setRelaxed(true) and then
> loadAllEnabled(), but as far as I can remember that does not work.
> Speaking politically correctly this is a little bit of a mess :-) I
> deserves to be cleaned up.
Fully agreed. I *wish* I had time to get this correct the very first
time. I wasn't expecting OpenLDAP or some other LDAP Server to be so
permissive... :/ My bad.
Expect the worse, get ready for it, and rejoy when it's better than expect !
