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.

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

Refactor JSON templates to be generated with ActiveModelSerializers instead of Rabl #4090

Merged
merged 1 commit into from
Jul 7, 2017

Conversation

Gargron
Copy link
Member

@Gargron Gargron commented Jul 6, 2017

Why:

  • Code looks cleaner, feels like it will be cleaner than Rabl when implementing ActivityPub serializers
  • I believe AMS might be more performant than Rabl

I have not yet managed to completely remove Rabl as a dependency. "Initial state" and OEmbed responses, as well as the old ActivityPub stuff that will be replaced still uses Rabl.

@Gargron
Copy link
Member Author

Gargron commented Jul 6, 2017

Benchmark:

s = Status.last
Benchmark.ms { 1000.times { InlineRenderer.render(s, nil, 'api/v1/statuses/show') } }

vs.

s = Status.last
Benchmark.ms { 1000.times { InlineRenderer.render(s, nil, :status) } }

Old (Rabl): 2138.066630999674ms
New (AMS): 1626.861528999143ms

@Gargron Gargron force-pushed the feature-refactor-json-templates branch from f02c3b3 to 6126a5b Compare July 6, 2017 20:15
@Gargron Gargron merged commit 8b2cad5 into master Jul 7, 2017
@Gargron Gargron deleted the feature-refactor-json-templates branch July 7, 2017 02:02
@ykzts
Copy link
Sponsor Member

ykzts commented Jul 7, 2017

This PR may be broken.

$ curl -sS -H"Authorization: Bearer ${MASTODON_ACCESS_TOKEN}" http://localhost:3000/api/v1/statuses/29 | jq '.'
{
  "id": 29,
  "created_at": "2017-07-07T06:24:50.062Z",
  "in_reply_to_id": null,
  "in_reply_to_account_id": null,
  "sensitive": false,
  "spoiler_text": "",
  "visibility": "public",
  "language": null,
  "uri": "tag:localhost:3000,2017-07-07:objectId=29:objectType=Status",
  "content": "<p>RT <span class=\"h-card\"><a href=\"http://localhost:3000/@ykzts\" class=\"u-url mention\">@<span>ykzts</span></a></span> a</p>",
  "url": "http://localhost:3000/@ykzts/29",
  "reblogs_count": 0,
  "favourites_count": 0,
  "favourited": false,
  "reblogged": true,
  "muted": false,
  "reblog": {
    "id": 28,
    "created_at": "2017-07-07T06:24:44.848Z",
    "in_reply_to_id": null,
    "in_reply_to_account_id": null,
    "sensitive": false,
    "spoiler_text": "",
    "visibility": "public",
    "language": "lb",
    "uri": "tag:localhost:3000,2017-07-07:objectId=28:objectType=Status",
    "content": "<p>a</p>",
    "url": "http://localhost:3000/@ykzts/28",
    "reblogs_count": 1,
    "favourites_count": 0,
    "favourited": false,
    "reblogged": true,
    "muted": false
  },
  "application": null,
  "account": {
    "id": 2,
    "username": "ykzts",
    "acct": "ykzts",
    "display_name": "",
    "locked": false,
    "created_at": "2017-07-07T05:17:04.201Z",
    "note": "<p></p>",
    "url": "http://localhost:3000/@ykzts",
    "avatar": "http://localhost:3000/avatars/original/missing.png",
    "avatar_static": "http://localhost:3000/avatars/original/missing.png",
    "header": "http://localhost:3000/headers/original/missing.png",
    "header_static": "http://localhost:3000/headers/original/missing.png",
    "followers_count": 1,
    "following_count": 1,
    "statuses_count": 15
  },
  "media_attachments": [],
  "mentions": [],
  "tags": []
}

missing reblog.account, reblog.application, reblog.media_attachments, reblog.mentions and reblog.tags.

@unarist
Copy link
Contributor

unarist commented Jul 7, 2017

#4095 was merged, but /api/v1/notifications seems still broken. Reblog notification includes reblog itself instead of original status, and notifications are not property ordered.

Gargron added a commit that referenced this pull request Jul 7, 2017
Gargron added a commit that referenced this pull request Jul 7, 2017
* Restore streaming API output format

Regression from #4090

* Remove whitespace
@unarist
Copy link
Contributor

unarist commented Jul 11, 2017

/api/v1/statuses/:id/unfavourite executes unfavouriting asynchronously, so it should return favourited: false regardless of current state, but it doesn't now.

Maybe this PR ignores @favourites_map?

https://github.com/tootsuite/mastodon/blob/8b2cad5/app/controllers/api/v1/statuses/favourites_controller.rb#L18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants