[thrift] branch master updated: THRIFT-4747: The 'omitempty' tag should not be appended to optional fields that have a default value

[jking]

This is an automated email from the ASF dual-hosted git repository.

jking pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git


The following commit(s) were added to refs/heads/master by this push:
     new c28a739  THRIFT-4747: The 'omitempty' tag should not be appended to 
optional fields that have a default value
c28a739 is described below

commit c28a739b8cb62fca9ea869d8d157c0b1d64565b8
Author: Renan Cakirerk <re...@uber.com>
AuthorDate: Mon Jan 21 16:25:10 2019 -0800

    THRIFT-4747: The 'omitempty' tag should not be appended to optional fields 
that have a default value
---
 compiler/cpp/src/thrift/generate/t_go_generator.cc | 8 +++++++-
 lib/go/test/GoTagTest.thrift                       | 1 +
 lib/go/test/tests/gotag_test.go                    | 9 +++++++++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.cc 
b/compiler/cpp/src/thrift/generate/t_go_generator.cc
index ec16b87..11246f3 100644
--- a/compiler/cpp/src/thrift/generate/t_go_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_go_generator.cc
@@ -1338,7 +1338,13 @@ void 
t_go_generator::generate_go_struct_definition(ostream& out,
       t_type* fieldType = (*m_iter)->get_type();
       string goType = type_to_go_type_with_opt(fieldType, 
is_pointer_field(*m_iter));
       string gotag = "db:\"" + escape_string((*m_iter)->get_name())  + "\" ";
-      if ((*m_iter)->get_req() == t_field::T_OPTIONAL) {
+
+      // Only add the `omitempty` tag if this field is optional and has no 
default value.
+      // Otherwise a proper value like `false` for a bool field will be 
ommitted from
+      // the JSON output since Go Marshal won't output `zero values`.
+      bool has_default = (*m_iter)->get_value();
+      bool is_optional = (*m_iter)->get_req() == t_field::T_OPTIONAL;
+      if (is_optional && !has_default) {
         gotag += "json:\"" + escape_string((*m_iter)->get_name()) + 
",omitempty\"";
       } else {
         gotag += "json:\"" + escape_string((*m_iter)->get_name()) + "\"";
diff --git a/lib/go/test/GoTagTest.thrift b/lib/go/test/GoTagTest.thrift
index 508b3b6..5667c6e 100644
--- a/lib/go/test/GoTagTest.thrift
+++ b/lib/go/test/GoTagTest.thrift
@@ -21,4 +21,5 @@ struct tagged {
     1: string string_thing,
     2: i64 int_thing (go.tag = "json:\"int_thing,string\""),
     3: optional i64 optional_int_thing
+    4: optional bool optional_bool_thing = false
 }
diff --git a/lib/go/test/tests/gotag_test.go b/lib/go/test/tests/gotag_test.go
index 32f056f..ff2f14e 100644
--- a/lib/go/test/tests/gotag_test.go
+++ b/lib/go/test/tests/gotag_test.go
@@ -51,3 +51,12 @@ func TestOptionalTag(t *testing.T) {
                t.Error("Unexpected default tag value for optional field")
        }
 }
+
+func TestOptionalTagWithDefaultValue(t *testing.T) {
+       s := gotagtest.Tagged{}
+       st := reflect.TypeOf(s)
+       field, ok := st.FieldByName("OptionalBoolThing")
+       if !ok || field.Tag.Get("json") != "optional_bool_thing" {
+               t.Error("Unexpected default tag value for optional field that 
has a default value")
+       }
+}