Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generation proto/field not nullable in response are not optional #284

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Generation proto/field not nullable in response are not optional #284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
06b3cfc
in response field optional are only the one nullable
AMontagu Apr 8, 2024
9b84d94
improve condition
AMontagu Apr 8, 2024
30b59e3
fix tests
AMontagu Apr 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions django_socio_grpc/protobuf/generation_plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ def transform_request_message(
"name": self.field_name,
"type": self.field_type,
"cardinality": self.field_cardinality,
}
},
in_request=True,
)
)
return proto_message
Expand Down Expand Up @@ -208,7 +209,11 @@ class AsListGenerationPlugin(BaseGenerationPlugin):
list_field_name: str = "results"

def transform_message_to_list(
self, service: Type["Service"], proto_message: Union[ProtoMessage, str], list_name: str
self,
service: Type["Service"],
proto_message: Union[ProtoMessage, str],
list_name: str,
in_request: bool = False,
) -> ProtoMessage:
try:
list_field_name = proto_message.serializer.Meta.message_list_attr
Expand Down Expand Up @@ -256,7 +261,9 @@ def transform_request_message(
message_name_constructor: MessageNameConstructor,
) -> ProtoMessage:
list_name = message_name_constructor.construct_request_list_name()
return self.transform_message_to_list(service, proto_message, list_name)
return self.transform_message_to_list(
service, proto_message, list_name, in_request=True
)


class ResponseAsListGenerationPlugin(AsListGenerationPlugin):
Expand All @@ -271,7 +278,9 @@ def transform_response_message(
message_name_constructor: MessageNameConstructor,
) -> ProtoMessage:
list_name = message_name_constructor.construct_response_list_name()
return self.transform_message_to_list(service, proto_message, list_name)
return self.transform_message_to_list(
service, proto_message, list_name, in_request=False
)


