myyang / django-pb-model

Protobuf mixin for django model
Other
112 stars 22 forks source link

Error on handling reverse relationship #23

Closed cecilulysess closed 4 years ago

cecilulysess commented 4 years ago

It's common that model add related_name at a FK field and the related model can use reverse relationship to access the model.

For example

class Author:
  pass

class Book:
  author = models.ForeignKey(Author, related_name='books')

And you have the corresponding message:

message Author {
  int32 id = 1;
  repeated Books books = 2;
}

message Book {
  int32 id = 1;
}

While pb-model does not necessarily want to expand this reverse relationship when Author's to_pb is called, however, it should not throw an error since it's common case that the proto has has the books field.

I created test case for this error: https://github.com/cecilulysess/django-pb-model/commit/f2966952bc2939260c9fb8a60a7ffb18323171f4

Exception thrown as:

Traceback (most recent call last):
  File "/home/jojo/workspace/django-pb-model/pb_model/tests/tests.py", line 297, in test_reverse_relation
    test_proto = deeper_relation_item.to_pb()
  File "/home/jojo/workspace/django-pb-model/pb_model/models.py", line 276, in to_pb
    self._to_proto_recursively(_pb_obj, _pb_to_dj_mapping, _dj_fields, depth)
  File "/home/jojo/workspace/django-pb-model/pb_model/models.py", line 258, in _to_proto_recursively
    self._to_pb(_dj_field_name, _field, _pb_obj, _dj_fields, _pb_dj_field_map, depth=depth)
  File "/home/jojo/workspace/django-pb-model/pb_model/models.py", line 245, in _to_pb
    raise DjangoPBModelError(
pb_model.models.DjangoPBModelError: Can't serialize Model(relations)'s field: <class 'pb_model.tests.models.DeeperRelation'>. Err: 'google.protobuf.pyext._message.RepeatedCompositeCo' object has no attribute 'CopyFrom'

I think the quick fix is simply add this at begining of _relation_to_protobuf method:


    if isinstance(dj_field_type, ManyToOneRel):
            print('Found original django field {} is reverse related, skip.'.
                  format(dj_field_type))
            return

WDYT?

myyang commented 4 years ago

I think it's ok to skip since m2m mapping from pb to django already does this from very beginning of this lib. 😂 Leave an hook function for customization and flexibility would be enough.

@cecilulysess Do you mind to submit a PR?

cecilulysess commented 4 years ago

DONE https://github.com/myyang/django-pb-model/pull/24