osync_xmlformat_validate() in format plugins

[Daniel Gollub]

Hi,

i just found that osync_xmlformat_validate() is quite expensive, since it
parses for each call the XSLT schemas from scratch (xmlSchemaParse()).

Callgraph of sync with vformat plugin and the vcard_to_xmlformat converter:
http://cryptomilch.de/~dgollub/OpenSync/opensync_profiling_xmlformat_validate.png
(Look for the redline in on the left bottom of the callgraph - 991x stands for
the 991 function calls.)

Actually we could do this XSLT schema parsing within in the OpenSync framework
once, and validate all xmlformats which got reproted with the same object
from a single xmlSchemaParse() call. This would require some changes for
osync_xmlformat_*() - i try to prepare a patch within the next days. This
would save us 990 of 991 xmlSchemaParse() calls in mentioned example. One
call for each used xmlformat. If someone would sync contacts and events this
would end up in two xmlSchemParse() calls for xmlformat-event and
xmlformat-contact....

Anyway- for now i would recommend to remove osync_xmlformat_validate() calls
from every format plugin converter function. It's O.K. for debugging tools
like "vconvert", but the plugin format conversion routines should be fast ;)

I already removed osync_xmlformat_validate() from the vformat plugin and from
gnokii-format and palm-format plugins. AFAIK there is only opie-sync and
sync4j-sync (is anyone still using this?) left which have
osync_xmlformat_validate(), at least in the OpenSync SVN - didn't checked yet
other format plugins.

best regards,
Daniel

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Daniel Gollub]

On Saturday 03 May 2008 17:16:45 Daniel Gollub wrote:
> i just found that osync_xmlformat_validate() is quite expensive, since it
> parses for each call the XSLT schemas from scratch (xmlSchemaParse()).
Ooops, XML schema - abbrev. XSD .. not XSLT
[...]

> Actually we could do this XSLT schema parsing within in the OpenSync
> framework once, and validate all xmlformats which got reproted with the
Again, XML schema - abbrev. XSD .. not XSLT
[...]

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Bjšoern Ricks]

Hi Daniel,

I started coding to get rid of osync_xml_validate and to have only one
instance of a xml schema for each objtype. Therefore I tried to reuse
the oop singleton pattern in C via a static GList. Could you please
review my first intuition because I am more familiar with C++ than C.

best regards,
Bjoern

> Hi,
>
> i just found that osync_xmlformat_validate() is quite expensive, since it
> parses for each call the XSLT schemas from scratch (xmlSchemaParse()).
>
> Callgraph of sync with vformat plugin and the vcard_to_xmlformat converter:
> http://cryptomilch.de/~dgollub/OpenSync/opensync_profiling_xmlformat_validate.png
> (Look for the redline in on the left bottom of the callgraph - 991x stands for
> the 991 function calls.)
>
> Actually we could do this XSLT schema parsing within in the OpenSync framework
> once, and validate all xmlformats which got reproted with the same object
> from a single xmlSchemaParse() call. This would require some changes for
> osync_xmlformat_*() - i try to prepare a patch within the next days. This
> would save us 990 of 991 xmlSchemaParse() calls in mentioned example. One
> call for each used xmlformat. If someone would sync contacts and events this
> would end up in two xmlSchemParse() calls for xmlformat-event and
> xmlformat-contact....
>
> Anyway- for now i would recommend to remove osync_xmlformat_validate() calls
> from every format plugin converter function. It's O.K. for debugging tools
> like "vconvert", but the plugin format conversion routines should be fast ;)
>
> I already removed osync_xmlformat_validate() from the vformat plugin and from
> gnokii-format and palm-format plugins. AFAIK there is only opie-sync and
> sync4j-sync (is anyone still using this?) left which have
> osync_xmlformat_validate(), at least in the OpenSync SVN - didn't checked yet
> other format plugins.
>
> best regards,
> Daniel
>  

--
/Bjoern Ricks

Index: opensync_xmlformat.c
===================================================================
--- opensync_xmlformat.c (Revision 3370)
+++ opensync_xmlformat.c (Arbeitskopie)
@@ -91,6 +91,80 @@
  return p;
 }
 
+OSyncXMLFormatSchema * osync_xmlformat_schema_get_instance(OSyncXMLFormat * xmlformat, OSyncError **error) {
+ static GList * schemas = NULL;
+ 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);
+ //TODO get 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_try_malloc0(sizeof(OSyncXMLFormatSchema), error);
+ if(!osyncschema) {
+ osync_trace(TRACE_EXIT_ERROR, "%s: %s" , __func__, osync_error_print(error));
+ //TODO release mutex
+ return NULL;
+ }
+ osyncschema->objtype = g_strdup(objtype);
+ char *schemafilepath = g_strdup_printf("%s%c%s%s%s",
+ 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(xmlSchema);
+ if (osyncschema->context == NULL) {
+ xmlSchemaFree(osyncschema->schema);
+ g_free(osyncschema->objtype);
+ g_free(osyncschema);
+ osyncschema = NULL;
+ //TODO set error
+ }
+ schemas = g_list_append(schemas, osyncschema);
+ }
+ //TODO release mutex
+ return osyncschema;
+
+}
+
+
+osync_bool osync_xmlformat_schema_validate(OSyncXMLFormatSchema * schema, OSyncXMLFormat * xmlformat)
+{
+ osync_assert(xmlformat);
+ osync_assert(schema);
+
+ int rc = 0;
+
+ /* Validate the document */
+ rc = xmlSchemaValidateDoc(schema->context, xmlformat->doc);
+
+ if(rc != 0)
+ return FALSE;
+ return TRUE;
+}
+
 /**
  * @brief Validate the xmlformat against its schema in inidivual path
  * @param xmlformat The pointer to a xmlformat object
@@ -399,8 +473,9 @@
 osync_bool osync_xmlformat_validate(OSyncXMLFormat *xmlformat)
 {
  osync_assert(xmlformat);
-
- return _osync_xmlformat_validate(xmlformat, NULL);
+
+ OSyncXMLFormatSchema * schema = osync_xmlformat_schema_get_instance(xmlformat, NULL);
+ return osync_xmlformat_schema_validate(schema, xmlformat);
 }
 
 /**
Index: opensync_xmlformat_internals.h
===================================================================
--- opensync_xmlformat_internals.h (Revision 3370)
+++ opensync_xmlformat_internals.h (Arbeitskopie)
@@ -45,7 +45,20 @@
 
 };
 
+struct OSyncXMLFormatSchema {
+
+ xmlSchemaPtr schema;
+
+ xmlSchemaValidCtxtPtr context;
+
+ const char *objtype
+};
+
 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)
+void osync_xmlformat_schema_free_instance(OSyncXMLFormatSchema *  schema, OSyncError **error);
+osync_bool osync_xmlformat_schema_validate(OSyncXMLFormatSchema * schema, OSyncXMLFormat * xmlformat);
+
 #endif /*OPENSYNC_XMLFORMAT_INTERNAL_H_*/

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Daniel Gollub]

On Thursday 19 June 2008 16:18:02 Bjoern Ricks wrote:
> I started coding to get rid of osync_xml_validate and to have only one
> instance of a xml schema for each objtype. Therefore I tried to reuse
> the oop singleton pattern in C via a static GList. Could you please
> review my first intuition because I am more familiar with C++ than C.

Thanks for looking into this!

The idea of using C++-singleton-like sounds interesting. I wonder if we would
change some more stuff, if this would be still required. For example make all
OSyncXMLFormat "inherit" of OSyncXMLFormatSchema? We just force/require for
all XMLFormats a schema file, by changing osync_xmlformat_new parameter list
to:

osync_xmlformat_new(const char *objtype, OSyncXMLFormatSchema *schema,
OSyncError **error);

And putting an OSyncXMLFormatSchema pointer into the internal struct.

Thoughts?

[...]

