Skip to content

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

Merged
Gargron merged 1 commit intomasterfrom
feature-refactor-json-templates
Jul 7, 2017
Merged

Refactor JSON templates to be generated with ActiveModelSerializers instead of Rabl#4090
Gargron merged 1 commit intomasterfrom
feature-refactor-json-templates

Conversation

@Gargron
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

3 participants