Le 11/07/2018 à 17:40, Radovan Semancik a écrit : > Hi, > > I have pushed my changes. It is pretty simple from the user point of view: > > defaultSchemaManager.setErrorHandler(schemaErrorHandler); > > I have removed old List<Throwable> errors and replaced it with > SchemaErrorHandler interface. I have figured that it is actually OK to > pay a cost of indirection here. It will only impact the servers with bad > schema, and those servers deserve to suffer anyway, right? :-) The real > reason was that by always using SchemaErrorHandler (and providing > default logging implemenation) I was able to somehow simplify and unify > error handling.
Good. > > However, I have encountered two issues: > > 1) There are two dissociateFromSchema() methods in Registries. One of > them is used, the other seems not to be used at all. The difference was > in the parameters. When I removed the "errors" parameter then those > method clashed. They seem to do quite different things, so they probably > should not have the same name anyway (if both are really used). So I > have renamed the wrong one to dissociateFromSchemaQuestionable(). > Emmanual, this is probably something you should have a look at. I feel a > bit lost here. Whoaa... You are doing some archeology here :-) It's almost a 10 years old code (OMG, I can't believe I wrote that already 9 years ago...), and my guess is that when I added the list of throwable, I forgot to remove the other method, which hasn't eveolved since then, which explain why the code is so different. I thi,k we are safe if you simply remove the method you have renamed... > > 2) Ava. I think Ava is supposed to be a simple object, almost like a > DTO. Not sure what you mean by DTO in this context... Because it's serializable ? It is a bit tangled with SchemaManager ... but I haven't found any > good way how to implement flexible logging in Ava (I do not want to > pollute SchemaManager interface with SchemaErrorHandler). Ava are used in many parts of the server, this makes it a bit complex. Typically, we need to have a schema awat-re Ava in the server, while the client might not have any schema. I'm not sure we need flexible logging in Ava though: it's enough to throw an exception with the error message, as we do. But I have > realized that Ava is such a simple object, it probably should not log > the errors at all. I don't disagree... It was helpfull when I wrote the class, because there are so many corner cases to deal wth, and the logs helped me back then. Now, it's a bit spurious. It is throwing exceptions anyway. So I think it would > be responsibility of the caller to log errors (exceptions). So I have > removed the logging statements from Ava constructors. I hope that this > is OK. I do think it's OK. I have also found potential issues with Ava > comparison/normalization .... I have put a comment in the source code. > This is probably something to have a look in the future. Actually, it's the equals() method, and in the comparison done when we have a SchemaManager, we have to assume the value of both Ava has already been normalized. This try-catch is a bit superfluous, we most certainly can get rid of it... > > Overall, the solution is not perfect. But I'm quite happy with that for > 2.0. It is definitely better than the temporary hack that we had in 1.0. Good job. Have you tried to build the server with those changes ? If not, I will do that tomorrow evening. Thanks Radovan ! -- Emmanuel Lecharny Symas.com directory.apache.org
pEpkey.asc
Description: application/pgp-keys