>  /**
>   * @brief Validate the xmlformat against its schema in inidivual path
>   * @param xmlformat The pointer to a xmlformat object
> @@ -399,8 +473,9 @@
>  osync_bool osync_xmlformat_validate(OSyncXMLFormat *xmlformat)
>  {
>         osync_assert(xmlformat);
> -
> -       return _osync_xmlformat_validate(xmlformat, NULL);
> +
> +       OSyncXMLFormatSchema * schema =
> osync_xmlformat_schema_get_instance(xmlformat, NULL); +       return
> osync_xmlformat_schema_validate(schema, xmlformat);

We should avoid code which is ignoring the error handling by passing NULL
instead of a OSyncError reference. (I know there is/was some code doing
so ... but we should get rid of those old code. It's just bad/wrong/....).
Feel free to change the paramter list vor the _validate() function to
(OSyncXMLFormat *xmlformat, OSyncError **error). You might set also an error
when osync_xmlformat_schema_validate is false:
osync_error_set(*error, OSYNC_ERROR_GENERIC /*or something different */, "XML
Format validation failed.");

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Bjšoern Ricks]

> On Thursday 19 June 2008 16:18:02 Bjoern Ricks wrote:
>  
>> I started coding to get rid of osync_xml_validate and to have only one
>> instance of a xml schema for each objtype. Therefore I tried to reuse
>> the oop singleton pattern in C via a static GList. Could you please
>> review my first intuition because I am more familiar with C++ than C.
>>    
>
> Thanks for looking into this!
>
> The idea of using C++-singleton-like sounds interesting. I wonder if we would
> change some more stuff, if this would be still required. For example make all
> OSyncXMLFormat "inherit" of OSyncXMLFormatSchema? We just force/require for
> all XMLFormats a schema file, by changing osync_xmlformat_new parameter list
> to:
>
> osync_xmlformat_new(const char *objtype, OSyncXMLFormatSchema *schema,
> OSyncError **error);
>
> And putting an OSyncXMLFormatSchema pointer into the internal struct.
>
> Thoughts?
>
> [...]
>  

That could be a better solution than to use static variables for each
objtype. I was thinking about that too. But I didn't dare to change the
parameter list.
But the new solution has another problem. Where should the
OSyncXMLFormatSchemas be created (and released)? We need only one
instance per objtype. Is there one osnyc engine per application? Then
the engine could be the right part to create OSyncXMLFormatSchemas.

>
>  
>>  /**
>>   * @brief Validate the xmlformat against its schema in inidivual path
>>   * @param xmlformat The pointer to a xmlformat object
>> @@ -399,8 +473,9 @@
>>  osync_bool osync_xmlformat_validate(OSyncXMLFormat *xmlformat)
>>  {
>>         osync_assert(xmlformat);
>> -
>> -       return _osync_xmlformat_validate(xmlformat, NULL);
>> +
>> +       OSyncXMLFormatSchema * schema =
>> osync_xmlformat_schema_get_instance(xmlformat, NULL); +       return
>> osync_xmlformat_schema_validate(schema, xmlformat);
>>    
>
> We should avoid code which is ignoring the error handling by passing NULL
> instead of a OSyncError reference. (I know there is/was some code doing
> so ... but we should get rid of those old code. It's just bad/wrong/....).
> Feel free to change the paramter list vor the _validate() function to
> (OSyncXMLFormat *xmlformat, OSyncError **error). You might set also an error
> when osync_xmlformat_schema_validate is false:
> osync_error_set(*error, OSYNC_ERROR_GENERIC /*or something different */, "XML
> Format validation failed.");
>  

:-) I was irritated because osync_xmlformat_validate had no error
parameter. Therefore I had to use a NULL value. So we should change the
function signature.
--
------------------------------------------------------------------------
University of Applied Sciences Osnabrueck
Faculty of Engineering and Computer Science
Dipl.-Inf. (FH) Bjoern Ricks

