Change IDs to strings rather than numbers in API JSON output#5019
Change IDs to strings rather than numbers in API JSON output#5019Gargron merged 4 commits intomastodon:masterfrom
Conversation
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.
|
Do we have to worry about JSON serialized to Redis or similar? what about the streaming API? |
|
Redis says this about scores:
We use status ids as scores in feed_manager so that's probably an issue |
|
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. |
|
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. |
|
seems like an issue given the score stuff above, and we may need to choose a way to pad it to make the lexicographic order correct. this also seems like an issue |
|
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.) |
|
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. |
|
(which are stored in redis as JSON values afaik) |
|
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 |
|
I'm not sure that helps. message is something like |
|
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.
|
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. |
|
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 |
|
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. |
This is equivalent to 591a9af from mastodon#5019, with an extra change for the addition to FeedManager#unpush.
|
This PR affects native client apps. |
|
Correct, this is a breaking change and will be released with Mastodon 2.0 |
|
I'm sorry. I was misunderstanding. |
|
Some thought: and FYI: PR note says "Tusky handled this change with no trouble.". |
* 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)
|
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. |
|
@aschmitz Oh, sorry. I misguessed Tusky's implementation. Excuse me. |
…astodon#5019)" This reverts commit 669fe9e. temp Signed-off-by: lindwurm <[email protected]>
* 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.
…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.
* 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.
* 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.
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.
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.