Address review feedback

Author: parthea

proto/message.py

@@ -377,12 +377,12 @@ def to_json(
         instance,
         *,
         use_integers_for_enums=True,
-        including_default_value_fields=True,
+        including_default_value_fields=None,
         preserving_proto_field_name=False,
         sort_keys=False,
         indent=2,
         float_precision=None,
-        always_print_fields_with_no_presence=True,
+        always_print_fields_with_no_presence=None,
     ) -> str:
         """Given a message instance, serialize it to json
 
@@ -395,8 +395,8 @@ def to_json(
             including_default_value_fields (Optional(bool)): Deprecated. Use argument
                 `always_print_fields_with_no_presence` instead. An option that
                 determines whether the default field values should be included in the results.
-                Default is True. If `always_print_fields_with_no_presence` is set to False, this
-                option has no effect.
+                This value must match `always_print_fields_with_no_presence`,
+                if both arguments are explictly set.
             preserving_proto_field_name (Optional(bool)): An option that
                 determines whether field name representations preserve
                 proto case (snake_case) or use lowerCamelCase. Default is False.
@@ -410,18 +410,30 @@ def to_json(
             always_print_fields_with_no_presence (Optional(bool)): If True, fields without
                 presence (implicit presence scalars, repeated fields, and map fields) will
                 always be serialized. Any field that supports presence is not affected by
-                this option (including singular message fields and oneof fields). If
-                `including_default_value_fields` is set to False, this option has no effect.
+                this option (including singular message fields and oneof fields).
+                This value must match `including_default_value_fields`,
+                if both arguments are explictly set.
         Returns:
             str: The json string representation of the protocol buffer.
         """
+
         # For backwards compatibility of this breaking change in protobuf 5.x which is specific to proto2
         # https://github.com/protocolbuffers/protobuf/commit/26995798757fbfef5cf6648610848e389db1fecf
+        if always_print_fields_with_no_presence is not None and including_default_value_fields is not None and always_print_fields_with_no_presence != including_default_value_fields:
+            raise ValueError("Arguments `always_print_fields_with_no_presence` and `including_default_value_fields` must match")
+
+        # By default, fields with no presense will be included in the results
+        # when both `always_print_fields_with_no_presence` and `including_default_value_fields` are not set
+        if always_print_fields_with_no_presence is None and including_default_value_fields is None:
+            print_fields = True
+        else:
+            print_fields = always_print_fields_with_no_presence or including_default_value_fields
+
         if PROTOBUF_VERSION[0] in ("3", "4"):
             return MessageToJson(
                 cls.pb(instance),
                 use_integers_for_enums=use_integers_for_enums,
-                including_default_value_fields=including_default_value_fields,
+                including_default_value_fields=print_fields,
                 preserving_proto_field_name=preserving_proto_field_name,
                 sort_keys=sort_keys,
                 indent=indent,
@@ -435,10 +447,7 @@ def to_json(
             return MessageToJson(
                 cls.pb(instance),
                 use_integers_for_enums=use_integers_for_enums,
-                always_print_fields_with_no_presence=(
-                    including_default_value_fields
-                    and always_print_fields_with_no_presence
-                ),
+                always_print_fields_with_no_presence=print_fields,
                 preserving_proto_field_name=preserving_proto_field_name,
                 sort_keys=sort_keys,
                 indent=indent,
@@ -468,15 +477,15 @@ def to_dict(
         *,
         use_integers_for_enums=True,
         preserving_proto_field_name=True,
-        including_default_value_fields=True,
+        including_default_value_fields=None,
         float_precision=None,
-        always_print_fields_with_no_presence=True,
+        always_print_fields_with_no_presence=None,
     ) -> "Message":
         """Given a message instance, return its representation as a python dict.
 
         Args:
             instance: An instance of this message type, or something
-                      compatible (accepted by the type's constructor).
+                compatible (accepted by the type's constructor).
             use_integers_for_enums (Optional(bool)): An option that determines whether enum
                 values should be represented by strings (False) or integers (True).
                 Default is True.
@@ -486,27 +495,38 @@ def to_dict(
             including_default_value_fields (Optional(bool)): Deprecated. Use argument
                 `always_print_fields_with_no_presence` instead. An option that
                 determines whether the default field values should be included in the results.
-                Default is True. If `always_print_fields_with_no_presence` is set to False, this
-                option has no effect.
+                This value must match `always_print_fields_with_no_presence`,
+                if both arguments are explictly set.
             float_precision (Optional(int)): If set, use this to specify float field valid digits.
                 Default is None.
             always_print_fields_with_no_presence (Optional(bool)): If True, fields without
                 presence (implicit presence scalars, repeated fields, and map fields) will
                 always be serialized. Any field that supports presence is not affected by
-                this option (including singular message fields and oneof fields). If
-                `including_default_value_fields` is set to False, this option has no effect.
+                this option (including singular message fields and oneof fields). This value
+                must match `including_default_value_fields`, if both arguments are explictly set.
 
         Returns:
             dict: A representation of the protocol buffer using pythonic data structures.
                   Messages and map fields are represented as dicts,
                   repeated fields are represented as lists.
         """
+
         # For backwards compatibility of this breaking change in protobuf 5.x which is specific to proto2
         # https://github.com/protocolbuffers/protobuf/commit/26995798757fbfef5cf6648610848e389db1fecf
+        if always_print_fields_with_no_presence is not None and including_default_value_fields is not None and always_print_fields_with_no_presence != including_default_value_fields:
+            raise ValueError("Arguments `always_print_fields_with_no_presence` and `including_default_value_fields` must match")
+
+        # By default, fields with no presense will be included in the results
+        # when both `always_print_fields_with_no_presence` and `including_default_value_fields` are not set
+        if always_print_fields_with_no_presence is None and including_default_value_fields is None:
+            print_fields = True
+        else:
+            print_fields = always_print_fields_with_no_presence or including_default_value_fields
+
         if PROTOBUF_VERSION[0] in ("3", "4"):
             return MessageToDict(
                 cls.pb(instance),
-                including_default_value_fields=including_default_value_fields,
+                including_default_value_fields=print_fields,
                 preserving_proto_field_name=preserving_proto_field_name,
                 use_integers_for_enums=use_integers_for_enums,
                 float_precision=float_precision,
@@ -518,10 +538,7 @@ def to_dict(
             # The old flag accidentally had inconsistent behavior between proto2 optional and proto3 optional fields
             return MessageToDict(
                 cls.pb(instance),
-                always_print_fields_with_no_presence=(
-                    including_default_value_fields
-                    and always_print_fields_with_no_presence
-                ),
+                always_print_fields_with_no_presence=print_fields,
                 preserving_proto_field_name=preserving_proto_field_name,
                 use_integers_for_enums=use_integers_for_enums,
                 float_precision=float_precision,