P.O. Box 1940 - 49009 Osnabrueck - Germany
Phone: [phone] Fax: [phone]
Email: b.ricks@... <mailto:b.ricks@...>
Web: http://www.ecs.fh-osnabrueck.de/ricks.html

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Bjšoern Ricks]

Hi Daniel,

> On Thursday 19 June 2008 16:18:02 Bjoern Ricks wrote:
>  
>> I started coding to get rid of osync_xml_validate and to have only one
>> instance of a xml schema for each objtype. Therefore I tried to reuse
>> the oop singleton pattern in C via a static GList. Could you please
>> review my first intuition because I am more familiar with C++ than C.
>>    
>
> Thanks for looking into this!
>  

You are welcome ;-)

> The idea of using C++-singleton-like sounds interesting. I wonder if we would
> change some more stuff, if this would be still required. For example make all
> OSyncXMLFormat "inherit" of OSyncXMLFormatSchema? We just force/require for
> all XMLFormats a schema file, by changing osync_xmlformat_new parameter list
> to:
>
> osync_xmlformat_new(const char *objtype, OSyncXMLFormatSchema *schema,
> OSyncError **error);
>
> And putting an OSyncXMLFormatSchema pointer into the internal struct.
>
> Thoughts?
>
>
>  

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.

>>  /**
>>   * @brief Validate the xmlformat against its schema in inidivual path
>>   * @param xmlformat The pointer to a xmlformat object
>> @@ -399,8 +473,9 @@
>>  osync_bool osync_xmlformat_validate(OSyncXMLFormat *xmlformat)
>>  {
>>         osync_assert(xmlformat);
>> -
>> -       return _osync_xmlformat_validate(xmlformat, NULL);
>> +
>> +       OSyncXMLFormatSchema * schema =
>> osync_xmlformat_schema_get_instance(xmlformat, NULL); +       return
>> osync_xmlformat_schema_validate(schema, xmlformat);
>>    
>
> We should avoid code which is ignoring the error handling by passing NULL
> instead of a OSyncError reference. (I know there is/was some code doing
> so ... but we should get rid of those old code. It's just bad/wrong/....).
> Feel free to change the paramter list vor the _validate() function to
> (OSyncXMLFormat *xmlformat, OSyncError **error). You might set also an error
> when osync_xmlformat_schema_validate is false:
> osync_error_set(*error, OSYNC_ERROR_GENERIC /*or something different */, "XML
> Format validation failed.");
>  

changed.

--
/Bjoern Ricks

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);
+osync_bool osync_xmlformat_schema_validate(OSyncXMLFormatSchema * schema, OSyncXMLFormat * xmlformat, OSyncError **error);
+
 #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);
+ fail_unless( osync_xmlformat_schema_validate(schema, xmlformat, &error) );
+
+ osync_xmlformat_schema_free(schema);
+
  osync_xmlformat_unref(xmlformat);
 
  destroy_testbed(testbed);

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Daniel Gollub]

On Thursday 26 June 2008 12:58:34 Bjoern Ricks wrote:

> 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.

(Sorry, for the delayed reply. Compeltely forgot to review this ...)

Ok, let's give it a try...
Some comments about coding style and moving the _schema stuff into seperated
files and introducing reference counting.



[...]

> 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;
> +} */

What about moving the entire osync_xmlformat_schema_* "class" into a seperated
file? e.g.: opensync/xmlformart/opensync_xmlformat_schema(_internals.h).(c|
h)?

--------------- 8< -------------------------

> +
> +/**
> + * @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);
> +
> +}

Instead of having simple _free() functions - what about reference counting?
And adding corresbonding _ref() and _unref() function?

> +
> +/**
> + * @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)
> +{

Not quite sure if there is a declartion in the CODING style file or not.
But "Type * variable" looks funny - would you mind to change this to "Type
*variable" ... at least i guess the other code looks like this. I hope you
don't mind... actually i'm not that kind of nitpicker ;)

Oh wait - maybe i'm...

> +       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;
> +}
> +
> +
>  /*@}*/

--------------------- >8 ------------------------

