Summary

Centralize serialization/deserialization of resource models.

Review Request #6587 — Created Nov. 14, 2014 and submitted Nov. 14, 2014, 6:07 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-2.0.x

Bugs

Depends On

Commit

7096395...

Reviewers

Groups

reviewboard

People

Description

Previously, all JavaScript resource models were responsible for
implementing toJSON() and parseResourceData() functions. In many cases,
there were inconsistencies in how they handled certain
serialization/deserialization operations.

Along with this, we had no way of mapping attribute names to JSON
fields. This meant that, in some cases, calling code had to know about
the JSON fields in order to perform certain operations (like
fine-grained saving of fields).

Now, resources list the attributes to serialize and deserialize, and can
provide a map between attribute names and JSON field names. They can
also list custom serializers/deserializers for different fields. All
common serializers are provided in a new RB.JSONSerializers object.

This doesn't fully remove the need for toJSON or parseResourceData
functions, but most are gone. Most others can be removed down the road
if we centralize text type processing into a mixin.

An upcoming change will allow fine-grained saving of fields based on
attribute names, allowing us to make faster API calls with less impact
on the server.

Testing Done

All unit tests pass. We had some extensive testing already for
serialization/deserialization, but some more tests were added to
cover all cases.

I played around with reviews, replies, comments, review requests, and
file attachments, without any problems.

That said, we may hit some regression somewhere, but we'll need to do
some real-world testing.

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/staticbundles.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/reviewGroupModel.js
    reviewboard/static/rb/js/resources/models/baseCommentModel.js
    reviewboard/static/rb/js/resources/models/tests/reviewRequestModelTests.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.js
    reviewboard/static/rb/js/resources/models/defaultReviewerModel.js
    reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js
    reviewboard/static/rb/js/resources/utils/serializers.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/models/tests/reviewModelTests.js
    reviewboard/static/rb/js/resources/models/tests/validateDiffModelTests.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.js
    reviewboard/static/rb/js/resources/models/diffCommentModel.js
    reviewboard/static/rb/js/resources/models/tests/fileAttachmentCommentModelTests.js
    reviewboard/static/rb/js/resources/models/validateDiffModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
    reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
    reviewboard/static/rb/js/resources/models/tests/baseCommentModelTests.js
    reviewboard/static/rb/js/resources/models/baseCommentReplyModel.js
    reviewboard/static/rb/js/resources/models/reviewModel.js
    reviewboard/static/rb/js/resources/models/tests/screenshotCommentModelTests.js
    reviewboard/static/rb/js/resources/models/reviewReplyModel.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/resources/models/screenshotModel.js
    reviewboard/static/rb/js/resources/models/tests/baseCommentReplyModelTests.js
    reviewboard/static/rb/js/resources/models/tests/diffCommentModelTests.js



Tool: Pyflakes
Processed Files:
    reviewboard/staticbundles.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/reviewGroupModel.js
    reviewboard/static/rb/js/resources/models/baseCommentModel.js
    reviewboard/static/rb/js/resources/models/tests/reviewRequestModelTests.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.js
    reviewboard/static/rb/js/resources/models/defaultReviewerModel.js
    reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js
    reviewboard/static/rb/js/resources/utils/serializers.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/models/tests/reviewModelTests.js
    reviewboard/static/rb/js/resources/models/tests/validateDiffModelTests.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.js
    reviewboard/static/rb/js/resources/models/diffCommentModel.js
    reviewboard/static/rb/js/resources/models/tests/fileAttachmentCommentModelTests.js
    reviewboard/static/rb/js/resources/models/validateDiffModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
    reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
    reviewboard/static/rb/js/resources/models/tests/baseCommentModelTests.js
    reviewboard/static/rb/js/resources/models/baseCommentReplyModel.js
    reviewboard/static/rb/js/resources/models/reviewModel.js
    reviewboard/static/rb/js/resources/models/tests/screenshotCommentModelTests.js
    reviewboard/static/rb/js/resources/models/reviewReplyModel.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/resources/models/screenshotModel.js
    reviewboard/static/rb/js/resources/models/tests/baseCommentReplyModelTests.js
    reviewboard/static/rb/js/resources/models/tests/diffCommentModelTests.js

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-2.0.x (d6066db)