Re: osync_xmlformat_validate() in format plugins

> The last days I was thinking about that and it is alway better not to
> use any static variables because of concurrent access. But I don't think
> we could avoid this problem without having more schema instances in the
> memory. If we use your solution every format plugin has to create its
> own instance of each schema which the plugin will support. This will be
> better than today but not really nice.
>
> Maybe you can review my code again and test it. I changed the xmlformat
> test suite to fit with the new solution, too. If my code is ok I will
> commit it. Afterwards I take a look at the format plugins and change
> them if necessary.

> Index: opensync/xmlformat/opensync_xmlformat.c
> ===================================================================
> --- opensync/xmlformat/opensync_xmlformat.c     (Revision 3381)
> +++ opensync/xmlformat/opensync_xmlformat.c     (Arbeitskopie)
> @@ -97,6 +97,7 @@
>   * @param path The individual schema path. If NULL the default
> OPENSYNC_SCHEMASDIR is used. * @return TRUE if xmlformat valid else FALSE
>   */
> +/*
>  osync_bool _osync_xmlformat_validate(OSyncXMLFormat *xmlformat, const char
> *path) {
>         osync_assert(xmlformat);
> @@ -111,8 +112,141 @@
>         g_free(schemafilepath);
>
>         return res;
> +} */

> +
> +/**
> + * @brief Create new OSyncXMLFormatSchema for xmlformat
> + * @param xmlformat The pointer to a xmlformat object. xmlformat->objtype
> is used to identify the schema file + * @param path The individual schema
> path. If NULL the default OPENSYNC_SCHEMASDIR is used. + * @param error The
> error which will hold the info in case of an error + * @return new
> OSyncXMLFormatSchema or NULL in case of an error
> + */
> +OSyncXMLFormatSchema * osync_xmlformat_schema_new(OSyncXMLFormat *
> xmlformat, const char *path, OSyncError **error) { +      
> OSyncXMLFormatSchema * osyncschema = NULL;
> +
> +       osync_trace(TRACE_ENTRY, "%s(%p, %p, %p)", __func__, xmlformat,
> path, error); +
> +       osync_assert(xmlformat);
> +
> +       osyncschema = osync_try_malloc0(sizeof(OSyncXMLFormatSchema),
> error); +       if(!osyncschema) {
> +               osync_trace(TRACE_EXIT_ERROR, "%s: %s" , __func__,
> osync_error_print(error)); +               // release mutex
> +               return NULL;
> +       }
> +       osyncschema->objtype =
> g_strdup(osync_xmlformat_get_objtype(xmlformat)); +       char
> *schemafilepath = g_strdup_printf("%s%c%s%s%s",
> +               path ? path : OPENSYNC_SCHEMASDIR,
> +               G_DIR_SEPARATOR,
> +               "xmlformat-",
> +               osyncschema->objtype,
> +               ".xsd");
> +
> +       xmlSchemaParserCtxtPtr xmlSchemaParserCtxt;
> +       xmlSchemaPtr xmlSchema;
> +       xmlSchemaValidCtxtPtr xmlSchemaValidCtxt;
> +
> +       xmlSchemaParserCtxt = xmlSchemaNewParserCtxt(schemafilepath);
> +       osyncschema->schema = xmlSchemaParse(xmlSchemaParserCtxt);
> +       xmlSchemaFreeParserCtxt(xmlSchemaParserCtxt);
> +
> +       osyncschema->context = xmlSchemaNewValidCtxt(osyncschema->schema);
> +       if (osyncschema->context == NULL) {
> +               xmlSchemaFree(osyncschema->schema);
> +               g_free(osyncschema->objtype);
> +               g_free(osyncschema);
> +               osyncschema = NULL;
> +               osync_error_set(error, OSYNC_ERROR_GENERIC, "XMLFormat
> validation failed. Could not create Schema Context."); +       }
> +       return osyncschema;
>  }
>
> +/**
> + * @brief Free existing OSyncXMLFormatSchema
> + * @param osyncschema Pointer to the Schema that shoud be freed
> + * @param error The error which will hold the info in case of an error
> + */
> +void osync_xmlformat_schema_free(OSyncXMLFormatSchema * osyncschema) {
> +
> +       osync_assert(osyncschema);
> +
> +       xmlSchemaFreeValidCtxt(osyncschema->context);
> +       xmlSchemaFree(osyncschema->schema);
> +       g_free(osyncschema->objtype);
> +       g_free(osyncschema);
> +
> +}

> +
> +/**
> + * @brief Get a  schema for the xmlformat.
> + *
> + * This function creates only one instance of a schema for each objtype.
> If a xmlformat is passed as a parameter with the same + * objtype as a
> xmlformat prior the returned pointer to a OSyncXMLFormatSchema instance is
> the same as before. + *
> + * @param xmlformat The pointer to a xmlformat object
> + * @param error The error which will hold the info in case of an error
> + * @return Pointer to a instance of OSyncXMLFormatSchema
> + */
> +
> +OSyncXMLFormatSchema * osync_xmlformat_schema_get_instance(OSyncXMLFormat
> * xmlformat, OSyncError **error) { +       static GList * schemas = NULL;
> +       static GStaticMutex mutex = G_STATIC_MUTEX_INIT;
> +       OSyncXMLFormatSchema * osyncschema = NULL;
> +       GList * entry;
> +       const char * objtype;
> +
> +       osync_trace(TRACE_ENTRY, "%s(%p, %p)", __func__, xmlformat, error);
> +
> +       osync_assert(xmlformat);
> +
> +       objtype = osync_xmlformat_get_objtype(xmlformat);
> +       // get mutex
> +       g_static_mutex_lock(&mutex);
> +       // find schema for objtype
> +       for ( entry = schemas; entry != NULL; entry=entry->next) { //
> should be fast enough for only a few objtypes +               osyncschema =
> (OSyncXMLFormatSchema *) entry->data; +               if
> (!strcmp(osyncschema->objtype, objtype) ) {
> +                       return osyncschema;
> +               }
> +               osyncschema = NULL;
> +       }
> +       if ( osyncschema == NULL ) {
> +               osyncschema = osync_xmlformat_schema_new(xmlformat, NULL,
> error); +               if ( osyncschema != NULL ) {
> +                       schemas = g_list_append(schemas, osyncschema);
> +               }
> +       }
> +       // release mutex
> +       g_static_mutex_unlock(&mutex);
> +       return osyncschema;
> +}
> +
> +/**
> + * @brief Validate the xmlformat against its schema
> + * @param xmlformat The pointer to a xmlformat object
> + * @param schema The pointer to a OSyncXMLFormatSchema object.
> + * @param error The error which will hold the info in case of an error
> + * @return TRUE if xmlformat valid else FALSE
> + */
> +
> +osync_bool osync_xmlformat_schema_validate(OSyncXMLFormatSchema * schema,
> OSyncXMLFormat * xmlformat, OSyncError **error)
> +{

> +       osync_assert(xmlformat);
> +       osync_assert(schema);
> +
> +       int rc = 0;
> +
> +       /* Validate the document */
> +       rc = xmlSchemaValidateDoc(schema->context, xmlformat->doc);
> +
> +       if(rc != 0) {
> +               osync_error_set(error, OSYNC_ERROR_GENERIC, "XMLFormat
> validation failed."); +               return FALSE;
> +       }
> +       return TRUE;
> +}
> +
> +
>  /*@}*/

>
>  /**
> @@ -394,13 +528,15 @@
>  /**
>   * @brief Validate the xmlformat against its schema
>   * @param xmlformat The pointer to a xmlformat object
> + * @param error The error which will hold the info in case of an error
>   * @return TRUE if xmlformat valid else FALSE
>   */
> -osync_bool osync_xmlformat_validate(OSyncXMLFormat *xmlformat)
> +osync_bool osync_xmlformat_validate(OSyncXMLFormat *xmlformat, OSyncError
> **error) {
>         osync_assert(xmlformat);
> -
> -       return _osync_xmlformat_validate(xmlformat, NULL);
> +
> +       OSyncXMLFormatSchema * schema =
> osync_xmlformat_schema_get_instance(xmlformat, error); +       return
> osync_xmlformat_schema_validate(schema, xmlformat, error); }
>
>  /**
> Index: opensync/xmlformat/opensync_xmlformat_internals.h
> ===================================================================
> --- opensync/xmlformat/opensync_xmlformat_internals.h   (Revision 3381)
> +++ opensync/xmlformat/opensync_xmlformat_internals.h   (Arbeitskopie)
> @@ -23,7 +23,9 @@
>  #ifndef OPENSYNC_XMLFORMAT_INTERNALS_H_
>  #define OPENSYNC_XMLFORMAT_INTERNALS_H_
>
> -#include <opensync/opensync_xml.h>
> +#include <libxml/xpath.h>
> +#include <libxml/xmlschemas.h>
> +#include <libxml/tree.h>
>
>  /**
>   * @brief Represent a XMLFormat object
> @@ -45,7 +47,24 @@
>
>  };
>

> +/**
> + * @brief Represents a Schema object
> + * @ingroup OSyncXMLFormatPrivateAPI
> + */
> +typedef struct OSyncXMLFormatSchema {
> +       /** The schema object */
> +       xmlSchemaPtr schema;
> +       /** The schema validation context */
> +       xmlSchemaValidCtxtPtr context;
> +       /** The object type of OSyncXMLFormat */
> +       char * objtype;
> +} OSyncXMLFormatSchema;

> +
>  int _osync_xmlformat_get_points(OSyncXMLPoints points[], int* cur_pos, int
> basic_points, const char* fieldname); -osync_bool
> _osync_xmlformat_validate(OSyncXMLFormat *xmlformat, const char *path);
>
> +OSyncXMLFormatSchema * osync_xmlformat_schema_get_instance(OSyncXMLFormat
> * xmlformat, OSyncError **error); +OSyncXMLFormatSchema *
> osync_xmlformat_schema_new(OSyncXMLFormat * xmlformat, const char *path,
> OSyncError **error); +void osync_xmlformat_schema_free(OSyncXMLFormatSchema
> * schema);

>  #endif /*OPENSYNC_XMLFORMAT_INTERNAL_H_*/
> Index: opensync/xmlformat/opensync_xmlformat.h
> ===================================================================
> --- opensync/xmlformat/opensync_xmlformat.h     (Revision 3381)
> +++ opensync/xmlformat/opensync_xmlformat.h     (Arbeitskopie)
> @@ -48,7 +48,7 @@
>  OSYNC_EXPORT OSyncXMLFieldList
> *osync_xmlformat_search_field(OSyncXMLFormat *xmlformat, const char *name,
> OSyncError **error, ...);
>
>  OSYNC_EXPORT osync_bool osync_xmlformat_assemble(OSyncXMLFormat
> *xmlformat, char **buffer, unsigned int *size); -OSYNC_EXPORT osync_bool
> osync_xmlformat_validate(OSyncXMLFormat *xmlformat); +OSYNC_EXPORT
> osync_bool osync_xmlformat_validate(OSyncXMLFormat *xmlformat, OSyncError
> **error);
>
>  OSYNC_EXPORT void osync_xmlformat_sort(OSyncXMLFormat *xmlformat);
>
> Index: tests/merger-tests/check_xmlformat.c
> ===================================================================
> --- tests/merger-tests/check_xmlformat.c        (Revision 3381)
> +++ tests/merger-tests/check_xmlformat.c        (Arbeitskopie)
> @@ -275,6 +275,7 @@
>         char *buffer;
>         unsigned int size;
>         OSyncError *error = NULL;
> +       OSyncXMLFormatSchema *schema = NULL;
>
>         fail_unless(osync_file_read("event.xml", &buffer, &size, &error),
> NULL); fail_unless(error == NULL, NULL);
> @@ -283,9 +284,13 @@
>         fail_unless(error == NULL, NULL);
>
>         g_free(buffer);
> -
> -       fail_unless(_osync_xmlformat_validate(xmlformat, testbed) != FALSE,
> NULL); -
> +       schema = osync_xmlformat_schema_new(xmlformat, testbed, &error);
> +       fail_if( schema == NULL );
> +       //fail_unless(_osync_xmlformat_validate(xmlformat, testbed) !=
> FALSE, NULL);