Skip to content

Change IDs to strings rather than numbers in API JSON output#5019

Merged
Gargron merged 4 commits intomastodon:masterfrom
aschmitz:feat/ids-as-strings
Sep 20, 2017
Merged

Change IDs to strings rather than numbers in API JSON output#5019
Gargron merged 4 commits intomastodon:masterfrom
aschmitz:feat/ids-as-strings

Conversation

@aschmitz
Copy link
Copy Markdown
Contributor

As discussed in #4801, this is being broken out into a separate PR to break up separate changes into separate PRs, and also to make it easier to identify immediate causes for changes post-squashing when merged.

From that PR:

Probably-Breaking Change

The API now returns strings for IDs rather than IDs. There is a good chance that this will break some API clients, although more testing is probably required. (To my surprise, Tusky handled this change with no trouble.) The rationale for this is that JavaScript cannot parse JSON with numbers with more than 53 bits precisely, and we intend to use 64 bits (or at the very least, currently 57 bits). While we could go the Twitter route and provide both a semantically-correct integer and a JavaScript-usable string, it appears as though serving only the string is likely to lead to fewer issues in the long term. I'm open to exposing IDs as both strings and integers if that decision appears more prudent, however. (Ruby can output this with no problem.) More rationale for this decision is in the commit message for 86bf2c3.

Somewhat predictably, the JS interface handled IDs as numbers, which in
JS are IEEE double-precision floats. This loses some precision when
working with numbers as large as those generated by the new ID scheme,
so we instead handle them here as strings. This is relatively simple,
and doesn't appear to have caused any problems, but should definitely
be tested more thoroughly than the built-in tests. Several days of use
appear to support this working properly.

BREAKING CHANGE:

The major(!) change here is that IDs are now returned as strings by the
REST endpoints, rather than as integers. In practice, relatively few
changes were required to make the existing JS UI work with this change,
but it will likely hit API clients pretty hard: it's an entirely
different type to consume. (The one API client I tested, Tusky, handles
this with no problems, however.)

Twitter ran into this issue when introducing Snowflake IDs, and decided
to instead introduce an `id_str` field in JSON responses. I have opted
to *not* do that, and instead force all IDs to 64-bit integers
represented by strings in one go. (I believe Twitter exacerbated their
problem by rolling out the changes three times: once for statuses, once
for DMs, and once for user IDs, as well as by leaving an integer ID
value in JSON. As they said, "If you’re using the `id` field with JSON
in a Javascript-related language, there is a very high likelihood that
the integers will be silently munged by Javascript interpreters. In most
cases, this will result in behavior such as being unable to load or
delete a specific direct message, because the ID you're sending to the
API is different than the actual identifier associated with the
message." [1]) However, given that this is a significant change for API
users, alternatives or a transition time may be appropriate.

1: https://blog.twitter.com/developer/en_us/a/2011/direct-messages-going-snowflake-on-sep-30-2011.html
These should be the last two. These were identified using eslint to try
to identify any plain casts to JavaScript numbers. (Some such casts are
legitimate, but these were not.)

Adding the following to .eslintrc.yml will identify casts to numbers:

~~~
  no-restricted-syntax:
  - warn
  - selector: UnaryExpression[operator='+'] > :not(Literal)
    message: Avoid the use of unary +
  - selector: CallExpression[callee.name='Number']
    message: Casting with Number() may coerce string IDs to numbers
~~~

The remaining three casts appear legitimate: two casts to array indices,
one in a server to turn an environment variable into a number.
This was made to make a test a bit less flakey, but has nothing to
do with this branch.
@nightpool
Copy link
Copy Markdown
Member

Do we have to worry about JSON serialized to Redis or similar? what about the streaming API?

@nightpool
Copy link
Copy Markdown
Member

nightpool commented Sep 20, 2017

Redis says this about scores:

Range of integer scores that can be expressed precisely

Redis sorted sets use a double 64-bit floating point number to represent the score. In all the architectures we support, this is represented as an IEEE 754 floating point number, that is able to represent precisely integer numbers between -(2^53) and +(2^53) included. In more practical terms, all the integers between -9007199254740992 and 9007199254740992 are perfectly representable. Larger integers, or fractions, are internally represented in exponential form, so it is possible that you get only an approximation of the decimal number, or of the very big integer, that you set as score.

We use status ids as scores in feed_manager so that's probably an issue

@aschmitz
Copy link
Copy Markdown
Contributor Author

I'm not aware of JSON that gets stored in Redis: some JSON gets sent to Redis pubsub, but that doesn't get stored (and any messages would switch when the application server code gets updated). A quick case-insensitive grep through for "redis" appears to confirm this, but if you know of somewhere where JSON is stored in Redis, that may need to be addressed.

The streaming API uses that Redis pubsub to get objects serialized by InlineRenderer, which itself uses REST::StatusSerializer or REST::NotificationSerializer, which are both updated here.

@Gargron
Copy link
Copy Markdown
Member

Gargron commented Sep 20, 2017

I think there are some places where raw JSON objects are sent into pubsub, without serializers, such as for status deletes, notification deletes.. Where it just sticks the ID of the object into an event. Those might need a to_s as well.

@nightpool
Copy link
Copy Markdown
Member

nightpool commented Sep 20, 2017

@aschmitz
Copy link
Copy Markdown
Contributor Author

The double-precision floats as Redis scores are probably best addressed in #4801, but the short version is that that PR does use IDs as scores, but also moves to storing the IDs as values.

Redis can support multiple entries with the same score, and sorts them lexicographically, which in almost all cases means in ID order (with the exception of cases where we are just rolling over to an extra digit in the decimal representation and still coming out to the same value as a double-precision float, which will happen once relatively soon, and then practically never again).

Previously, the Redis sorted sets used the potential for different scores and IDs to track whether an entry was a reblog or not, and to deduplicate reblogs. #4801 introduces a separate sorted set to track reblogs, so we can get away with having the same score and value for the feed sorted set. (This also makes it somewhat easier to track all recent reblogs of a status that a user might see, potentially allowing the UI to list all of them, and avoiding the issue of an un-reblog resulting in a non-followed user's status showing up in the feed, but those are potential future improvements and not super relevant here.)

@nightpool
Copy link
Copy Markdown
Member

Ah, I missed that there were redis changes that didn't get split out. Sounds good. Aside from the streaming API code I linked, the other place we use ids heavily that I could think of being a problem is sidekiq job arguments.

@nightpool
Copy link
Copy Markdown
Member

(which are stored in redis as JSON values afaik)

@aschmitz
Copy link
Copy Markdown
Contributor Author

Yeah, that streaming inadvertent cast is likely to be an issue in a few cases. I think we can get away with changing line 267 to:

const encodedPayload = typeof payload === 'number' ? JSON.stringify(message) : JSON.stringify(payload);

(Or '"' + message + '"' if you prefer.) Does that seem like a viable strategy, or should I go through and change the values that are sent to Redis via publishing?

@nightpool
Copy link
Copy Markdown
Member

I'm not sure that helps. message is something like {event: "delete", payload: 231268371283, queued_at: 199292301}. The payload is supposed to be just an ID, and we'd be passing an object with a nested payload. We'd need to change anywhere we consumed the streaming API to accommodate that and it seems worse to do it there then to just fix the publish calls, right?

@aschmitz
Copy link
Copy Markdown
Contributor Author

Oops, neither of those are going to be practical, I'll have to go through and change any publishes of IDs to strings.

Per
mastodon#5019 (comment)
we need these changes to send deleted status IDs as strings, not
integers.
@nightpool
Copy link
Copy Markdown
Member

Okay i think LGTM if we fix the redis publishes. We should get this out in the wild soon enough to find any bugs and make sure the API developers have enough time to upgrade.

@aschmitz
Copy link
Copy Markdown
Contributor Author

591a9af should fix the Redis issues in this branch, although I believe I'll need to make a different set of changes in #4801, because I moved some of the status deletion code around.

I believe deletes are the only cases where IDs are the only data passed, and I double-checked with egrep -Ri "redis.*publish", but if you're aware of other cases or want to double-check, you probably know that code better than I do.

@nightpool
Copy link
Copy Markdown
Member

Yeah, I double checked all the Redis calls and these seem to be the only ones (excepting the use of scores) that could be a problem.

aschmitz added a commit to aschmitz/mastodon that referenced this pull request Sep 20, 2017
This is equivalent to 591a9af from
mastodon#5019, with an extra change for the addition to FeedManager#unpush.
@Gargron Gargron merged commit 669fe9e into mastodon:master Sep 20, 2017
@kvvzr
Copy link
Copy Markdown

kvvzr commented Sep 20, 2017

This PR affects native client apps.
Almost native apps are using strict languages, they can not convert from Long to String type and will crash. (or error is displayed

@Gargron
Copy link
Copy Markdown
Member

Gargron commented Sep 20, 2017

Correct, this is a breaking change and will be released with Mastodon 2.0

@kvvzr
Copy link
Copy Markdown

kvvzr commented Sep 20, 2017

I'm sorry. I was misunderstanding.
When parsing old API response, it just manually converts IDs to String type, so it is not difficult... 🙇

@clworld
Copy link
Copy Markdown
Contributor

clworld commented Sep 20, 2017

Some thought:
I feel 'breaking change released with Mastodon 2.0' is not a far future thing.
Pawoo already 1.6 and other huge instance may get 1.6 soon.
So, almost nothing is preventing from releasing 2.0. I think.
Time remains for 3rd party developer to deal with new api response structure is not huge, isn't it?

and FYI: PR note says "Tusky handled this change with no trouble.".
but author of SubwayTooter says
"Android API JSON parser parses Number String as Double and convert into Long when calling optLong(),
So, It has prescision problem like Javascript (works only on < 63bits)."
Thus, also Tusky may caught trouble with > 53bit IDs without changes.

heraldofsolace added a commit to heraldofsolace/mastodon that referenced this pull request Sep 20, 2017
* Fetch reblogs as Announce activity instead of Note object (mastodon#4672)

* Process Create / Announce activity in FetchRemoteStatusService

* Use activity URL in ActivityPub for reblogs

* Redirect to the original status on StatusesController#show

* Add configuration to disable private status federation over PuSH (mastodon#4582)

* Disable babel-loader cache when development environment (mastodon#4684)

* Don't load Roboto webfont when system font is used in the app (mastodon#4591)

* Don't load Roboto webfont when system font is used in the app

* remove trailing whitespace

* Update Russian translation (mastodon#4685)

* Add Russian translation (ru)

* Fix a missing comma

* Fix the wording for better consistency

* Update Russian translation

* Arrange Russian setting alphabetically

* Fix syntax error

* Update Russian translation

* Fix formatting error

* Update Russian translation

* Update Russian translation

* Update ru.jsx

* Fix syntax error

* Remove two_factor_auth.warning (appears obsolete)

* Add missing strings in ru.yml

A lot of new strings translated, especially for the newly added admin section

* Fix translation consistency

* Update Russian translation

* Update Russian translation (pluralizations)

* Update Russian translation

* Update Russian translation

* Update Russian translation (pin)

* Update Russian translation (account deletion)

* Fix extra line

* Update Russian translation (sessions)

* Update Russian translation

* Update Russian translation

* Fix merge conflicts (revert)

* Pinned statuses (mastodon#4675)

* Pinned statuses

* yarn manage:translations

* i18n: Update Polish translation mastodon#4675 (mastodon#4692)

Signed-off-by: Marcin Mikołajczak <[email protected]>

* Add label for application scopes (mastodon#4691)

* Add label for application scopes

* hint

* Update addressable to version 2.5.2 (mastodon#4686)

* i18n Updated strings (mastodon#4675 - pinned toot) (mastodon#4695)

* Added string for pinned toots

* Pinned toot mastodon#4675 + missing string

Somehow I deleted it "enabled_success"

* update after advice

* Adjust styles of landing pages. (mastodon#4682)

* Adjust about.scss

* Delete trailing whitespace.

* Change timezone of the datetime to what browser specifies (mastodon#4688)

* Apply user timezone for the title attribute of .time-ago (mastodon#4693)

* Allow multiple pinned statuses to be shown and make them be ordered b… (mastodon#4690)

* Allow multiple pinned statuses to be shown and make them be ordered by pinned date

* Set timestamps NOT NULL

* Make single-line pinned_statuses

* Spec for pinned_statuses

* Remove redundant empty line

* Fix ar.json (mastodon#4699)

Remove ! from compose_form.publish

* Fix missing at-sign (regression from mastodon#4688) (mastodon#4705)

* authorize-follow-requests-after-unlocking (mastodon#4658)

* Added new translations of error messages, block and mute domains and users, privacy disclaimers, etc (mastodon#4700)

* Added new translations of error messages, block and mute domains and users

* Added new translations of error messages, block and mute domains and users

* Add handling of Linked Data Signatures in payloads (mastodon#4687)

* Add handling of Linked Data Signatures in payloads

* Add a way to sign JSON, fix canonicalization of signature options

* Fix signatureValue encoding, send out signed JSON when distributing

* Add missing security context

* Set margin between character-counter and compose-form__buttons (mastodon#4698)

For some languages publish translation is long.

* Add ActivityPub serializer for Undo of Announce (mastodon#4703)

* Use Tombstone and _:atomUri in Delete activities as fallback (mastodon#4704)

* Forward ActivityPub deletes to followers of rebloggers (mastodon#4706)

* Add _:inReplyToAtomUri to ActivityPub (mastodon#4702)

* Allow Symbol keyed Hash in LinkedDataSignature (mastodon#4715)

SerializarbleResource#as_json serializes to Symbol keyed Hash, but current
implementation of LinkedDataSignature expects String keyed Hash.

So it generates broken payload.

* Adjust RTL styles (mastodon#4712)

* Add japanese translations for Pinned statuses based on pawoo. (mastodon#4717)

Add japanese translations for pin_errors.

* Shorten display of large numbers on public profiles (mastodon#4711)

* Adjust public profile pages (mastodon#4713)

* Adjust account-grid in public profiles

Full-width card on mobile UI. Set break-word for long name and ID. Fix margin.

* Reduce padding-bottom of public profiles

* Revive next prev buttons in mobile public profiles

In followers followees pages.

* Revert break-word for username

* Fix overflow of display_name

Need re-setting text-overflow and overflow in display: block;

* Adjust "signed in as" pages (mastodon#4720)

* Adjust "signed in as" pages


Fix min-width


Set width of .account-header .name

To apply text-overflow and overflow settings
Set overflow for detailed-status__display-name

* Remove trailing whitespace

* Add japanese translations for shorten display of large numbers (mastodon#4722)

* rescue HTTP::ConnectionError in RemoteFollowController#create (mastodon#4726)

* Fix deletion of status which has been reblogged (mastodon#4728)

* Fix Delete activity handling when the status has been reblogged (mastodon#4729)

* Generalized the infinite scrollable list (mastodon#4697)

* Do not scroll the columns area due to redirection (mastodon#4541)

Commit 9d1f8b9 scrolls the columns area
when the route changes since the user is likely to want to see the
rightmost column in such cases.

However, redirection is automatic and does not indicate users' intension.
Do not scroll the columns area due to one.

* Serialize ActivityPub alternate link into OStatus deletes, handle it (mastodon#4730)

Requires moving Atom rendering from DistributionWorker (where
`stream_entry.status` is already nil) to inline (where
`stream_entry.status.destroyed?` is true) and distributing that.

Unfortunately, such XML renderings can no longer be easily chained
together into one payload of n items.

* fix error when single columns mode. (mastodon#4734)

* Scroll smoothly to the right (mastodon#4735)

* Update bundler-audit and brakeman (mastodon#4740)

* Remove unneccesary indices (mastodon#4738)

We only look up status_pins by account_id, or account_id and status_id,
never by status_id

* Update status embeds (mastodon#4742)

- Use statuses controller for embeds instead of stream entries controller
- Prefer /@:username/:id/embed URL for embeds
- Use /@:username as author_url in OEmbed
- Add follow link to embeds which opens web intent in new window
- Use redis cache in development
- Cache entire embed

* Use request.remote_ip instead of request.ip (mastodon#4744)

* Add close tag of iframe for oEmbed response (mastodon#4745)

* Add close tag of iframe for oEmbed response

* add comma

* Forward ActivityPub creates that reply to local statuses (mastodon#4709)

* Forward ActivityPub creates that reply to local statuses

* Fix test

* Fix wrong signers

* error fixed (when loading pages in single column mode.) (mastodon#4746)

* Update to Alpine 3.6 (mastodon#4747)

* Add sharedInbox to actors (mastodon#4737)

* Embed modal (mastodon#4748)

* Embed modal

* Proxy OEmbed requests from web UI

* Fix the usages of Detect Passive Events (mastodon#4749)

* Guarantee Subscription service first account has proper URL details (mastodon#4732)

* Guarantee Subscription service first account has proper URL details

Subscription Service potentially could break if the first user suspended
themselves, creating a situation where the urls that populate throughout
subscription service's PuSH request would cause the remote API to throw 503 errors.

Guaranteeing that the first account picked is not suspended prevents this problem.

* Fix style issue

* Don't process ActivityPub payload if signature is invalid (mastodon#4752)

* Don't process ActivityPub payload if signature is invalid

* Fix style issue

* Remove identity context from output of LinkedDataSignature (mastodon#4753)

* Fallback from perform_via_activitypub on private posts (mastodon#4758)

Currently, private / direct posts via OStatus from AP compatible instance will be dropped due to failing to fetch AP version.

So this fallbacks to OStatus handling:

* when failed to fetch ActivityPub version
* when status is neither :public nor :unlisted

* Convert OStatus tag to ActivityPub id on in_reply_to resolution (mastodon#4756)

* Refactor Web::PushSubscription, remove welcome message (mastodon#4524)

* Refactor Web::PushSubscription, remove welcome message

* Add missing helper

* Use locale of the receiver on push notifications (mastodon#4519)

* Remove unused translations

* Fix dir on notifications

* Update FR locales (mastodon#4714)

* Make the fr locales up-to-date with the last changes (new profile view, applications)

* Use the same wording for toots in fr.yml and fr.json

* Translate the pin related strings

* Translate pin-related locales on the front-end

* Add missing locales in doorkeeper.fr.yml and remove un-used ones

* Change "posts" back to "status" in the /about/more page in fr.yml

* Fix typos for "status" in fr.yml

* fix typo for "status" in fr.json

* Remove duplicate string

* Non-breaking space before punctuation

* 'Better' translation for "unpin"

* Put back 'pouet' where it was already

* Fix

* Fix

* Make first use less overwhelming with browser permissions (mastodon#4760)

- Ask for desktop notifications after 1 minute of use instead of
  instantly
- Ask for protocol handler permission after 5 minutes of use
  instead of instantly

* Use system's default font on non web UI pages (mastodon#4553)

* Use system's default font on non web UI pages

* Remove import for Redirect

* Make PreviewCard records reuseable between statuses (mastodon#4642)

* Make PreviewCard records reuseable between statuses

**Warning!** Migration truncates preview_cards tablec

* Allow a wider thumbnail for link preview, display it in horizontal layout (mastodon#4648)

* Delete preview cards files before truncating

* Rename old table instead of truncating it

* Add mastodon:maintenance:remove_deprecated_preview_cards

* Ignore deprecated_preview_cards in schema definition

* Fix null behaviour

* Fix NoMethodError (mastodon#4762)

* Deduplicate with local status on Create activity (mastodon#4763)

*  Adjust padding on the public profile page (mastodon#4757)

* Fix a style issue on the public profile page for some mobile browsers

Signed-off-by: Cygnan <[email protected]>

* Set padding-bottom to 20px

Signed-off-by: Cygnan <[email protected]>

* Do not rely on activity arriving exactly once after delete arrived (mastodon#4754)

* Avoid sending some ActivityPub payloads if the receiver will get them through distribution (mastodon#4739)

* Finish up embed modal feature (mastodon#4759)

* Add embed button to dropdowns of in-timeline statuses

* yarn run manage:translations

* Add ActivityPub handler for Delete->Actor activities (mastodon#4761)

* Add link to 'noscript' message (mastodon#4561)

* Add link to 'noscript' message

Signed-off-by: Marcin Mikołajczak <[email protected]>

* remove indent

* Add RoutingHelper (mastodon#4769)

* Add japanese translations for embed modal feature. (mastodon#4770)

* Use updated ActivityStreams context (added: sharedInbox) (mastodon#4764)

* Define missing JSON-LD properties (mastodon#4767)

Using _: property names is discouraged, as in the future,
canonicalization may throw an error when encountering that instead
of discarding it silently like it does now.

We are defining some ActivityStreams properties which we expect
to land in ActivityStreams eventually, to ensure that future versions
of Mastodon will remain compatible with this even once that happens.
Those would be `locked`, `sensitive` and `Hashtag`

We are defining a custom context inline for some properties which we
do not expect to land in any other context. `atomUri`, `inReplyToAtomUri`
and `conversation` are part of the custom defined OStatus context.

* Disable embed modal when private status (mastodon#4773)

* Disable embed modal when private status

* Remove `reblogDisabled`

* Fix profile page when use system's font (mastodon#4774)

* Add text color style for noscript link (mastodon#4772)

* Add Japanese translate for mastodon#4561 (mastodon#4771)

* Adjust settings pages (mastodon#4765)

* views: Adjust heading positions

* Adjust settings pages

Adjust label. Adjust tables. Adjust admin/reports/* pages. Fix 2FA QR code style for narrow devices. Widen several pages. Increase contrast.

* Remove trailing whitespace

* i18n: Update Polish translation (mastodon#4775)

Signed-off-by: Marcin Mikołajczak <[email protected]>

* Don't unconditionally call `preventDefault` and `stopPropagation` on all keyup events (mastodon#4777)

* UploadArea should only preventDefault for Escape

This will make accessibility for some things less effortful, since we won't have to define a prior event handler to do whatever should be happening by default.

* Remove workaround for fixed bug in SettingToggle

SettingToggle was toggling itself in response to keydown of space, and then the keyup was doing it again

* Fix WebPush (regression from mastodon#4524) (mastodon#4778)

* Fix NoMethodError in Web::PushSubscription

```
undefined method `site_contact_email' for #<Class:0x00005976d13c40>

/usr/local/bundle/gems/activerecord-5.1.3/lib/active_record/dynamic_matchers.rb:22:in `method_missing'
/usr/local/bundle/gems/attr_encrypted-3.0.3/lib/attr_encrypted.rb:295:in `method_missing'
/usr/local/bundle/gems/attr_encrypted-3.0.3/lib/attr_encrypted/adapters/active_record.rb:129:in `method_missing_with_attr_encrypted'
/mastodon/app/models/web/push_subscription.rb:53:in `push_payload'
```

* Specify serializer in Web::NotificationSerializer

* Raise an error for remote url in StatusFinder (mastodon#4776)

* Raise an error for remote url in StatusFinder

Previous implementation had allowed remote url with status id which also exists on local.

Then that bug leads /api/web/embed to return wrong embed url.

* Fix oembed_controller_spec

* fix text position of NSFW in Safari (Mac/iPhone) (mastodon#4570)

* Make "unfollow" undo pending outgoing follow request too (mastodon#4781)

* Make "unfollow" undo pending outgoing follow request too

* Add cancel button to web UI when awaiting follow request approval

* Make the hourglass button do the cancelling

* i18n: Improve Polish translation (mastodon#4783)

Signed-off-by: Marcin Mikołajczak <[email protected]>

* Validate data of Imports (mastodon#4782)

* Rename "locked" to "manuallyApprovesFollowers" in ActivityPub (mastodon#4779)

See: <https://www.w3.org/wiki/Activity_Streams_extensions#as:manuallyApprovesFollowers>

* Improve client-side German i18n (mastodon#4785)

* Instantly upgrade account to ActivityPub if we receive ActivityPub payload (mastodon#4766)

* Bump version to 1.6.0rc1 (mastodon#4768)

* Explicitly define attached file of DeprecatedPreviewCard (mastodon#4786)

The path template of the attached files must explicitly be defined because
it is contradicting to the name of the class.

* l10n Occitan update for Embed, cancel follow request, ... (mastodon#4788)

* Update: some missing strings

* Updates missing strings

* New string

* Update oc.json

* Update oc.yml

* Update oc.json

* Croatian translation - updated  (mastodon#4183)

* Update hr.json

* Update hr.json

* Use next instead of return in task (mastodon#4787)

* Make german translation more gender neutral mastodon#4755 (mastodon#4789)

* Fix short number locales (mastodon#4790)

Overwrite values from rails-i18n by manually overwriting in every locale.
We want numbers like 1.5K in every language

* Fix a problem that notification column goes to top (mastodon#4792)

* Fix mastodon#4551 - Use correct syntax for content preloading (mastodon#4798)

* Fix streaming url to lowercase (mastodon#4804)

* Show pinned statuses only in the top of the profile page (mastodon#4803)

* Show pinned statuses only in the top of the profile page

* Refactor AccountsController#show_pinned_statuses?

* Fix some ActivityPub JSON bugs (mastodon#4796)

- Fix assumption that `url` is always a string. Handle it if it's an
  array of strings, array of objects, object, or string, both for
  accounts and for objects
- `sharedInbox` is actually supposed to be under `endpoints`, handle
  both cases and adjust the serializer

* Update ar.yml (mastodon#4810)

Some little changes to "ar" locale

* Translation korean added (mastodon#4802)

* comment correction (mastodon#4812)

* add index_notifications_on_id_and_account_id_and_activity_type on notifications table (mastodon#4750)

* Adjust status embeds (mastodon#4808)

* Adjust status embeds

Adjust styles of embed code. Adjust styles of embed pages. Fix overflow of embed-modal.

* Remove trailing whitespace

* Using width from the variable

* Fix mastodon#4794 - Fake instant follow in API response when account is believed unlocked (mastodon#4799)

* Fix mentions in direct statuses not being delivered via AP (mastodon#4806)

* Do not execute the job with the same arguments as the retry job (mastodon#4814)

* swift-enable the paperclip! 📎 (mastodon#2322)

* Add environment sample for OpenStack Swift (mastodon#4816)

* fix text position of NSFW for video file (mastodon#4819)

* Update react-intl to version 2.4.0 (mastodon#4820)

* fix scroll position (mastodon#4821)

* i18n: update Persian translation (mastodon#4822)

* Add Smartphone screen favourite back button and adjust styles (mastodon#4813)

* Feat add get-back button on favourite columnHeader

* Style adjust nice looking get-back button

* Fix delete media query and add padding right

* fix: restore padding and add lastchild style for back-button

* Switch to static URIs, new URI format in both protocols for new statuses (mastodon#4815)

* Decouple Status#local? from uri being nil

* Replace on-the-fly URI generation with stored URIs

- Generate URI in after_save hook for local statuses
- Use static value in TagManager when available, fallback to tag format
- Make TagManager use ActivityPub::TagManager to understand new format
- Adjust tests

* Use other heuristic for locality of old statuses, do not perform long query

* Exclude tombstone stream entries from Atom feed

* Prevent nil statuses from landing in Pubsubhubbub::DistributionWorker

* Fix URI not being saved (mastodon#4818)

* Add more specs for Status

* Save generated uri immediately

and also fix method order to minimize diff.

* Fix alternate HTML URL in Atom

* Fix tests

* Remove not-null constraint from statuses migration to speed it up

* Bump version to 1.6.0rc2

* Fix locking migration on statuses table. Nullable column and NO default value (mastodon#4825)

* Update fr.json (mastodon#4830)

typo

* i10n update OC and FR files (mastodon#4824)

* Onboarding: corrections

Some missing letters and spaces or better wording

* Embed

Translated as Intégrer in FR / Embarcar in OC

* Use casecmp() instead of casecmp?() for now (mastodon#4832)

* Use casecmp() instead of casecmp?() for now

casecmp?() is only available in ruby 2.4.0.  Users running earlier ruby versions
would see errors, e.g., running
RAILS_ENV=production rails mastodon:maintenance:remove_deprecated_preview_cards.

* Correctly check whether casecmp() returns 0

* Feat add validation for report comment: characters under 1000 valid (mastodon#4833)

* Add Pinned toot column (mastodon#4817)

* Add Pinned_toot_section

* Fix add frozen_string_literal

* Fix delete no need controller and tests

* Fix replace query strings to axios params

* Fix change value to accountId and disabling more button

* Use <button> instead of <div role="button"> (mastodon#4835)

* Enable UniqueRetryJobMiddleware even when called from sidekiq worker (mastodon#4836)

* Fix mastodon#4834 - Adjust Status#local and Status#remote scopes (mastodon#4839)

* i10n OC / FR update Pinned toots (mastodon#4842)

* Added column.pins

New strings

* Added column.pins

* Update confirmation_instructions.oc.html.erb

* Update confirmation_instructions.oc.text.erb

* Update password_change.oc.html.erb

* Update password_change.oc.text.erb

* Update reset_password_instructions.oc.html.erb

* Update reset_password_instructions.oc.text.erb

* Update confirmation_instructions.oc.html.erb

* Update confirmation_instructions.oc.text.erb

* i18n: Update Polish translation (mastodon#4845)

Signed-off-by: Marcin Mikołajczak <[email protected]>

* Fetch statuses/following/followers numbers from ActivityPub collections (mastodon#4840)

* "Mute conversation" option on all own toots, not just in notifications (mastodon#4844)

That way you can mute notifications for a toot before you get replies
to it or boosts or favourites

* Fix language filter codes (mastodon#4841)

* Fix language filter codes

CLD3 returns BCP-47 language identifier, filter settings expect
identifiers in the ISO 639-1 format. Convert between formats,
and exclude duplicate languages from filter choices (zh-CN->zh)

* Fix zh name

* i10n update OC and FR (mastodon#4849)

* Missing "navigation_bar.pins"

* Missing "navigation_bar.pins"

* Handle stream_entry URL correctly in ActivityPub (mastodon#4854)

In before, the method uses stream_entry id as status id, so replied status was wrongly selected.

This PR uses StatusFinder which was introduced with `Api::Web::EmbedsController`.

* Refresh timeline after toot while the timeline is disconnected (mastodon#4858)

To reflect status posting immediately, we've inserted the status into timelines directly. However, status insertion changes "latest status", and it means next timeline refresh only fetches statuses since the inserted status. This behavior is very bad for disconnected timeline and mobile views.

After this patch, it refreshes timeline for disconnected timelines, instead of direct insertion.

* Fix mastodon#4850 - When visibility missing from API call to toot, fallback to user preference (mastodon#4861)

* Fix mastodon#4852 - Check if already requested from FollowService (mastodon#4855)

* Fix mastodon#1004 - Temporarily pause timeline if there's been recent mouse movement (mastodon#4859)

* Scrollable tables in settings pages (mastodon#4857)

* Scrollable tables in settings pages

* Add space before curly brace

* Add missing reject_media check before avatar download via ActivityPub (mastodon#4862)

* Fix second report (regression from 3b81baa) (mastodon#4863)

* Fix scroll behavior and others on paused timeline (mastodon#4864)

Resolved:

* Lot of redundant renders while mouse moving
* Scroll jumping when timeline loaded
* Scroll position isn't kept when statuses below the scrollTop was deleted then new status arrived

Unresolved:

* Scroll position isn't kept when statuses over the scrollTop was deleted then new status arrived
-> It needs to know which statuses are over the scrollTop
* New status indicator should be active when new statuses arrived while mouse moved recently
-> It needs a) update indicator in ScrollableList, or b) set scrollTop status while mouse moving

* Bump version to 1.6.0rc4

* Disable mouse-based pause from mastodon#4859 (mastodon#4865)

It wasn't working ideally and introduced some annoying false positivies

* Add script to make embedded iframes autosize (mastodon#4853)

* Fix errors preventing UnsubscribeService from working (mastodon#4866)

* i10n minors changes for 1.6 (mastodon#4867)

* wrong preposition + typo

* wrong preposition + typo

* Typo

* Typo

* minor changes

* minor changes

* Set fallback address when empty notification address (mastodon#4868)

* Fix dimensions of loading component for compose drawer (mastodon#4872)

* Default follows for new users (mastodon#4871)

When a new user confirms their e-mail, bootstrap their home timeline
by automatically following a set of accounts. By default, all local
admin accounts (that are unlocked). Can be customized by new admin
setting (comma-separated usernames, local and unlocked only)

* Hide modal loading screen for media/video/boost/confirm/actions modals (mastodon#4873)

* Bump version to 1.6.0rc5

* Fix Japanese translation (mastodon#4876)

I translated the additional text ( added by mastodon#4871)

* Fix POST /api/v1/follows error when already following (mastodon#4878)

* Bump to 1.6.0

* i18n: Update Polish translation (mastodon#4881)

* l10n: update Persian translation (mastodon#4880)

* l10n: update Persian translation

* l10n: fix missing Persian translation

* l10n: Full PT-BR translation (mastodon#4882)

* devise.pt-BR.yml now fully translated

* pt-BR.json now fully translated

* pt-BR.yml partially translated; 46 lines left

* pt-BR.yml now fully translated

* simple_form.pt-BR.yml fully translated

* doorkeeper.pt-BR.yml now fully translated

* E-mail instructions on app/views/user_mailer added and fully translated

* PT-BR translation for mastodon#4871

* Deleted an unwanted caracter on pt-BR.yml

* Fixing typos on pt-BR.yml

* Added translation for Pinned toots tab on pt-BR.json

* Added missing translation for navigation_bar.pins

* Add OpenStack Keystone V3 support (mastodon#4889)

Keystone V2 is deprecated in favour of V3. This adds the necessary
connection parameters for establishing a V3 connection. Connections
to V2 endpoints are still possible and the configuration should
remain compatible.

This also introduces a SWIFT_REGION variable for multi-region
OpenStack environments and a SWIFT_CACHE_TTL that controls how long
tokens and other meta-data is cached for. Caching tokens avoids
rate-limiting errors that would result in media uploads becoming
unavailable during high load or when using tasks like
media:remove_remote. fog-openstack only supports token caching for
V3 endpoints, so a recommendation for using V3 was added.

* Reset preview image if avatar/header image selection was cancelled (mastodon#4893)

* Bump rails from 5.1.3 to 5.1.4 (mastodon#4875)

Bumps [rails](https://github.com/rails/rails) from 5.1.3 to 5.1.4.
- [Commits](rails/rails@v5.1.3...v5.1.4)

* Bump puma from 3.9.1 to 3.10.0 (mastodon#4879)

Bumps [puma](https://github.com/puma/puma) from 3.9.1 to 3.10.0.
- [Release notes](https://github.com/puma/puma/releases/tag/v3.10.0)
- [Changelog](https://github.com/puma/puma/blob/master/History.md)
- [Commits](puma/puma@v3.9.1...v3.10.0)

* Fix error when following locked accounts (mastodon#4896)

* Fix count numbers from ActivityPub not being saved (mastodon#4899)

They are marked as read-only by Rails, but we know what we are doing,
so we are un-marking them as such.

The mastodon:maintenance:update_counter_caches task is not really
supposed to be run anymore (it was a one-time thing during an upgrade)
however, just in case, I have modified it to not touch ActivityPub
accounts.

Also, no point writing to logger from these rake tasks, since they
are not to be run from cron. Better to give stdout feedback.

* Fix mastodon#4894 - Merge context hash into final JSON hash after key transform (mastodon#4898)

* Fix nil error for old toots that don't have a conversation (mastodon#4900)

* Clean up and improve generated OpenGraph tags (mastodon#4901)

- Return all images as og:image
- Return videos as og:image (preview) and og:video
- Return profile:username on profiles

* Add section for protocol specific information on the admin page (mastodon#4910)

This PR adds section for protocol specific information, then always show
both of OStatus and ActivityPub. Specifically, this will help admins to
check PuSH subscription status and unsubscribe manually, even `protocol`
has been changed.

This also includes below changes:

* Add `overflow: hidden` to prevent float leaking
* Add missing fields for ActivityPub

* [WiP] Whenever a remote keypair changes, unfollow them and re-subscribe to … (mastodon#4907)

* Whenever a remote keypair changes, unfollow them and re-subscribe to them

In Mastodon (it could be different for other OStatus or AP-enabled software),
a keypair change is indicative of whole user (or instance) data loss. In this
situation, the “new” user might be different, and almost certainly has an empty
followers list. In this case, Mastodon instances will disagree on follower
lists, leading to unreliable delivery and “shadow followers”, that is users
believed by a remote instance to be followers, without the affected user
knowing.

Drawbacks of this change are:
1. If an user legitimately changes public key for some reason without losing
   data (not possible in Mastodon at the moment), they will have their remote
   followers unsubscribed/re-subscribed needlessly.
2. Depending of the number of remote followers, this may generate quite some
   traffic.
3. If the user change is an attempt at usurpation, the remote followers will
   unknowingly follow the usurper. Note that this is *not* a change of
   behavior, Mastodon already behaves like that, although delivery might be
   unreliable, and the usurper would not have known the former user's
   followers.

* Rename ResubscribeWorker to RefollowWorker

* Process followers in batches

* Specify libicu explicitly in Aptfile (mastodon#4920)

It seems libicu-dev no longer installs libicu55 needed by charlock_holmes.

* Fix height cache (mastodon#4909)

* Fix mastodon#4918 - Limit pinned toots to 5 (mastodon#4923)

* Fix mastodon#4917 - Add missing suspend checks (mastodon#4921)

* Make instance names in  into links to user list in the instance (mastodon#4924)

* Add instance search feature (mastodon#4925)

* Fix ActivityPub handling of replies with WEB_DOMAIN (mastodon#4895) (mastodon#4904)

* Fix ActivityPub handling of replies when LOCAL_DOMAIN ≠ WEB_DOMAIN (mastodon#4895)

For all intents and purposes, `local_url?` is used to check if an URL refers
to the Web UI or the various API endpoints of the local instances. Those things
reside on `WEB_DOMAIN` and not `LOCAL_DOMAIN`.

* Change local_url? spec, as all URLs handled by Mastodon are based on WEB_DOMAIN

* fix share intent. (mastodon#4926)

* Updating Dutch translation (mastodon#4927)

* Update doorkeeper.nl.yml

* Update nl.yml

* Update simple_form.nl.yml

* Update nl.json

* Update en.json

* Update en.json

* Update nl.json

* i18n: Update Polish translation (mastodon#4929)

Signed-off-by: Marcin Mikołajczak <[email protected]>

* Uploads for admin site settings (mastodon#4913)

* Improve OpenGraph tags for about pages

* Add thumbnail admin setting

* Fix error

* Fix up

* Fix refollowing (mastodon#4931)

* Make RefollowWorker ActivityPub-only to avoid potential identifier mismatches

* Don't call RefollowWorker on new accounts

* Redesign video player (mastodon#4911)

* Redesign video player

* Use new video player on static public pages too

* Use media gallery component on static public pages too

* Pause video when hiding it

* Full-screen sizing on WebKit

* Add aria labels to video player buttons

* Display link card on public status page

* Fix fullscreen from modal sizing issue

* Remove contain: strict property to fix fullscreen from columns

* Give video player fluid max-width (mastodon#4935)

* Support OpenGraph video embeds (mastodon#4897)

* Support OpenGraph video embeds

It's not really OpenGraph, it's twitter:player property, but it's
not OEmbed so that fits. For example, this allows Twitch clips to
be displayed as embeds.

Also, fixes glitch-soc#135

* Fix invalid OpenGraph cards being saved through attaching and
revisit URLs after 14 days

* Add Japanese translate for mastodon#4913 (mastodon#4936)

* l10n update OC/FR video redesign (mastodon#4938)

* l10n update for Redesign video player (mastodon#4911)

* Update videp

* Update

I hope this time format works well.

* One missing string

* Update time format

I'd like the complete name of the month in the Long format and the short one in the short format.
I hope it works now

* Add Japanese translate for mastodon#4911 (mastodon#4943)

* Another Dutch language update (mastodon#4944)

* Update nl.json

* Update nl.yml

* Update nl.json

* i18n: Update Polish translation (mastodon#4942)

* i18n: Update Polish translation

Signed-off-by: Marcin Mikołajczak <[email protected]>

* i18n: Update Polish translation

Signed-off-by: Marcin Mikołajczak <[email protected]>

* Update pl.yml

* Revert unique retry job (mastodon#4937)

* Revert "Enable UniqueRetryJobMiddleware even when called from sidekiq worker (mastodon#4836)"

This reverts commit 6859d4c.

* Revert "Do not execute the job with the same arguments as the retry job (mastodon#4814)"

This reverts commit be7ffa2.

* Include requested URL into the message on network errors (mastodon#4945)

* Fix mastodon#4908 - Do not keep remote file names, generate random (mastodon#4934)

* l10n: update Persian translation (mastodon#4946)

* Enable to recognize most kinds of characters as URL paths (mastodon#4941)

* Add missing Japanese translations (mastodon#4947)

* Fix race condition when receiving an ActivityPub Create multiple times (mastodon#4930)

* Fix race condition when receiving an ActivityPub Create multiple times

* Use a RedisLock to avoid concurrent processing of a same Create activity

* Add scheduled worker to purge old user IPs (mastodon#4951)

* Add scheduled worker to purge old user IPs

* Use ruby 1.9 hash syntax

* l10n: PT-BR translation updated (mastodon#4953)

* devise.pt-BR.yml now fully translated

* pt-BR.json now fully translated

* pt-BR.yml partially translated; 46 lines left

* pt-BR.yml now fully translated

* simple_form.pt-BR.yml fully translated

* doorkeeper.pt-BR.yml now fully translated

* E-mail instructions on app/views/user_mailer added and fully translated

* PT-BR translation for mastodon#4871

* Deleted an unwanted caracter on pt-BR.yml

* Fixing typos on pt-BR.yml

* Added translation for Pinned toots tab on pt-BR.json

* Added missing translation for navigation_bar.pins

* Fixed spelling on pt-BR.yml

* Update pt-BR.json

* Remove redundant width/height values from SVGs to fix Safari bug (mastodon#4956)

* When web UI URL used while logged out, redirect to static page (mastodon#4954)

* Fix invisible load more button (mastodon#4962)

* Fix behavior while the button is invisible
e.g. pointer cursor, couldn't open contextmenu
* Avoid rendering the button to remove blank space if no more items are available or no items are rendered

* When accessing uncached media attachment, redownload it (mastodon#4955)

* When accessing uncached media attachment, redownload it

* Prevent re-download of rejected media

* Bump to 1.6.1

* Fix filterable_languages method of SettingsHelper (mastodon#4966)

* Fix hasSize condition in secSet and sizes. (mastodon#4969)

* Fix AP serialization error when thread is missing (mastodon#4970)

`Status#reply?` may returns true even if the thread is missing.
e.g. the replied status was deleted or couldn't be fetched.

Then it raises NoMethodError on various AP json serialization.

This issue won't happen on Atom serialization because it checks thread
existence using `StreamEntry#threaded?` instead.

* correct URL pattern used in text length counter in WebUI (mastodon#4968)

* Fix an error in ReplyDistributionWorker when replied status was deleted (mastodon#4974)

Reply distribution is proceed by Sidekiq, so replied status may be deleted before this.

* Adjust landing pages 2 (mastodon#4967)

* Adjust landing pages 2

Fix styles of terms page
Remove action buttons from timeline in about page
Adjust styles of short description
Adjust form inputs
Set autocomplete off for username and email box in registration form. Remove line breakings.

* Revert removing action buttons

* Minor Chinese check & jsx addition (mastodon#4973)

* zh-*: transition from "like" back to "fav"

This commit reverts the translation for the yellow-star "fav" button
back to "fav" in Chinese. Some ambuiguity between "like" and "fav" is
deliberately used in zh-TW/HK by using the existing phrase "最爱"
(favorite (adj.), lit. love-most) instead of "收藏" (favourite (v.),
"collect") in some instances.

Fixes mastodon#3511.

* zh-*: apply suggestions for PR mastodon#4557

* zh-cn: de-monetize ya account

In Chinese two separate characters, 账 and 帐, can be used to spell the
word for account (账/帐户). However, the one with a 贝 on the left is
evolved from the latter specifically for monetary purposes. Since
people usually can't figure out which one to use, it might be a good
idea to use the original not-so-money one.

* zh-*: complete jsx translation

* Escape URL parts on formatting local status (mastodon#4975)

* Remove local_only scope in Status (mastodon#4977)

* So Spanish. Much changes. Wow. (mastodon#4976)

* Fix an error when actor json couldn't be fetched in ResolveRemoteAccountService (mastodon#4979)

* Fix an error when actor json couldn't be fetched in ResolveRemoteAccountService

* Add specs

* Randomize sidekiq-scheduler cron schedule (mastodon#4980)

SubscriptionsScheduler in particular causes high load across the
entire fediverse at 5 AM UTC every day. Randomizing cron schedules
and/or adding a random delay is considered best practice to avoid
this issue.

* Fix cancellation of scroll to the right (mastodon#4978)

* Raise an error on getting activity uri for remote status (mastodon#4984)

We had returned `nil` for that case, but this raises an error instead, as a wrong usage of the method.
This method is currently only used in ActivitySerializer.

* Validate uri presence for remote status (mastodon#4985)

* Oauth code in input form and add description message (mastodon#4986)

* Oauth code in a input form and add description

* New authcode description

* Some improvements in the Spanish translation (mastodon#4991)

* Add `strip_insignificant_zeros: true` option to `number_to_human` (mastodon#4993)

* Bump ruby version to 2.4.2 (mastodon#4958)

* Bump ruby version to 2.4.2

https://www.ruby-lang.org/en/news/2017/09/14/ruby-2-4-2-released/

Gemfile.lock is also updated.

TODO
- [ ] Update Dockerfile with Alpine release of ruby-2.4.2

* Revert jwt version

It seems that jwt 2.0.0 fails in some environment.
ref. zaru/webpush#42

* Bump Ruby version on docker image

* Use Account.local.sum(statuses_count) instead of Status.local.count (mastodon#4996)

It is faster.

* Do not add image size without meta to OGP (regression from mastodon#4901) (mastodon#4995)

* Add published property to ActivityPub activity for reblogs (mastodon#5000)

Since reblogs are serialized as Announce activity, its published property can be used for the creation time of reblog.

* Re-allow underscore on valid_url_path_ending_chars (mastodon#4999)

Limiting allowed characters in the last character of the URL is came from twitter-text, but underscore is allowed on there, and Mastodon before mastodon#4941.

* Add will-change to improve scrolling perf (mastodon#5001)

* A few updates to the Spanish translation and some typos fixing (mastodon#4997)

* So Spanish. Much changes. Wow.

* Some little fixes

* Updated es.yml, and fixed some ortographical errors

* Some little changes to simple_form.es.yml

* Yeah, so much translations

* Spanish e-mail messages

* Remove unused message

* Custom emoji (mastodon#4988)

* Custom emoji

- In OStatus: `<link rel="emoji" name="coolcat" href="http://..." />`
- In ActivityPub: `{ type: "Emoji", name: ":coolcat:", href: "http://..." }`
- In REST API: Status object includes `emojis` array (`shortcode`, `url`)
- Domain blocks with reject media stop emojis
- Emoji file up to 50KB
- Web UI handles custom emojis
- Static pages render custom emojis as `<img />` tags

Side effects:

- Undo mastodon#4500 optimization, as I needed to modify it to restore
  shortcode handling in emojify()
- Formatter#plaintext should now make sure stripped out line-breaks
  and paragraphs are replaced with newlines

* Fix emoji at the start not being converted

* Run i18n-tasks normalize (mastodon#5003)

* Admin interface for listing, adding and removing custom emojis (mastodon#5002)

* Admin interface for listing, adding and removing custom emojis

* Only display local ones in the list

* Define emoji context for ActivityPub (mastodon#5004)

* Define emoji context for ActivityPub

* Fix the emojo

* Use general Mastodon context instead

* Fix custom emojis index (mastodon#5006)

* Remove ubuntu-toolchain-r-test (mastodon#5005)

* Fix incomplete account records being read (mastodon#4998)

* Fix incomplete account records being read

- Put account processing into redis lock
- Do not save until record is complete

* Fix spaces

* Hide sensitive image in default on the public pages (mastodon#5009)

Additionally, this restores previous background / text color for media spoiler.

* Add support for multiple themes (mastodon#4959)

* Add support for selecting a theme

* Fix codeclimate issues

* Look up site default style if current user is not available due to e.g. not being logged in

* Remove outdated comment in common.js

* Address requested changes in themes PR

* Fix codeclimate issues

* Explicitly check current_account in application controller and only check theme availability if non-nil

* codeclimate

* explicit precedence with &&

* Fix code style in application_controller according to @nightpool's suggestion, use default style in embedded.html.haml

* codeclimate: indentation + return

* Use OrderedCollectionPage to return followers/following list (mastodon#4949)

* Set touchstart listener to 'passive', remove 'once' (mastodon#5011)

* Fix non-local statuses are html_encoded in public_page. (mastodon#5012)

* Introduce OStatus::TagManager (mastodon#5008)

* Fix race condition when processing incoming OStatus messages (mastodon#5013)

* Avoid races in incoming OStatus toots processing

* oops

* oops again

* i18n: Update Polish translation (mastodon#5015)

Signed-off-by: Marcin Mikołajczak <[email protected]>

* redo mastodon#4500 with customEmojis (mastodon#5016)

* Fix custom emojis with non-1:1 aspect ratio being stretched (mastodon#5017)

* Change IDs to strings rather than numbers in API JSON output (mastodon#5019)

* Fix JavaScript interface with long IDs

Somewhat predictably, the JS interface handled IDs as numbers, which in
JS are IEEE double-precision floats. This loses some precision when
working with numbers as large as those generated by the new ID scheme,
so we instead handle them here as strings. This is relatively simple,
and doesn't appear to have caused any problems, but should definitely
be tested more thoroughly than the built-in tests. Several days of use
appear to support this working properly.

BREAKING CHANGE:

The major(!) change here is that IDs are now returned as strings by the
REST endpoints, rather than as integers. In practice, relatively few
changes were required to make the existing JS UI work with this change,
but it will likely hit API clients pretty hard: it's an entirely
different type to consume. (The one API client I tested, Tusky, handles
this with no problems, however.)

Twitter ran into this issue when introducing Snowflake IDs, and decided
to instead introduce an `id_str` field in JSON responses. I have opted
to *not* do that, and instead force all IDs to 64-bit integers
represented by strings in one go. (I believe Twitter exacerbated their
problem by rolling out the changes three times: once for statuses, once
for DMs, and once for user IDs, as well as by leaving an integer ID
value in JSON. As they said, "If you’re using the `id` field with JSON
in a Javascript-related language, there is a very high likelihood that
the integers will be silently munged by Javascript interpreters. In most
cases, this will result in behavior such as being unable to load or
delete a specific direct message, because the ID you're sending to the
API is different than the actual identifier associated with the
message." [1]) However, given that this is a significant change for API
users, alternatives or a transition time may be appropriate.

1: https://blog.twitter.com/developer/en_us/a/2011/direct-messages-going-snowflake-on-sep-30-2011.html

* Additional fixes for stringified IDs in JSON

These should be the last two. These were identified using eslint to try
to identify any plain casts to JavaScript numbers. (Some such casts are
legitimate, but these were not.)

Adding the following to .eslintrc.yml will identify casts to numbers:

~~~
  no-restricted-syntax:
  - warn
  - selector: UnaryExpression[operator='+'] > :not(Literal)
    message: Avoid the use of unary +
  - selector: CallExpression[callee.name='Number']
    message: Casting with Number() may coerce string IDs to numbers
~~~

The remaining three casts appear legitimate: two casts to array indices,
one in a server to turn an environment variable into a number.

* Back out RelationshipsController Change

This was made to make a test a bit less flakey, but has nothing to
do with this branch.

* Change internal streaming payloads to stringified IDs as well

Per
mastodon#5019 (comment)
we need these changes to send deleted status IDs as strings, not
integers.

* Add japanese translations for custom emoji. (mastodon#5007)

* Add japanese translations for custom emoji.

* Remove spaces.

* Change destroyed_msg.

* Revert "Change destroyed_msg."

* l10n Occitan: theme site, custom emoji, Oauth, newcomers follow admins, ... (mastodon#5023)

* Oauto code string

* Theme, custom emoji and more

* Site theme

* added two spaces wierdly gone away

* Update oc.yml

Changes adviced

* Remove unnecessary css property (mastodon#5025)

* Add Japanese translations for multiple themes, custom emoji (mastodon#5026)

* Add Japanese translations for multiple themes

* Change Japanese translation for custom emoji's created_msg.

* Better Japanese translations (mastodon#5030)
@aschmitz
Copy link
Copy Markdown
Contributor Author

The note about optLong in Android is interesting, although that's a surprising behavior. It looks like Tusky actually stores IDs as strings anyway, though? (For example, see https://github.com/Vavassor/Tusky/blob/master/app/src/main/java/com/keylesspalace/tusky/entity/Status.java#L65 .) At any rate, Tusky is able to reply to, reblog, and delete statuses using the right, non-truncated ID, so I suspect the string type may be saving it from having to make any changes in this case.

@clworld
Copy link
Copy Markdown
Contributor

clworld commented Sep 21, 2017

@aschmitz Oh, sorry. I misguessed Tusky's implementation. Excuse me.

lindwurm added a commit to akane-blue/mastodon that referenced this pull request Sep 22, 2017
Gargron pushed a commit that referenced this pull request Oct 4, 2017
* Use non-serial IDs

This change makes a number of nontrivial tweaks to the data model in
Mastodon:

* All IDs are now 8 byte integers (rather than mixed 4- and 8-byte)
* IDs are now assigned as:
  * Top 6 bytes: millisecond-resolution time from epoch
  * Bottom 2 bytes: serial (within the millisecond) sequence number
  * See /lib/tasks/db.rake's `define_timestamp_id` for details, but
    note that the purpose of these changes is to make it difficult to
    determine the number of objects in a table from the ID of any
    object.
* The Redis sorted set used for the feed will have values used to look
  up toots, rather than scores. This is almost always the same as the
  existing behavior, except in the case of boosted toots. This change
  was made because Redis stores scores as double-precision floats,
  which cannot store the new ID format exactly. Note that this doesn't
  cause problems with sorting/pagination, because ZREVRANGEBYSCORE
  sorts lexicographically when scores are tied. (This will still cause
  sorting issues when the ID gains a new significant digit, but that's
  extraordinarily uncommon.)

Note a couple of tradeoffs have been made in this commit:

* lib/tasks/db.rake is used to enforce many/most column constraints,
  because this commit seems likely to take a while to bring upstream.
  Enforcing a post-migrate hook is an easier way to maintain the code
  in the interim.
* Boosted toots will appear in the timeline as many times as they have
  been boosted. This is a tradeoff due to the way the feed is saved in
  Redis at the moment, but will be handled by a future commit.

This would effectively close Mastodon's #1059, as it is a
snowflake-like system of generating IDs. However, given how involved
the changes were simply within Mastodon, it may have unexpected
interactions with some clients, if they store IDs as doubles
(or as 4-byte integers). This was a problem that Twitter ran into with
their "snowflake" transition, particularly in JavaScript clients that
treated IDs as JS integers, rather than strings. It therefore would be
useful to test these changes at least in the web interface and popular
clients before pushing them to all users.

* Fix JavaScript interface with long IDs

Somewhat predictably, the JS interface handled IDs as numbers, which in
JS are IEEE double-precision floats. This loses some precision when
working with numbers as large as those generated by the new ID scheme,
so we instead handle them here as strings. This is relatively simple,
and doesn't appear to have caused any problems, but should definitely
be tested more thoroughly than the built-in tests. Several days of use
appear to support this working properly.

BREAKING CHANGE:

The major(!) change here is that IDs are now returned as strings by the
REST endpoints, rather than as integers. In practice, relatively few
changes were required to make the existing JS UI work with this change,
but it will likely hit API clients pretty hard: it's an entirely
different type to consume. (The one API client I tested, Tusky, handles
this with no problems, however.)

Twitter ran into this issue when introducing Snowflake IDs, and decided
to instead introduce an `id_str` field in JSON responses. I have opted
to *not* do that, and instead force all IDs to 64-bit integers
represented by strings in one go. (I believe Twitter exacerbated their
problem by rolling out the changes three times: once for statuses, once
for DMs, and once for user IDs, as well as by leaving an integer ID
value in JSON. As they said, "If you’re using the `id` field with JSON
in a Javascript-related language, there is a very high likelihood that
the integers will be silently munged by Javascript interpreters. In most
cases, this will result in behavior such as being unable to load or
delete a specific direct message, because the ID you're sending to the
API is different than the actual identifier associated with the
message." [1]) However, given that this is a significant change for API
users, alternatives or a transition time may be appropriate.

1: https://blog.twitter.com/developer/en_us/a/2011/direct-messages-going-snowflake-on-sep-30-2011.html

* Restructure feed pushes/unpushes

This was necessary because the previous behavior used Redis zset scores
to identify statuses, but those are IEEE double-precision floats, so we
can't actually use them to identify all 64-bit IDs. However, it leaves
the code in a much better state for refactoring reblog handling /
coalescing.

Feed-management code has been consolidated in FeedManager, including:

* BatchedRemoveStatusService no longer directly manipulates feed zsets
* RemoveStatusService no longer directly manipulates feed zsets
* PrecomputeFeedService has moved its logic to FeedManager#populate_feed

(PrecomputeFeedService largely made lots of calls to FeedManager, but
didn't follow the normal adding-to-feed process.)

This has the effect of unifying all of the feed push/unpush logic in
FeedManager, making it much more tractable to update it in the future.

Due to some additional checks that must be made during, for example,
batch status removals, some Redis pipelining has been removed. It does
not appear that this should cause significantly increased load, but if
necessary, some optimizations are possible in batch cases. These were
omitted in the pursuit of simplicity, but a batch_push and batch_unpush
would be possible in the future.

Tests were added to verify that pushes happen under expected conditions,
and to verify reblog behavior (both on pushing and unpushing). In the
case of unpushing, this includes testing behavior that currently leads
to confusion such as Mastodon's #2817, but this codifies that the
behavior is currently expected.

* Rubocop fixes

I could swear I made these changes already, but I must have lost them
somewhere along the line.

* Address review comments

This addresses the first two comments from review of this feature:

#4801 (comment)
#4801 (comment)

This adds an optional argument to FeedManager#key, the subtype of feed
key to generate. It also tests to ensure that FeedManager's settings are
such that reblogs won't be tracked forever.

* Hardcode IdToBigints migration columns

This addresses a comment during review:
#4801 (comment)

This means we'll need to make sure that all _id columns going forward
are bigints, but that should happen automatically in most cases.

* Additional fixes for stringified IDs in JSON

These should be the last two. These were identified using eslint to try
to identify any plain casts to JavaScript numbers. (Some such casts are
legitimate, but these were not.)

Adding the following to .eslintrc.yml will identify casts to numbers:

~~~
  no-restricted-syntax:
  - warn
  - selector: UnaryExpression[operator='+'] > :not(Literal)
    message: Avoid the use of unary +
  - selector: CallExpression[callee.name='Number']
    message: Casting with Number() may coerce string IDs to numbers
~~~

The remaining three casts appear legitimate: two casts to array indices,
one in a server to turn an environment variable into a number.

* Only implement timestamp IDs for Status IDs

Per discussion in #4801, this is only being merged in for Status IDs at
this point. We do this in a migration, as there is no longer use for
a post-migration hook. We keep the initialization of the timestamp_id
function as a Rake task, as it is also needed after db:schema:load (as
db/schema.rb doesn't store Postgres functions).

* Change internal streaming payloads to stringified IDs as well

This is equivalent to 591a9af from
#5019, with an extra change for the addition to FeedManager#unpush.

* Ensure we have a status_id_seq sequence

Apparently this is not a given when specifying a custom ID function,
so now we ensure it gets created. This uses the generic version of this
function to more easily support adding additional tables with timestamp
IDs in the future, although it would be possible to cut this down to a
less generic version if necessary. It is only run during db:schema:load
or the relevant migration, so the overhead is extraordinarily minimal.

* Transition reblogs to new Redis format

This provides a one-way migration to transition old Redis reblog entries
into the new format, with a separate tracking entry for reblogs.

It is not invertible because doing so could (if timestamp IDs are used)
require a database query for each status in each users' feed, which is
likely to be a significant toll on major instances.

* Address review comments from @akihikodaki

No functional changes.

* Additional review changes

* Heredoc cleanup

* Run db:schema:load hooks for test in development

This matches the behavior in Rails'
ActiveRecord::Tasks::DatabaseTasks.each_current_configuration, which
would otherwise break `rake db:setup` in development.

It also moves some functionality out to a library, which will be a good
place to put additional related functionality in the near future.
rutan pushed a commit to rutan/mastodon that referenced this pull request Oct 11, 2017
…n#5019)

* Fix JavaScript interface with long IDs

Somewhat predictably, the JS interface handled IDs as numbers, which in
JS are IEEE double-precision floats. This loses some precision when
working with numbers as large as those generated by the new ID scheme,
so we instead handle them here as strings. This is relatively simple,
and doesn't appear to have caused any problems, but should definitely
be tested more thoroughly than the built-in tests. Several days of use
appear to support this working properly.

BREAKING CHANGE:

The major(!) change here is that IDs are now returned as strings by the
REST endpoints, rather than as integers. In practice, relatively few
changes were required to make the existing JS UI work with this change,
but it will likely hit API clients pretty hard: it's an entirely
different type to consume. (The one API client I tested, Tusky, handles
this with no problems, however.)

Twitter ran into this issue when introducing Snowflake IDs, and decided
to instead introduce an `id_str` field in JSON responses. I have opted
to *not* do that, and instead force all IDs to 64-bit integers
represented by strings in one go. (I believe Twitter exacerbated their
problem by rolling out the changes three times: once for statuses, once
for DMs, and once for user IDs, as well as by leaving an integer ID
value in JSON. As they said, "If you’re using the `id` field with JSON
in a Javascript-related language, there is a very high likelihood that
the integers will be silently munged by Javascript interpreters. In most
cases, this will result in behavior such as being unable to load or
delete a specific direct message, because the ID you're sending to the
API is different than the actual identifier associated with the
message." [1]) However, given that this is a significant change for API
users, alternatives or a transition time may be appropriate.

1: https://blog.twitter.com/developer/en_us/a/2011/direct-messages-going-snowflake-on-sep-30-2011.html

* Additional fixes for stringified IDs in JSON

These should be the last two. These were identified using eslint to try
to identify any plain casts to JavaScript numbers. (Some such casts are
legitimate, but these were not.)

Adding the following to .eslintrc.yml will identify casts to numbers:

~~~
  no-restricted-syntax:
  - warn
  - selector: UnaryExpression[operator='+'] > :not(Literal)
    message: Avoid the use of unary +
  - selector: CallExpression[callee.name='Number']
    message: Casting with Number() may coerce string IDs to numbers
~~~

The remaining three casts appear legitimate: two casts to array indices,
one in a server to turn an environment variable into a number.

* Back out RelationshipsController Change

This was made to make a test a bit less flakey, but has nothing to
do with this branch.

* Change internal streaming payloads to stringified IDs as well

Per
mastodon#5019 (comment)
we need these changes to send deleted status IDs as strings, not
integers.
rutan pushed a commit to rutan/mastodon that referenced this pull request Oct 11, 2017
* Use non-serial IDs

This change makes a number of nontrivial tweaks to the data model in
Mastodon:

* All IDs are now 8 byte integers (rather than mixed 4- and 8-byte)
* IDs are now assigned as:
  * Top 6 bytes: millisecond-resolution time from epoch
  * Bottom 2 bytes: serial (within the millisecond) sequence number
  * See /lib/tasks/db.rake's `define_timestamp_id` for details, but
    note that the purpose of these changes is to make it difficult to
    determine the number of objects in a table from the ID of any
    object.
* The Redis sorted set used for the feed will have values used to look
  up toots, rather than scores. This is almost always the same as the
  existing behavior, except in the case of boosted toots. This change
  was made because Redis stores scores as double-precision floats,
  which cannot store the new ID format exactly. Note that this doesn't
  cause problems with sorting/pagination, because ZREVRANGEBYSCORE
  sorts lexicographically when scores are tied. (This will still cause
  sorting issues when the ID gains a new significant digit, but that's
  extraordinarily uncommon.)

Note a couple of tradeoffs have been made in this commit:

* lib/tasks/db.rake is used to enforce many/most column constraints,
  because this commit seems likely to take a while to bring upstream.
  Enforcing a post-migrate hook is an easier way to maintain the code
  in the interim.
* Boosted toots will appear in the timeline as many times as they have
  been boosted. This is a tradeoff due to the way the feed is saved in
  Redis at the moment, but will be handled by a future commit.

This would effectively close Mastodon's mastodon#1059, as it is a
snowflake-like system of generating IDs. However, given how involved
the changes were simply within Mastodon, it may have unexpected
interactions with some clients, if they store IDs as doubles
(or as 4-byte integers). This was a problem that Twitter ran into with
their "snowflake" transition, particularly in JavaScript clients that
treated IDs as JS integers, rather than strings. It therefore would be
useful to test these changes at least in the web interface and popular
clients before pushing them to all users.

* Fix JavaScript interface with long IDs

Somewhat predictably, the JS interface handled IDs as numbers, which in
JS are IEEE double-precision floats. This loses some precision when
working with numbers as large as those generated by the new ID scheme,
so we instead handle them here as strings. This is relatively simple,
and doesn't appear to have caused any problems, but should definitely
be tested more thoroughly than the built-in tests. Several days of use
appear to support this working properly.

BREAKING CHANGE:

The major(!) change here is that IDs are now returned as strings by the
REST endpoints, rather than as integers. In practice, relatively few
changes were required to make the existing JS UI work with this change,
but it will likely hit API clients pretty hard: it's an entirely
different type to consume. (The one API client I tested, Tusky, handles
this with no problems, however.)

Twitter ran into this issue when introducing Snowflake IDs, and decided
to instead introduce an `id_str` field in JSON responses. I have opted
to *not* do that, and instead force all IDs to 64-bit integers
represented by strings in one go. (I believe Twitter exacerbated their
problem by rolling out the changes three times: once for statuses, once
for DMs, and once for user IDs, as well as by leaving an integer ID
value in JSON. As they said, "If you’re using the `id` field with JSON
in a Javascript-related language, there is a very high likelihood that
the integers will be silently munged by Javascript interpreters. In most
cases, this will result in behavior such as being unable to load or
delete a specific direct message, because the ID you're sending to the
API is different than the actual identifier associated with the
message." [1]) However, given that this is a significant change for API
users, alternatives or a transition time may be appropriate.

1: https://blog.twitter.com/developer/en_us/a/2011/direct-messages-going-snowflake-on-sep-30-2011.html

* Restructure feed pushes/unpushes

This was necessary because the previous behavior used Redis zset scores
to identify statuses, but those are IEEE double-precision floats, so we
can't actually use them to identify all 64-bit IDs. However, it leaves
the code in a much better state for refactoring reblog handling /
coalescing.

Feed-management code has been consolidated in FeedManager, including:

* BatchedRemoveStatusService no longer directly manipulates feed zsets
* RemoveStatusService no longer directly manipulates feed zsets
* PrecomputeFeedService has moved its logic to FeedManager#populate_feed

(PrecomputeFeedService largely made lots of calls to FeedManager, but
didn't follow the normal adding-to-feed process.)

This has the effect of unifying all of the feed push/unpush logic in
FeedManager, making it much more tractable to update it in the future.

Due to some additional checks that must be made during, for example,
batch status removals, some Redis pipelining has been removed. It does
not appear that this should cause significantly increased load, but if
necessary, some optimizations are possible in batch cases. These were
omitted in the pursuit of simplicity, but a batch_push and batch_unpush
would be possible in the future.

Tests were added to verify that pushes happen under expected conditions,
and to verify reblog behavior (both on pushing and unpushing). In the
case of unpushing, this includes testing behavior that currently leads
to confusion such as Mastodon's mastodon#2817, but this codifies that the
behavior is currently expected.

* Rubocop fixes

I could swear I made these changes already, but I must have lost them
somewhere along the line.

* Address review comments

This addresses the first two comments from review of this feature:

mastodon#4801 (comment)
mastodon#4801 (comment)

This adds an optional argument to FeedManager#key, the subtype of feed
key to generate. It also tests to ensure that FeedManager's settings are
such that reblogs won't be tracked forever.

* Hardcode IdToBigints migration columns

This addresses a comment during review:
mastodon#4801 (comment)

This means we'll need to make sure that all _id columns going forward
are bigints, but that should happen automatically in most cases.

* Additional fixes for stringified IDs in JSON

These should be the last two. These were identified using eslint to try
to identify any plain casts to JavaScript numbers. (Some such casts are
legitimate, but these were not.)

Adding the following to .eslintrc.yml will identify casts to numbers:

~~~
  no-restricted-syntax:
  - warn
  - selector: UnaryExpression[operator='+'] > :not(Literal)
    message: Avoid the use of unary +
  - selector: CallExpression[callee.name='Number']
    message: Casting with Number() may coerce string IDs to numbers
~~~

The remaining three casts appear legitimate: two casts to array indices,
one in a server to turn an environment variable into a number.

* Only implement timestamp IDs for Status IDs

Per discussion in mastodon#4801, this is only being merged in for Status IDs at
this point. We do this in a migration, as there is no longer use for
a post-migration hook. We keep the initialization of the timestamp_id
function as a Rake task, as it is also needed after db:schema:load (as
db/schema.rb doesn't store Postgres functions).

* Change internal streaming payloads to stringified IDs as well

This is equivalent to 591a9af from
mastodon#5019, with an extra change for the addition to FeedManager#unpush.

* Ensure we have a status_id_seq sequence

Apparently this is not a given when specifying a custom ID function,
so now we ensure it gets created. This uses the generic version of this
function to more easily support adding additional tables with timestamp
IDs in the future, although it would be possible to cut this down to a
less generic version if necessary. It is only run during db:schema:load
or the relevant migration, so the overhead is extraordinarily minimal.

* Transition reblogs to new Redis format

This provides a one-way migration to transition old Redis reblog entries
into the new format, with a separate tracking entry for reblogs.

It is not invertible because doing so could (if timestamp IDs are used)
require a database query for each status in each users' feed, which is
likely to be a significant toll on major instances.

* Address review comments from @akihikodaki

No functional changes.

* Additional review changes

* Heredoc cleanup

* Run db:schema:load hooks for test in development

This matches the behavior in Rails'
ActiveRecord::Tasks::DatabaseTasks.each_current_configuration, which
would otherwise break `rake db:setup` in development.

It also moves some functionality out to a library, which will be a good
place to put additional related functionality in the near future.
cobodo pushed a commit to cobodo/mastodon that referenced this pull request Oct 20, 2017
* Use non-serial IDs

This change makes a number of nontrivial tweaks to the data model in
Mastodon:

* All IDs are now 8 byte integers (rather than mixed 4- and 8-byte)
* IDs are now assigned as:
  * Top 6 bytes: millisecond-resolution time from epoch
  * Bottom 2 bytes: serial (within the millisecond) sequence number
  * See /lib/tasks/db.rake's `define_timestamp_id` for details, but
    note that the purpose of these changes is to make it difficult to
    determine the number of objects in a table from the ID of any
    object.
* The Redis sorted set used for the feed will have values used to look
  up toots, rather than scores. This is almost always the same as the
  existing behavior, except in the case of boosted toots. This change
  was made because Redis stores scores as double-precision floats,
  which cannot store the new ID format exactly. Note that this doesn't
  cause problems with sorting/pagination, because ZREVRANGEBYSCORE
  sorts lexicographically when scores are tied. (This will still cause
  sorting issues when the ID gains a new significant digit, but that's
  extraordinarily uncommon.)

Note a couple of tradeoffs have been made in this commit:

* lib/tasks/db.rake is used to enforce many/most column constraints,
  because this commit seems likely to take a while to bring upstream.
  Enforcing a post-migrate hook is an easier way to maintain the code
  in the interim.
* Boosted toots will appear in the timeline as many times as they have
  been boosted. This is a tradeoff due to the way the feed is saved in
  Redis at the moment, but will be handled by a future commit.

This would effectively close Mastodon's #1059, as it is a
snowflake-like system of generating IDs. However, given how involved
the changes were simply within Mastodon, it may have unexpected
interactions with some clients, if they store IDs as doubles
(or as 4-byte integers). This was a problem that Twitter ran into with
their "snowflake" transition, particularly in JavaScript clients that
treated IDs as JS integers, rather than strings. It therefore would be
useful to test these changes at least in the web interface and popular
clients before pushing them to all users.

* Fix JavaScript interface with long IDs

Somewhat predictably, the JS interface handled IDs as numbers, which in
JS are IEEE double-precision floats. This loses some precision when
working with numbers as large as those generated by the new ID scheme,
so we instead handle them here as strings. This is relatively simple,
and doesn't appear to have caused any problems, but should definitely
be tested more thoroughly than the built-in tests. Several days of use
appear to support this working properly.

BREAKING CHANGE:

The major(!) change here is that IDs are now returned as strings by the
REST endpoints, rather than as integers. In practice, relatively few
changes were required to make the existing JS UI work with this change,
but it will likely hit API clients pretty hard: it's an entirely
different type to consume. (The one API client I tested, Tusky, handles
this with no problems, however.)

Twitter ran into this issue when introducing Snowflake IDs, and decided
to instead introduce an `id_str` field in JSON responses. I have opted
to *not* do that, and instead force all IDs to 64-bit integers
represented by strings in one go. (I believe Twitter exacerbated their
problem by rolling out the changes three times: once for statuses, once
for DMs, and once for user IDs, as well as by leaving an integer ID
value in JSON. As they said, "If you’re using the `id` field with JSON
in a Javascript-related language, there is a very high likelihood that
the integers will be silently munged by Javascript interpreters. In most
cases, this will result in behavior such as being unable to load or
delete a specific direct message, because the ID you're sending to the
API is different than the actual identifier associated with the
message." [1]) However, given that this is a significant change for API
users, alternatives or a transition time may be appropriate.

1: https://blog.twitter.com/developer/en_us/a/2011/direct-messages-going-snowflake-on-sep-30-2011.html

* Restructure feed pushes/unpushes

This was necessary because the previous behavior used Redis zset scores
to identify statuses, but those are IEEE double-precision floats, so we
can't actually use them to identify all 64-bit IDs. However, it leaves
the code in a much better state for refactoring reblog handling /
coalescing.

Feed-management code has been consolidated in FeedManager, including:

* BatchedRemoveStatusService no longer directly manipulates feed zsets
* RemoveStatusService no longer directly manipulates feed zsets
* PrecomputeFeedService has moved its logic to FeedManager#populate_feed

(PrecomputeFeedService largely made lots of calls to FeedManager, but
didn't follow the normal adding-to-feed process.)

This has the effect of unifying all of the feed push/unpush logic in
FeedManager, making it much more tractable to update it in the future.

Due to some additional checks that must be made during, for example,
batch status removals, some Redis pipelining has been removed. It does
not appear that this should cause significantly increased load, but if
necessary, some optimizations are possible in batch cases. These were
omitted in the pursuit of simplicity, but a batch_push and batch_unpush
would be possible in the future.

Tests were added to verify that pushes happen under expected conditions,
and to verify reblog behavior (both on pushing and unpushing). In the
case of unpushing, this includes testing behavior that currently leads
to confusion such as Mastodon's mastodon#2817, but this codifies that the
behavior is currently expected.

* Rubocop fixes

I could swear I made these changes already, but I must have lost them
somewhere along the line.

* Address review comments

This addresses the first two comments from review of this feature:

mastodon#4801 (comment)
mastodon#4801 (comment)

This adds an optional argument to FeedManager#key, the subtype of feed
key to generate. It also tests to ensure that FeedManager's settings are
such that reblogs won't be tracked forever.

* Hardcode IdToBigints migration columns

This addresses a comment during review:
mastodon#4801 (comment)

This means we'll need to make sure that all _id columns going forward
are bigints, but that should happen automatically in most cases.

* Additional fixes for stringified IDs in JSON

These should be the last two. These were identified using eslint to try
to identify any plain casts to JavaScript numbers. (Some such casts are
legitimate, but these were not.)

Adding the following to .eslintrc.yml will identify casts to numbers:

~~~
  no-restricted-syntax:
  - warn
  - selector: UnaryExpression[operator='+'] > :not(Literal)
    message: Avoid the use of unary +
  - selector: CallExpression[callee.name='Number']
    message: Casting with Number() may coerce string IDs to numbers
~~~

The remaining three casts appear legitimate: two casts to array indices,
one in a server to turn an environment variable into a number.

* Only implement timestamp IDs for Status IDs

Per discussion in mastodon#4801, this is only being merged in for Status IDs at
this point. We do this in a migration, as there is no longer use for
a post-migration hook. We keep the initialization of the timestamp_id
function as a Rake task, as it is also needed after db:schema:load (as
db/schema.rb doesn't store Postgres functions).

* Change internal streaming payloads to stringified IDs as well

This is equivalent to 591a9af from
mastodon#5019, with an extra change for the addition to FeedManager#unpush.

* Ensure we have a status_id_seq sequence

Apparently this is not a given when specifying a custom ID function,
so now we ensure it gets created. This uses the generic version of this
function to more easily support adding additional tables with timestamp
IDs in the future, although it would be possible to cut this down to a
less generic version if necessary. It is only run during db:schema:load
or the relevant migration, so the overhead is extraordinarily minimal.

* Transition reblogs to new Redis format

This provides a one-way migration to transition old Redis reblog entries
into the new format, with a separate tracking entry for reblogs.

It is not invertible because doing so could (if timestamp IDs are used)
require a database query for each status in each users' feed, which is
likely to be a significant toll on major instances.

* Address review comments from @akihikodaki

No functional changes.

* Additional review changes

* Heredoc cleanup

* Run db:schema:load hooks for test in development

This matches the behavior in Rails'
ActiveRecord::Tasks::DatabaseTasks.each_current_configuration, which
would otherwise break `rake db:setup` in development.

It also moves some functionality out to a library, which will be a good
place to put additional related functionality in the near future.
nilsding added a commit to nilsding/mastodon.cr that referenced this pull request May 1, 2022
This change has been introduced back in 2017 already as part of
mastodon/mastodon#5019.  I think by now it's safe to assume that this
field is always a string.
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.

5 participants