>
>  /**
> @@ -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 @@
>
>  };
>

------------------ 8< ---------------

> +/**
> + * @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;

----------------------- >8 ------------
Move this into opensync/xmlformat/opensync_xmlformat_schema_internals.h?

> +
>  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);

------------- 8< -------
> +osync_bool osync_xmlformat_schema_validate(OSyncXMLFormatSchema * schema,
> OSyncXMLFormat * xmlformat, OSyncError **error); +
--------- >8 ----------
Move this into opensync/xmlfomrat/opensync_xmlformat_schema.h?

>  #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);

Just remove deadcode...

> +       fail_unless( osync_xmlformat_schema_validate(schema,
> xmlformat, &error) ); +
> +       osync_xmlformat_schema_free(schema);
> +
>         osync_xmlformat_unref(xmlformat);
>
>         destroy_testbed(testbed);



-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Bjšoern Ricks]

>
> What about moving the entire osync_xmlformat_schema_* "class" into a seperated
> file? e.g.: opensync/xmlformart/opensync_xmlformat_schema(_internals.h).(c|
> h)?
>
> --------------- 8< -------------------------
>  
Done.
>  
>> [...]
>>    
>
> Instead of having simple _free() functions - what about reference counting?
> And adding corresbonding _ref() and _unref() function?
>  
Done. I have just only to add some osync_xmlformat_schema_get_instance
calls in osync_engine_initialize as you have mentioned in IRC.

>  
>> +
>> +/**
>> + * @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)
>> +{
>>    
>
> Not quite sure if there is a declartion in the CODING style file or not.
> But "Type * variable" looks funny - would you mind to change this to "Type
> *variable" ... at least i guess the other code looks like this. I hope you
> don't mind... actually i'm not that kind of nitpicker ;)
>
> Oh wait - maybe i'm...
>  

:D

I am indifferent between type* variable, type * variable and type
*variable. It is all the same to me ;-)
>
> ------------- 8< -------
>  
>> +osync_bool osync_xmlformat_schema_validate(OSyncXMLFormatSchema * schema,
>> OSyncXMLFormat * xmlformat, OSyncError **error); +
>>    
> --------- >8 ----------
> Move this into opensync/xmlfomrat/opensync_xmlformat_schema.h?
>  
done.

Now there is only the additional test coding left for me to close ticket
#753.
--
/Bjoern Ricks

Index: opensync/opensync-xmlformat.h
===================================================================
--- opensync/opensync-xmlformat.h (Revision 3426)
+++ opensync/opensync-xmlformat.h (Arbeitskopie)
@@ -24,6 +24,7 @@
 OPENSYNC_BEGIN_DECLS
 
 #include "xmlformat/opensync_xmlformat.h"
+#include "xmlformat/opensync_xmlformat_schema.h"
 #include "xmlformat/opensync_xmlfield.h"
 #include "xmlformat/opensync_xmlfieldlist.h"
 
Index: opensync/xmlformat/opensync_xmlformat.c
===================================================================
--- opensync/xmlformat/opensync_xmlformat.c (Revision 3426)
+++ 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,7 +112,7 @@
  g_free(schemafilepath);
 
  return res;
-}
+} */
 
 /*@}*/
 
