GNOME Bugzilla – Bug 737712
Add RelaxNG functionality
Last modified: 2014-10-16 11:38:56 UTC
Created attachment 287507 [details] Reworked patch (version 2) As discussed on the libxml++ mailinglist: While upgrading the firmware in our products, I collected the patches we made to open source software over the years, with the intention of submitting these back upstream. One such patch concerns libxml++, which was made by my colleague. This patch adds support for rng validation (RelaxNG) to libxml++. I have attached a reworked version of this patch after comments by Kjell ... 2. If we add RelaxNG validation, we should also add an example in the examples directory. 3. Are the copyright notices in the new files correct, or just copied from other files in libxml++? Is there a person named Ari Johnson, claiming copyright to these files? If not, I recommend copying the copyright notice from one of the newer files in libxml++, such as libxml++/nodes/xincludestart.h, just adjusting the year. ... Copyright notices have been updated, the person named was also named in the original file, hence I will leave him in. I wrote an example based on examples/schemavalidation, but I was unable to test it as mm-common is not installed on my development system here (thus preventing me from generating the configure script, for starters).
Created attachment 287508 [details] Example based on schemavalidation.cc
Created attachment 287707 [details] [review] patch: Add RelaxNG support I've modified your patch. Comments?
Created attachment 287708 [details] [review] patch: Add relaxngvalidation example program patch: Add relaxngvalidation example program You didn't attach input data to the example program. I copied the data from the schemavalidation example and converted the XML schema to a RelaxNG schema with a converter I found at http://debeissat.nicolas.free.fr/XSDtoRNG.php. Have you got better input data? I'll push these two patches unless I get objections within a week or two.
Created attachment 287753 [details] [review] patch: Add RelaxNG support This is a better version of the new classes.
The reworked code for the new classes looks good and should be able to do the job. But the interface for both the schema and validator class differs from the classes implemented for the xsd schema. We tried to keep the interfaces exactly the same for both schemas in the original patch, so developers should be able to interchange both schemas in the code without too much hassle. If you prefer to clean up and refine the interface for the relaxng classes, maybe the xsd schema classes should be reworked as well to match the same interface?
I didn't realize that the tight similarity between the new classes and the XSD classes was a very deliberate design decision. Even to the point of keeping the mis-spelling of the protected variable embbeded_shema_. There are several ugly details in Schema and SchemaValidator, but only some of them can be fixed without breaking API or ABI. Examples of ugly details: - The description of Schema::Schema(Document* document = 0, bool embed = false) says "document XMLSchema document, 0 to create an empty schema document." document = 0 makes the constructor throw an exception, because xmlSchemaParse() fails to parse an empty document. - When I tested Schema::get_name(), get_target_namespace() and get_version(), they all throwed exceptions because they use impl_->name, impl_->targetNamespace and impl_->version without first testing if they are 0. The exceptions can be easily avoided, but I wonder if these methods ever return useful information. Is it important that SchemaRNG is as similar to Schema as possible, and RelaxNGValidator is as similar to SchemaValidator as possible? In my patch in comment 4, RelaxNGValidator is quite similar to SchemaValidator. There are more differences between SchemaRNG and Schema. I can change SchemaRNG::SchemaRNG(const Document& document) SchemaRNG::parse_document(const Document& document) to SchemaRNG::SchemaRNG(const Document* document) SchemaRNG::set_document(const Document* document) if you think that's an improvement. It would make SchemaRNG slightly more similar to Schema.
We would actually prefer to keep the interface for the relaxng schema as defined in your patch (comment 4). If we are going to deviate from the definition from the xsd schema classes we should do it properly. So we suggest to stick to your patch. For us it is not important to have both interfaces the same, we only use relaxng and it should be easy to refactor our code to use the new interface. But, the next step could be refactoring the xsd schemas as well so that they match the relaxng schema classes. You could even think of making an abstract base class for schemas, defining a generic interface. This interface is then implemented by both the relaxng, xsd and any other schema implementation. This will make sure everyone is using the same interface. This will break the API, so the users should be well informed about this change. You could think of making temporary wrapper classes implementing the old interfaces to help users migrate to the new model.
I've also thought about abstract base classes for RelaxNG schemas and XSD schemas. Something like class SchemaBase : public NonCopyable class RelaxNGSchema : public SchemaBase class XsdSchema : public SchemaBase class SchemaValidatorBase : public Validator class RelaxNGValidator : public SchemaValidatorBase class XsdValidator : public SchemaValidatorBase Schema and SchemaValidator can be kept, but marked as deprecated and replaced by XsdSchema and XsdValidator. That would not break API now. Deprecated API is kept until libxml++ 3.0.0 is released. Don't know when that will be. This would be an alternative to the patch in comment 4, not a second step after that patch has been pushed. I'll probably do this. The result would be a better structured program. (This is not the only part of libxml++ that needs refactoring. The whole hierarchy of Node classes is badly structured. That's a much bigger job.)
Ok, sounds like a good idea. I suppose RelaxNG will be added in the xml++ 3.0.0 release as well? Or will this be sooner?
It will be sooner. All the new classes have other names than the old classes. The new classes and the old ones that will be deprecated, can exist simultaneously.
I have pushed two commits that add new classes and deprecate old ones. Add XsdSchema and XsdValidator. Deprecate Schema and SchemaValidator https://git.gnome.org/browse/libxml++/commit/?id=dd71d6319e43229899c6a8146071b5c4a67ed265 Add RelaxNGSchema and RelaxNGValidator https://git.gnome.org/browse/libxml++/commit/?id=cc998a01e532a64f727e2dacabf9d3de5e87c7b0 I close this bug. Feel free to reopen it (or file a new one), if you find bugs in the new classes. It would be fine if anyone else could test them before they are included in a stable libxml++ release.