[ 
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/11/14 7:18 PM:
-------------------------------------------------------------

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-v2.patch ] (was: 
-[^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

h3. Files

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

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, 0.9.1
>            Reporter: xq.gzh
>            Assignee: Jens Geyer
>              Labels: Isset, serializers
>             Fix For: 0.9.2
>
>         Attachments: 1964-v2.patch, 1964.patch, ReproduceTheIssue.zip, 
> THRIFT-1964-serializer-vs-isset-v2.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)

Reply via email to