@@ -394,13 +395,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_schema.c
===================================================================
--- opensync/xmlformat/opensync_xmlformat_schema.c (Revision 0)
+++ opensync/xmlformat/opensync_xmlformat_schema.c (Revision 0)
@@ -0,0 +1,198 @@
+/*
+ * libopensync - A synchronization framework
+ * Copyright (C) 2006  NetNix Finland Ltd <netnix@...>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * Author: Bjoern Ricks <bjoern.ricks@...>
+ *
+ */
+
+#include "opensync.h"
+#include "opensync_internals.h"
+
+#include "opensync-xmlformat.h"
+#include "opensync-xmlformat_internals.h"
+
+/**
+ * @defgroup OSyncXMLFormatPrivateAPI OpenSync XMLFormat Internals
+ * @ingroup OSyncPrivate
+ * @brief The private part of the OSyncXMLFormat
+ *
+ */
+/*@{*/
+
+/**
+ * @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");
+
+ osyncschema->ref_count = 1;
+
+ 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;
+}
+
+/*@}*/
+
+/**
+ * @defgroup OSyncXMLFormatAPI OpenSync XMLFormat
+ * @ingroup OSyncPublic
+ * @brief The public part of the OSyncXMLFormat
+ *
+ */
+/*@{*/
+
+/**
+ * @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) ) {
+ osync_xmlformat_schema_ref(osyncschema);
+ 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;
+}
+
+/**
+ * @brief Decrement the reference counter. The OSyncXMLFormatSchema object will
+ *  be freed if there is no more reference to it.
+ * @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_unref(OSyncXMLFormatSchema *osyncschema) {
+
+ osync_assert(osyncschema);
+
+ if (g_atomic_int_dec_and_test(&(osyncschema->ref_count))) {
+ xmlSchemaFreeValidCtxt(osyncschema->context);
+ xmlSchemaFree(osyncschema->schema);
+ g_free(osyncschema->objtype);
+ g_free(osyncschema);
+ }
+
+}
+
+
+/**
+ * @brief Increments the reference counter
+ * @param osyncschema The pointer to a OSyncXMLFormatSchema object
+ */
+OSyncXMLFormatSchema *osync_xmlformat_schema_ref(OSyncXMLFormatSchema *osyncschema)
+{
+ osync_assert(osyncschema);
+
+ g_atomic_int_inc(&(osyncschema->ref_count));
+
+ return osyncschema;
+}
+/*@}*/
Index: opensync/xmlformat/opensync-xmlformat_internals.h
===================================================================
--- opensync/xmlformat/opensync-xmlformat_internals.h (Revision 3426)
+++ opensync/xmlformat/opensync-xmlformat_internals.h (Arbeitskopie)
@@ -26,6 +26,7 @@
 #include <opensync/opensync_xml.h>
 
 #include "opensync_xmlformat_internals.h"
+#include "opensync_xmlformat_schema_internals.h"
 #include "opensync_xmlfield_internals.h"
 #include "opensync_xmlfieldlist_internals.h"
 
Index: opensync/xmlformat/opensync_xmlformat_internals.h
===================================================================
--- opensync/xmlformat/opensync_xmlformat_internals.h (Revision 3426)
+++ opensync/xmlformat/opensync_xmlformat_internals.h (Arbeitskopie)
@@ -23,7 +23,6 @@
 #ifndef OPENSYNC_XMLFORMAT_INTERNALS_H_
 #define OPENSYNC_XMLFORMAT_INTERNALS_H_
 
-#include <opensync/opensync_xml.h>
 
 /**
  * @brief Represent a XMLFormat object
@@ -46,6 +45,6 @@
 };
 
 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);
 
+
 #endif /*OPENSYNC_XMLFORMAT_INTERNAL_H_*/
