You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Jens Geyer (JIRA)" <ji...@apache.org> on 2014/01/04 01:34:51 UTC

[jira] [Comment Edited] (THRIFT-1964) 'Isset' causes problems with C#/.NET serializers

    [ https://issues.apache.org/jira/browse/THRIFT-1964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13862075#comment-13862075 ] 

Jens Geyer edited comment on THRIFT-1964 at 1/4/14 12:33 AM:
-------------------------------------------------------------

Hi [~xqgzh] and [~carlyeks],

{quote}
change the name of 'Isset' to 'ClassNameIssert', it's just a little change, because there's only one reference to this typename inside the class. please check fix_Isset_xmlserializer.patch
{quote}

A better (compatible) approach could be one of these

{code}
[XmlType(Namespace = "http://tempuri.org/Thrift.Test.A")]  
// or
[XmlType("Thrift_Test_A_Isset")]
{code}

but neither would solve the unset optionals issue. I have tried some things and found, that the various serialization concepts used in C#/.NET are a really PITA to deal with, because they overlap in some details and differ completely in other. 


h3. System.Xml.Serialization.XmlSerializer 

The best way to deal with this flavour is to produce XML like the following (no nullables here):

{code:xml}
<?xml version="1.0"?>
<C xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
   xmlns:xsd="http://www.w3.org/2001/XMLSchema">
  <A>
    <Message>test</Message>
    <Zero>0</Zero>
  </A>
  <B>
    <IsBool>true</IsBool>
    <Message>test2</Message>
    <A>
      <Message>test3</Message>
      <Zero>0</Zero>
    </A>
  </B>
</C>
{code}

The <Value_not_set> tag is omitted, because this value is not set. Thus, the property is not touched during deserialization and so is the {{isset}} flag, which is exactly what we want. To achieve this, we need two things: (1) the {{XmlIgnore}} flag at the {{isset}} member variable, and (2) a bunch of [ShouldSerialize*() methods|http://msdn.microsoft.com/en-us/library/53b8022e(vs.71).aspx] telling the serializer whether or not an optional field is to be serialized:

{code}
public bool ShouldSerializeMessage()
{
  return __isset.message;
}
{code}

h3. DataContractJsonSerializer and related serializer classes

Unfortunately, the {{ShouldSerialize*()}} approach does not work here, so we have to find another way. The single biggest issue with unset optionals is, that even if the {{isset}} flags are serialized, they are read earlier, before all the data members. That problem can be solved by means of the {{DataMember(Order = <number>)}} attribute as [outlined over here|http://msdn.microsoft.com/en-us/library/ms729813(v=vs.110).aspx]. 

After these changes we get some JSON as follows:


{code}
{
   "A":{
      "Message":"test",
      "Value_not_set":0,
      "Zero":0,
      "__isset":{ "message":true, "value_not_set":false, "zero":true }
   },
   "B":{
      "A":{
         "Message":"test3",
         "Value_not_set":0,
         "Zero":0,
         "__isset":{ "message":true, "value_not_set":false, "zero":true }
      },
      "IsBool":true,
      "Message":"test2",
      "__isset":{
         "a":true,
         "isBool":true,
         "message":true
      }
   },
   "__isset":{ "a":true, "b":true }
}
{code}

which does not follow the same structure as the XML above, but does what we want.

h3. Tests 
 
 - sucessfully tested with C# 2010 for Desktop and for Windows Phone 7.x
 - WCF and Mono not yet tested

h3. Files

 - [^modified-testcase.zip]
 - [^THRIFT-1964-serializer-vs-isset.patch]

What do you think?



was (Author: jensg):
Hi [~xqgzh] and [~carlyeks],

{quote}
change the name of 'Isset' to 'ClassNameIssert', it's just a little change, because there's only one reference to this typename inside the class. please check fix_Isset_xmlserializer.patch
{quote}

A better (compatible) approach could be one of these

{code}
[XmlType(Namespace = "http://tempuri.org/Thrift.Test.A")]  
// or
[XmlType("Thrift_Test_A_Isset")]
{code}

but neither would solve the unset optionals issue. I have tried some things and found, that the various serialization concepts used in C#/.NET are a really PITA to deal with, because they overlap in some details and differ completely in other. 


h3. System.Xml.Serialization.XmlSerializer 

The best way to deal with this flavour is to produce XML like the following (no nullables here):

{code:xml}
<?xml version="1.0"?>
<C xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
   xmlns:xsd="http://www.w3.org/2001/XMLSchema">
  <A>
    <Message>test</Message>
    <Zero>0</Zero>
  </A>
  <B>
    <IsBool>true</IsBool>
    <Message>test2</Message>
    <A>
      <Message>test3</Message>
      <Zero>0</Zero>
    </A>
  </B>
</C>
{code}

The <Value_not_set> tag is omitted, because this value is not set. Thus, the property is not touched during deserialization and so is the {{isset}} flag, which is exactly what we want. To achieve this, we need two things: (1) the {{XmlIgnore}} flag at the {{isset}} member variable, and (2) a bunch of [ShouldSerialize*() methods|http://msdn.microsoft.com/en-us/library/53b8022e(vs.71).aspx] telling the serializer whether or not an optional field is to be serialized:

{code}
public bool ShouldSerializeMessage()
{
  return __isset.message;
}
{code}

h3. DataContractJsonSerializer and related serializer classes

Unfortunately, the {{ShouldSerialize*()}} approach does not work here, so we have to find another way. The single biggest issue with unset optionals is, that even if the {{isset}} flags are serialized, they are read earlier, before all the data members. That problem can be solved by means of the {{DataMember(Order = <number>)}} attribute as [outlined over here|http://msdn.microsoft.com/en-us/library/ms729813(v=vs.110).aspx]. 

After these changes we get some JSON as follows:


{code}
{
   "A":{
      "Message":"test",
      "Value_not_set":0,
      "Zero":0,
      "__isset":{ "message":true, "value_not_set":false, "zero":true }
   },
   "B":{
      "A":{
         "Message":"test3",
         "Value_not_set":0,
         "Zero":0,
         "__isset":{ "message":true, "value_not_set":false, "zero":true }
      },
      "IsBool":true,
      "Message":"test2",
      "__isset":{
         "a":true,
         "isBool":true,
         "message":true
      }
   },
   "__isset":{ "a":true, "b":true }
}
{code}

which does not follow the same structure as the XML above, but does what we want.

h3. Tests 
 
 - sucessfully tested with C# 2010 for Desktop and for Windows Phone 7.x
 - WCF and Mono not yet tested

What do you think?


> 'Isset' causes problems with C#/.NET serializers
> ------------------------------------------------
>
>                 Key: THRIFT-1964
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1964
>             Project: Thrift
>          Issue Type: Bug
>          Components: C# - Compiler
>    Affects Versions: 0.9
>         Environment: .net framework 3.5
>            Reporter: xq.gzh
>              Labels: Isset
>         Attachments: 1964-v2.patch, 1964.patch, ReproduceTheIssue.zip, THRIFT-1964-serializer-vs-isset.patch, a.thrift, fix_Isset_xmlserializer.patch, fix_isset_problem_test.zip, modified-testcase.zip
>
>   Original Estimate: 8h
>  Remaining Estimate: 8h
>
> same class name 'Isset' in user defined class will cause xmlserializer crashed. 
> below is the sample thrift:  
> struct A {
>  1: string x;
> }
> struct B {
>  1: string y;
> }
> struct C {
>  1:A a
>  2:B b
> }
> generate code and try xmlserialize instance of class C. it crashed.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)