class RequestAndResponseAsListGenerationPlugin(
Expand Down
58 changes: 40 additions & 18 deletions django_socio_grpc/protobuf/proto_classes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,22 +88,25 @@ def field_line(self) -> str:
return " ".join(values)

@classmethod
def _get_cardinality(cls, field: serializers.Field):
ProtoGeneratorPrintHelper.print("field.default: ", field.default)
def _get_cardinality(
cls, field: serializers.Field, in_request: bool = False
) -> FieldCardinality:
"""
INFO - AM - 04/01/2023
If field can be null -> optional
if field is not required -> optional. Since DRF 3.0 When using model default, only required is set to False. The model default is not set into the field as just passing None will result in model default. https://github.com/encode/django-rest-framework/issues/2683
if field.default is set (meaning not None or empty) -> optional
if we are in a request message and field is not required -> optional. Since DRF 3.0 When using model default, only required is set to False. The model default is not set into the field as just passing None will result in model default. https://github.com/encode/django-rest-framework/issues/2683
if we are in a request message and field.default is set (meaning not None or empty) -> optional

Not dealing with field.allow_blank now as it doesn't seem to be related to OPTIONAl and more about validation and only exist for charfield
"""
if field.allow_null or not field.required or field.default not in [None, empty]:
if field.allow_null:
return FieldCardinality.OPTIONAL
if in_request and (not field.required or field.default not in [None, empty]):
return FieldCardinality.OPTIONAL
return FieldCardinality.NONE

@classmethod
def from_field_dict(cls, field_dict: FieldDict) -> "ProtoField":
def from_field_dict(cls, field_dict: FieldDict, in_request: bool = False) -> "ProtoField":
cardinality = field_dict.get("cardinality", FieldCardinality.NONE)
name = field_dict["name"]
field_type = field_dict["type"]
Expand Down Expand Up @@ -156,28 +159,29 @@ def from_field(
to_message: Callable = None,
parent_serializer: serializers.Serializer = None,
name_if_recursive: str = None,
in_request: bool = False,
) -> "ProtoField":
"""
to_message, parent_serializer, name_if_recursive only used if field is ListSerializer with a child being a Serializer
"""
cardinality = cls._get_cardinality(field)
cardinality = cls._get_cardinality(field, in_request=in_request)

if isinstance(field, serializers.SerializerMethodField):
ProtoGeneratorPrintHelper.print(f"{field.field_name} is SerializerMethodField")
return cls._from_serializer_method_field(field)
return cls._from_serializer_method_field(field, in_request=in_request)

elif isinstance(field, serializers.ManyRelatedField):
ProtoGeneratorPrintHelper.print(f"{field.field_name} is ManyRelatedField")
proto_field = cls._from_related_field(
field.child_relation, source_attrs=field.source_attrs
field.child_relation, source_attrs=field.source_attrs, in_request=in_request
)
proto_field.name = field.field_name
proto_field.cardinality = FieldCardinality.REPEATED
return proto_field

elif isinstance(field, serializers.RelatedField):
ProtoGeneratorPrintHelper.print(f"{field.field_name} is RelatedField")
return cls._from_related_field(field)
return cls._from_related_field(field, in_request=in_request)

if isinstance(field, serializers.ListField):
ProtoGeneratorPrintHelper.print(f"{field.field_name} is ListField")
Expand All @@ -187,7 +191,11 @@ def from_field(
f"{field.field_name} ListField child is a serializer"
)
proto_field = cls.from_serializer(
field.child, to_message, parent_serializer, name_if_recursive
field.child,
to_message,
parent_serializer,
name_if_recursive,
in_request=in_request,
)
field_type = proto_field.field_type
else:
Expand Down Expand Up @@ -217,11 +225,12 @@ def from_serializer(
to_message: Callable,
parent_serializer: serializers.Serializer = None,
name_if_recursive: str = None,
in_request: bool = False,
) -> "ProtoField":
"""
Create a ProtoField from a Serializer, which will be converted to a ProtoMessage with `to_message`
"""
cardinality = cls._get_cardinality(field)
cardinality = cls._get_cardinality(field, in_request=in_request)
serializer_class = field.__class__
if getattr(field, "many", False):
cardinality = FieldCardinality.REPEATED
Expand All @@ -246,11 +255,14 @@ def from_serializer(

@classmethod
def _from_related_field(
cls, field: serializers.RelatedField, source_attrs: Optional[List[str]] = None
cls,
field: serializers.RelatedField,
source_attrs: Optional[List[str]] = None,
in_request: bool = False,
) -> "ProtoField":
serializer: serializers.Serializer = field.root

cardinality = cls._get_cardinality(field)
cardinality = cls._get_cardinality(field, in_request=in_request)

if not source_attrs:
source_attrs = field.source_attrs
Expand Down Expand Up @@ -298,9 +310,9 @@ def _from_related_field(

@classmethod
def _from_serializer_method_field(
cls, field: serializers.SerializerMethodField
cls, field: serializers.SerializerMethodField, in_request: bool = False
) -> "ProtoField":
cardinality = cls._get_cardinality(field)
cardinality = cls._get_cardinality(field, in_request=in_request)
method_name = field.method_name

try:
Expand Down Expand Up @@ -378,6 +390,11 @@ class ProtoMessage:
comments: Optional[List[str]] = None
serializer: Optional[serializers.BaseSerializer] = None
imported_from: Optional[str] = None
suffix: ClassVar = ""

@classmethod
def in_request(cls):
return cls.suffix == REQUEST_SUFFIX

def get_all_messages(self) -> Dict[str, "ProtoMessage"]:
messages = {self.name: self}
Expand Down Expand Up @@ -440,7 +457,10 @@ def from_field_dicts(
fields = []
return cls(
name=name,
fields=[ProtoField.from_field_dict(field) for field in fields],
fields=[
ProtoField.from_field_dict(field, in_request=cls.in_request())
for field in fields
],
)

@classmethod
Expand Down Expand Up @@ -484,6 +504,7 @@ def from_serializer(
or MessageNameConstructor.get_base_name_from_serializer_with_suffix(
serializer, getattr(cls, "suffix", "")
),
in_request=cls.in_request(),
)
)
else:
Expand All @@ -499,12 +520,13 @@ def from_serializer(
or MessageNameConstructor.get_base_name_from_serializer_with_suffix(
serializer, getattr(cls, "suffix", "")
),
in_request=cls.in_request(),
)
)
# INFO - AM - 07/01/2022 - else the serializer needs to implement to_proto_message
elif hasattr(serializer, "to_proto_message"):
fields = [
ProtoField.from_field_dict(dict_field)
ProtoField.from_field_dict(dict_field, in_request=cls.in_request())
for dict_field in serializer().to_proto_message()
]
else:
Expand Down
60 changes: 30 additions & 30 deletions django_socio_grpc/tests/fakeapp/grpc/fakeapp.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ message BasicProtoListChildRequest {
}

message BasicProtoListChildResponse {
optional int32 id = 1;
int32 id = 1;
string title = 2;
optional string text = 3;
}
Expand Down Expand Up @@ -250,8 +250,8 @@ message CustomNameForResponse {

// Test comment for whole message
message CustomRetrieveResponseSpecialFieldsModelResponse {
optional string uuid = 1;
optional int32 default_method_field = 2;
string uuid = 1;
int32 default_method_field = 2;
repeated google.protobuf.Struct custom_method_field = 3;
}

Expand Down Expand Up @@ -313,25 +313,25 @@ message DefaultValueRequest {
}

message DefaultValueResponse {
optional int32 id = 1;
optional string string_required_but_serializer_default = 2;
optional int32 int_required_but_serializer_default = 3;
optional bool boolean_required_but_serializer_default = 4;
optional string string_default_but_serializer_default = 5;
optional string string_nullable_default_but_serializer_default = 6;
int32 id = 1;
string string_required_but_serializer_default = 2;
int32 int_required_but_serializer_default = 3;
bool boolean_required_but_serializer_default = 4;
string string_default_but_serializer_default = 5;
string string_nullable_default_but_serializer_default = 6;
string string_required = 7;
optional string string_blank = 8;
string string_blank = 8;
optional string string_nullable = 9;
optional string string_default = 10;
optional string string_default_and_blank = 11;
string string_default = 10;
string string_default_and_blank = 11;
optional string string_null_default_and_blank = 12;
int32 int_required = 13;
optional int32 int_nullable = 14;
optional int32 int_default = 15;
int32 int_default = 15;
bool boolean_required = 16;
optional bool boolean_nullable = 17;
optional bool boolean_default_false = 18;
optional bool boolean_default_true = 19;
bool boolean_default_false = 18;
bool boolean_default_true = 19;
}

message DefaultValueRetrieveRequest {
Expand All @@ -351,13 +351,13 @@ message ForeignModelListResponse {
}

message ForeignModelResponse {
optional string uuid = 1;
string uuid = 1;
string name = 2;
}

message ForeignModelRetrieveCustomResponse {
string name = 1;
optional string custom = 2;
string custom = 2;
}

message ForeignModelRetrieveCustomRetrieveRequest {
Expand All @@ -370,7 +370,7 @@ message ImportStructEvenInArrayModelRequest {
}

message ImportStructEvenInArrayModelResponse {
optional string uuid = 1;
string uuid = 1;
repeated google.protobuf.Struct this_is_crazy = 2;
}

Expand All @@ -381,7 +381,7 @@ message ManyManyModelRequest {
}

message ManyManyModelResponse {
optional string uuid = 1;
string uuid = 1;
string name = 2;
}

Expand Down Expand Up @@ -411,8 +411,8 @@ message RecursiveTestModelRequest {
}

message RecursiveTestModelResponse {
optional string uuid = 1;
optional RecursiveTestModelResponse parent = 2;
string uuid = 1;
RecursiveTestModelResponse parent = 2;
repeated RecursiveTestModelResponse children = 3;
}

Expand Down Expand Up @@ -448,13 +448,13 @@ message RelatedFieldModelRequest {
}

message RelatedFieldModelResponse {
optional string uuid = 1;
optional ForeignModelResponse foreign = 2;
string uuid = 1;
ForeignModelResponse foreign = 2;
repeated ManyManyModelResponse many_many = 3;
optional int32 slug_test_model = 4;
repeated bool slug_reverse_test_model = 5;
repeated string slug_many_many = 6;
optional string proto_slug_related_field = 7;
string proto_slug_related_field = 7;
string custom_field_name = 8;
repeated string many_many_foreigns = 9;
}
Expand Down Expand Up @@ -495,7 +495,7 @@ message SimpleRelatedFieldModelRequest {
}

message SimpleRelatedFieldModelResponse {
optional string uuid = 1;
string uuid = 1;
optional string foreign = 2;
optional string slug_test_model = 3;
repeated string many_many = 4;
Expand Down Expand Up @@ -539,10 +539,10 @@ message SpecialFieldsModelRequest {
// Special Fields Model
// with two lines comment
message SpecialFieldsModelResponse {
optional string uuid = 1;
optional google.protobuf.Struct meta_datas = 2;
string uuid = 1;
google.protobuf.Struct meta_datas = 2;
repeated int32 list_datas = 3;
optional bytes binary = 4;
bytes binary = 4;
}

message SpecialFieldsModelRetrieveRequest {
Expand Down Expand Up @@ -610,7 +610,7 @@ message UnitTestModelRequest {
}

message UnitTestModelResponse {
optional int32 id = 1;
int32 id = 1;
string title = 2;
optional string text = 3;
}
Expand Down Expand Up @@ -655,7 +655,7 @@ message UnitTestModelWithStructFilterRequest {
}

message UnitTestModelWithStructFilterResponse {
optional int32 id = 1;
int32 id = 1;
string title = 2;
optional string text = 3;
}
Expand Down
370 changes: 185 additions & 185 deletions django_socio_grpc/tests/fakeapp/grpc/fakeapp_pb2.py
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion django_socio_grpc/tests/fakeapp/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ class RelatedFieldModelSerializer(proto_serializers.ModelProtoSerializer):
foreign = ForeignModelSerializer(read_only=True)
many_many = ManyManyModelSerializer(many=True)

slug_test_model = serializers.SlugRelatedField(slug_field="special_number", read_only=True)
slug_test_model = serializers.SlugRelatedField(
slug_field="special_number", read_only=True, allow_null=True
)
slug_reverse_test_model = serializers.SlugRelatedField(
slug_field="is_active", read_only=True, many=True
)
Expand Down
Loading
Loading