Index: opensync/xmlformat/opensync_xmlformat_schema_internals.h
===================================================================
--- opensync/xmlformat/opensync_xmlformat_schema_internals.h (Revision 0)
+++ opensync/xmlformat/opensync_xmlformat_schema_internals.h (Revision 0)
@@ -0,0 +1,47 @@
+/*
+ * libopensync - A synchronization framework
+ * Copyright (C) 2008  NetNix Finland Ltd <netnix@...>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * Author: Bjoern Ricks <bjoern.ricks@...>
+ *
+ */
+
+#ifndef OPENSYNC_XMLFORMAT_SCHEMA_INTERNALS_H_
+#define OPENSYNC_XMLFORMAT_SCHEMA_INTERNALS_H_
+
+#include <libxml/xpath.h>
+#include <libxml/xmlschemas.h>
+#include <libxml/tree.h>
+
+/**
+ * @brief Represents a Schema object
+ * @ingroup OSyncXMLFormatPrivateAPI
+ */
+struct OSyncXMLFormatSchema {
+ /** The schema object */
+ xmlSchemaPtr schema;
+ /** The schema validation context */
+ xmlSchemaValidCtxtPtr context;
+ /** The object type of OSyncXMLFormat */
+ char *objtype;
+ /** The reference counter for this object */
+ int ref_count;
+};
+
+OSyncXMLFormatSchema *osync_xmlformat_schema_new(OSyncXMLFormat *xmlformat, const char *path, OSyncError **error);
+
+#endif /* OPENSYNC_XMLFORMAT_SCHEMA_INTERNALS_H_ */
Index: opensync/xmlformat/opensync_xmlformat.h
===================================================================
--- opensync/xmlformat/opensync_xmlformat.h (Revision 3426)
+++ 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: opensync/xmlformat/opensync_xmlformat_schema.h
===================================================================
--- opensync/xmlformat/opensync_xmlformat_schema.h (Revision 0)
+++ opensync/xmlformat/opensync_xmlformat_schema.h (Revision 0)
@@ -0,0 +1,31 @@
+/*
+ * libopensync - A synchronization framework
+ * Copyright (C) 2008  Bjoern Ricks <bjoern.ricks@...>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * Author: Bjoern Ricks <bjoern.ricks@...>
+ *
+ */
+
+#ifndef OPENSYNC_XMLFORMAT_SCHEMA_H_
+#define OPENSYNC_XMLFORMAT_SCHEMA_H_
+
+OSYNC_EXPORT OSyncXMLFormatSchema *osync_xmlformat_schema_get_instance(OSyncXMLFormat *xmlformat, OSyncError **error);
+OSYNC_EXPORT void osync_xmlformat_schema_unref(OSyncXMLFormatSchema *schema);
+OSYNC_EXPORT OSyncXMLFormatSchema *osync_xmlformat_schema_ref(OSyncXMLFormatSchema *osyncschema);
+OSYNC_EXPORT osync_bool osync_xmlformat_schema_validate(OSyncXMLFormatSchema *schema, OSyncXMLFormat *xmlformat, OSyncError **error);
+
+#endif /* OPENSYNC_XMLFORMAT_SCHEMA_H_ */
Index: opensync/CMakeLists.txt
===================================================================
--- opensync/CMakeLists.txt (Revision 3426)
+++ opensync/CMakeLists.txt (Arbeitskopie)
@@ -57,6 +57,7 @@
    xmlformat/opensync_xmlfield.c
    xmlformat/opensync_xmlfieldlist.c
    xmlformat/opensync_xmlformat.c
+   xmlformat/opensync_xmlformat_schema.c
 )
 
 ADD_LIBRARY( opensync SHARED ${libopensync_LIB_SRCS} )
Index: opensync/opensync.h
===================================================================
--- opensync/opensync.h (Revision 3426)
+++ opensync/opensync.h (Arbeitskopie)
@@ -177,6 +177,7 @@
 typedef struct OSyncCapabilities OSyncCapabilities;
 typedef struct OSyncCapability OSyncCapability;
 typedef struct OSyncXMLFormat OSyncXMLFormat;
+typedef struct OSyncXMLFormatSchema OSyncXMLFormatSchema;
 typedef struct OSyncXMLField OSyncXMLField;
 typedef struct OSyncXMLFieldList OSyncXMLFieldList;
 typedef struct OSyncMerger OSyncMerger;
Index: tests/merger-tests/check_xmlformat.c
===================================================================
--- tests/merger-tests/check_xmlformat.c (Revision 3426)
+++ tests/merger-tests/check_xmlformat.c (Arbeitskopie)
@@ -2,7 +2,7 @@
 
 #include <opensync/opensync-xmlformat.h>
 
-#include "opensync/xmlformat/opensync_xmlformat_internals.h"
+#include "opensync/xmlformat/opensync-xmlformat_internals.h"
 
 START_TEST (xmlformat_new)
 {
@@ -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,12 @@
  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_schema_validate(schema, xmlformat, &error) );
+
+ osync_xmlformat_schema_unref(schema);
+
  osync_xmlformat_unref(xmlformat);
 
  destroy_testbed(testbed);
@@ -364,6 +368,25 @@
 }
 END_TEST
 
+START_TEST (xmlformat_schema_get_instance)
+{
+ OSyncError *error = NULL;
+ OSyncXMLFormat *xmlformat = osync_xmlformat_new("contact", &error);
+ fail_unless(xmlformat != NULL, NULL);
+ fail_unless(error == NULL, NULL);
+
+ OSyncXMLFormatSchema *schema1 = osync_xmlformat_schema_get_instance(xmlformat, &error);
+ OSyncXMLFormatSchema *schema2 = osync_xmlformat_schema_get_instance(xmlformat, &error);
+
+ fail_unless( schema1 == schema2 );
+ fail_unless( schema1->ref_count == 2 );
+
+ osync_xmlformat_schema_unref(schema1);
+ osync_xmlformat_schema_unref(schema2);
+
+}
+END_TEST
+
 Suite *xmlformat_suite(void)
 {
  Suite *s = suite_create("XMLFormat");
@@ -379,6 +402,9 @@
  create_case(s, "xmlformat_compare_ignore_fields", xmlformat_compare_ignore_fields);
  create_case(s, "xmlformat_event_schema", xmlformat_event_schema);
 
+ // xmlformat schema
+ create_case(s, "xmlformat_schema_get_instance", xmlformat_schema_get_instance);
+
  // xmlfield
  create_case(s, "xmlfield_new", xmlfield_new);
  create_case(s, "xmlfield_sort", xmlfield_sort);

-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel

[Daniel Gollub]

On Wednesday 09 July 2008 20:21:55 Bjoern Ricks wrote:
> Now there is only the additional test coding left for me to close ticket
> #753.

Nice!

Tip: if you change the CMAKE_BUILD_TYPE to Profiling then you will get code
coverage results. Just  run the coverage.sh script in the tests directory. Or
run make Experimental and check http://opensync.org/testing for the coverage
results for your tests and file you want to test.

Let me know if you have further question on this ....

See some additional comments on your code....

> --
> /Bjoern Ricks
>
> Index: opensync/opensync-xmlformat.h
> ===================================================================
> --- opensync/opensync-xmlformat.h       (Revision 3426)
> +++ opensync/opensync-xmlformat.h       (Arbeitskopie)
> @@ -24,6 +24,7 @@
>  OPENSYNC_BEGIN_DECLS
>
>  #include "xmlformat/opensync_xmlformat.h"
> +#include "xmlformat/opensync_xmlformat_schema.h"
>  #include "xmlformat/opensync_xmlfield.h"
>  #include "xmlformat/opensync_xmlfieldlist.h"
>
> Index: opensync/xmlformat/opensync_xmlformat.c
> ===================================================================
> --- opensync/xmlformat/opensync_xmlformat.c     (Revision 3426)
> +++ 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,7 +112,7 @@
>         g_free(schemafilepath);
>
>         return res;
> -}
> +} */

Deadcode ... just remove it.

[...]

>
>  /**
> Index: opensync/xmlformat/opensync_xmlformat_schema.c
> ===================================================================
> --- opensync/xmlformat/opensync_xmlformat_schema.c      (Revision 0)
> +++ opensync/xmlformat/opensync_xmlformat_schema.c      (Revision 0)
> @@ -0,0 +1,198 @@
> +/*
> + * libopensync - A synchronization framework
> + * Copyright (C) 2006  NetNix Finland Ltd <netnix@...>

It's your code - isn't it? I guess thats just copy&paste mistake...
(Or did you got already offered contracting work? Hire Bjoern! ;))

> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> USA + *
> + * Author: Bjoern Ricks <bjoern.ricks@...>
> + *
> + */

-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
Opensync-devel mailing list
Opensync-devel@...
https://lists.sourceforge.net/lists/listinfo/opensync-devel