From 0d5f3ae33135f2cae28c13df6bba63d396dd08cc Mon Sep 17 00:00:00 2001 From: ThibG Date: Sun, 10 Nov 2019 23:04:48 +0100 Subject: Fix already-known remote private toots not being searchable by URL (#12336) * Fix already-known remote private toots not being searchable by URL Whenever an URL search fails, and an account is provided, try getting a private status matching that URL from database. * Fix searching by `id` field instead of `uri` --- app/services/resolve_url_service.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'app/services') diff --git a/app/services/resolve_url_service.rb b/app/services/resolve_url_service.rb index aa883597a..4e971a4b8 100644 --- a/app/services/resolve_url_service.rb +++ b/app/services/resolve_url_service.rb @@ -24,6 +24,12 @@ class ResolveURLService < BaseService status = FetchRemoteStatusService.new.call(resource_url, body, protocol) authorize_with @on_behalf_of, status, :show? unless status.nil? status + elsif fetched_resource.nil? && @on_behalf_of.present? + # It may happen that the resource is a private toot, and thus not fetchable, + # but we can return the toot if we already know about it. + status = Status.find_by(uri: @url) || Status.find_by(url: @url) + authorize_with @on_behalf_of, status, :show? unless status.nil? + status end end -- cgit From dfea7368c934f600bd0b6b93b4a6c008a4e265b0 Mon Sep 17 00:00:00 2001 From: ThibG Date: Wed, 13 Nov 2019 23:02:10 +0100 Subject: Add bookmarks (#7107) * Add backend support for bookmarks Bookmarks behave like favourites, except they aren't shared with other users and do not have an associated counter. * Add spec for bookmark endpoints * Add front-end support for bookmarks * Introduce OAuth scopes for bookmarks * Add bookmarks to archive takeout * Fix migration * Coding style fixes * Fix rebase issue * Update bookmarked_statuses to latest UI changes * Update bookmark actions to properly reflect status changes in state * Add bookmarks item to single-column layout * Make active bookmarks red --- app/controllers/api/v1/bookmarks_controller.rb | 67 +++++++++++++ .../api/v1/statuses/bookmarks_controller.rb | 39 ++++++++ app/javascript/mastodon/actions/bookmarks.js | 90 ++++++++++++++++++ app/javascript/mastodon/actions/interactions.js | 80 ++++++++++++++++ .../mastodon/components/status_action_bar.js | 7 ++ .../mastodon/containers/status_container.js | 10 ++ .../mastodon/features/bookmarked_statuses/index.js | 104 +++++++++++++++++++++ .../mastodon/features/getting_started/index.js | 4 +- .../features/status/components/action_bar.js | 7 ++ app/javascript/mastodon/features/status/index.js | 11 +++ .../features/ui/components/columns_area.js | 2 + .../features/ui/components/navigation_panel.js | 1 + app/javascript/mastodon/features/ui/index.js | 2 + .../mastodon/features/ui/util/async-components.js | 4 + app/javascript/mastodon/reducers/status_lists.js | 29 ++++++ app/javascript/mastodon/reducers/statuses.js | 6 ++ app/javascript/styles/mastodon/components.scss | 4 + app/javascript/styles/mastodon/variables.scss | 2 + app/models/bookmark.rb | 26 ++++++ app/models/concerns/account_associations.rb | 1 + app/models/concerns/account_interactions.rb | 4 + app/models/status.rb | 5 + app/presenters/status_relationships_presenter.rb | 2 + app/serializers/rest/status_serializer.rb | 9 ++ app/services/backup_service.rb | 21 +++++ config/initializers/doorkeeper.rb | 2 + config/locales/doorkeeper.en.yml | 2 + config/routes.rb | 4 + db/migrate/20180831171112_create_bookmarks.rb | 17 ++++ db/schema.rb | 12 +++ .../api/v1/bookmarks_controller_spec.rb | 78 ++++++++++++++++ .../api/v1/statuses/bookmarks_controller_spec.rb | 57 +++++++++++ spec/fabricators/bookmark_fabricator.rb | 4 + 33 files changed, 712 insertions(+), 1 deletion(-) create mode 100644 app/controllers/api/v1/bookmarks_controller.rb create mode 100644 app/controllers/api/v1/statuses/bookmarks_controller.rb create mode 100644 app/javascript/mastodon/actions/bookmarks.js create mode 100644 app/javascript/mastodon/features/bookmarked_statuses/index.js create mode 100644 app/models/bookmark.rb create mode 100644 db/migrate/20180831171112_create_bookmarks.rb create mode 100644 spec/controllers/api/v1/bookmarks_controller_spec.rb create mode 100644 spec/controllers/api/v1/statuses/bookmarks_controller_spec.rb create mode 100644 spec/fabricators/bookmark_fabricator.rb (limited to 'app/services') diff --git a/app/controllers/api/v1/bookmarks_controller.rb b/app/controllers/api/v1/bookmarks_controller.rb new file mode 100644 index 000000000..cf4cba8dd --- /dev/null +++ b/app/controllers/api/v1/bookmarks_controller.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +class Api::V1::BookmarksController < Api::BaseController + before_action -> { doorkeeper_authorize! :read, :'read:bookmarks' } + before_action :require_user! + after_action :insert_pagination_headers + + respond_to :json + + def index + @statuses = load_statuses + render json: @statuses, each_serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id) + end + + private + + def load_statuses + cached_bookmarks + end + + def cached_bookmarks + cache_collection( + Status.reorder(nil).joins(:bookmarks).merge(results), + Status + ) + end + + def results + @_results ||= account_bookmarks.paginate_by_max_id( + limit_param(DEFAULT_STATUSES_LIMIT), + params[:max_id], + params[:since_id] + ) + end + + def account_bookmarks + current_account.bookmarks + end + + def insert_pagination_headers + set_pagination_headers(next_path, prev_path) + end + + def next_path + api_v1_bookmarks_url pagination_params(max_id: pagination_max_id) if records_continue? + end + + def prev_path + api_v1_bookmarks_url pagination_params(since_id: pagination_since_id) unless results.empty? + end + + def pagination_max_id + results.last.id + end + + def pagination_since_id + results.first.id + end + + def records_continue? + results.size == limit_param(DEFAULT_STATUSES_LIMIT) + end + + def pagination_params(core_params) + params.slice(:limit).permit(:limit).merge(core_params) + end +end diff --git a/app/controllers/api/v1/statuses/bookmarks_controller.rb b/app/controllers/api/v1/statuses/bookmarks_controller.rb new file mode 100644 index 000000000..bb9729cf5 --- /dev/null +++ b/app/controllers/api/v1/statuses/bookmarks_controller.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +class Api::V1::Statuses::BookmarksController < Api::BaseController + include Authorization + + before_action -> { doorkeeper_authorize! :write, :'write:bookmarks' } + before_action :require_user! + + respond_to :json + + def create + @status = bookmarked_status + render json: @status, serializer: REST::StatusSerializer + end + + def destroy + @status = requested_status + @bookmarks_map = { @status.id => false } + + bookmark = Bookmark.find_by!(account: current_user.account, status: @status) + bookmark.destroy! + + render json: @status, serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new([@status], current_user&.account_id, bookmarks_map: @bookmarks_map) + end + + private + + def bookmarked_status + authorize_with current_user.account, requested_status, :show? + + bookmark = Bookmark.find_or_create_by!(account: current_user.account, status: requested_status) + + bookmark.status.reload + end + + def requested_status + Status.find(params[:status_id]) + end +end diff --git a/app/javascript/mastodon/actions/bookmarks.js b/app/javascript/mastodon/actions/bookmarks.js new file mode 100644 index 000000000..544ed2ff2 --- /dev/null +++ b/app/javascript/mastodon/actions/bookmarks.js @@ -0,0 +1,90 @@ +import api, { getLinks } from '../api'; +import { importFetchedStatuses } from './importer'; + +export const BOOKMARKED_STATUSES_FETCH_REQUEST = 'BOOKMARKED_STATUSES_FETCH_REQUEST'; +export const BOOKMARKED_STATUSES_FETCH_SUCCESS = 'BOOKMARKED_STATUSES_FETCH_SUCCESS'; +export const BOOKMARKED_STATUSES_FETCH_FAIL = 'BOOKMARKED_STATUSES_FETCH_FAIL'; + +export const BOOKMARKED_STATUSES_EXPAND_REQUEST = 'BOOKMARKED_STATUSES_EXPAND_REQUEST'; +export const BOOKMARKED_STATUSES_EXPAND_SUCCESS = 'BOOKMARKED_STATUSES_EXPAND_SUCCESS'; +export const BOOKMARKED_STATUSES_EXPAND_FAIL = 'BOOKMARKED_STATUSES_EXPAND_FAIL'; + +export function fetchBookmarkedStatuses() { + return (dispatch, getState) => { + if (getState().getIn(['status_lists', 'bookmarks', 'isLoading'])) { + return; + } + + dispatch(fetchBookmarkedStatusesRequest()); + + api(getState).get('/api/v1/bookmarks').then(response => { + const next = getLinks(response).refs.find(link => link.rel === 'next'); + dispatch(importFetchedStatuses(response.data)); + dispatch(fetchBookmarkedStatusesSuccess(response.data, next ? next.uri : null)); + }).catch(error => { + dispatch(fetchBookmarkedStatusesFail(error)); + }); + }; +}; + +export function fetchBookmarkedStatusesRequest() { + return { + type: BOOKMARKED_STATUSES_FETCH_REQUEST, + }; +}; + +export function fetchBookmarkedStatusesSuccess(statuses, next) { + return { + type: BOOKMARKED_STATUSES_FETCH_SUCCESS, + statuses, + next, + }; +}; + +export function fetchBookmarkedStatusesFail(error) { + return { + type: BOOKMARKED_STATUSES_FETCH_FAIL, + error, + }; +}; + +export function expandBookmarkedStatuses() { + return (dispatch, getState) => { + const url = getState().getIn(['status_lists', 'bookmarks', 'next'], null); + + if (url === null || getState().getIn(['status_lists', 'bookmarks', 'isLoading'])) { + return; + } + + dispatch(expandBookmarkedStatusesRequest()); + + api(getState).get(url).then(response => { + const next = getLinks(response).refs.find(link => link.rel === 'next'); + dispatch(importFetchedStatuses(response.data)); + dispatch(expandBookmarkedStatusesSuccess(response.data, next ? next.uri : null)); + }).catch(error => { + dispatch(expandBookmarkedStatusesFail(error)); + }); + }; +}; + +export function expandBookmarkedStatusesRequest() { + return { + type: BOOKMARKED_STATUSES_EXPAND_REQUEST, + }; +}; + +export function expandBookmarkedStatusesSuccess(statuses, next) { + return { + type: BOOKMARKED_STATUSES_EXPAND_SUCCESS, + statuses, + next, + }; +}; + +export function expandBookmarkedStatusesFail(error) { + return { + type: BOOKMARKED_STATUSES_EXPAND_FAIL, + error, + }; +}; diff --git a/app/javascript/mastodon/actions/interactions.js b/app/javascript/mastodon/actions/interactions.js index 2dc4c574c..28c6b1a62 100644 --- a/app/javascript/mastodon/actions/interactions.js +++ b/app/javascript/mastodon/actions/interactions.js @@ -33,6 +33,14 @@ export const UNPIN_REQUEST = 'UNPIN_REQUEST'; export const UNPIN_SUCCESS = 'UNPIN_SUCCESS'; export const UNPIN_FAIL = 'UNPIN_FAIL'; +export const BOOKMARK_REQUEST = 'BOOKMARK_REQUEST'; +export const BOOKMARK_SUCCESS = 'BOOKMARKED_SUCCESS'; +export const BOOKMARK_FAIL = 'BOOKMARKED_FAIL'; + +export const UNBOOKMARK_REQUEST = 'UNBOOKMARKED_REQUEST'; +export const UNBOOKMARK_SUCCESS = 'UNBOOKMARKED_SUCCESS'; +export const UNBOOKMARK_FAIL = 'UNBOOKMARKED_FAIL'; + export function reblog(status) { return function (dispatch, getState) { dispatch(reblogRequest(status)); @@ -187,6 +195,78 @@ export function unfavouriteFail(status, error) { }; }; +export function bookmark(status) { + return function (dispatch, getState) { + dispatch(bookmarkRequest(status)); + + api(getState).post(`/api/v1/statuses/${status.get('id')}/bookmark`).then(function (response) { + dispatch(importFetchedStatus(response.data)); + dispatch(bookmarkSuccess(status, response.data)); + }).catch(function (error) { + dispatch(bookmarkFail(status, error)); + }); + }; +}; + +export function unbookmark(status) { + return (dispatch, getState) => { + dispatch(unbookmarkRequest(status)); + + api(getState).post(`/api/v1/statuses/${status.get('id')}/unbookmark`).then(response => { + dispatch(importFetchedStatus(response.data)); + dispatch(unbookmarkSuccess(status, response.data)); + }).catch(error => { + dispatch(unbookmarkFail(status, error)); + }); + }; +}; + +export function bookmarkRequest(status) { + return { + type: BOOKMARK_REQUEST, + status: status, + }; +}; + +export function bookmarkSuccess(status, response) { + return { + type: BOOKMARK_SUCCESS, + status: status, + response: response, + }; +}; + +export function bookmarkFail(status, error) { + return { + type: BOOKMARK_FAIL, + status: status, + error: error, + }; +}; + +export function unbookmarkRequest(status) { + return { + type: UNBOOKMARK_REQUEST, + status: status, + }; +}; + +export function unbookmarkSuccess(status, response) { + return { + type: UNBOOKMARK_SUCCESS, + status: status, + response: response, + }; +}; + +export function unbookmarkFail(status, error) { + return { + type: UNBOOKMARK_FAIL, + status: status, + error: error, + }; +}; + export function fetchReblogs(id) { return (dispatch, getState) => { dispatch(fetchReblogsRequest(id)); diff --git a/app/javascript/mastodon/components/status_action_bar.js b/app/javascript/mastodon/components/status_action_bar.js index 0bfbd8879..4fa2c1158 100644 --- a/app/javascript/mastodon/components/status_action_bar.js +++ b/app/javascript/mastodon/components/status_action_bar.js @@ -23,6 +23,7 @@ const messages = defineMessages({ cancel_reblog_private: { id: 'status.cancel_reblog_private', defaultMessage: 'Unboost' }, cannot_reblog: { id: 'status.cannot_reblog', defaultMessage: 'This post cannot be boosted' }, favourite: { id: 'status.favourite', defaultMessage: 'Favourite' }, + bookmark: { id: 'status.bookmark', defaultMessage: 'Bookmark' }, open: { id: 'status.open', defaultMessage: 'Expand this status' }, report: { id: 'status.report', defaultMessage: 'Report @{name}' }, muteConversation: { id: 'status.mute_conversation', defaultMessage: 'Mute conversation' }, @@ -66,6 +67,7 @@ class StatusActionBar extends ImmutablePureComponent { onEmbed: PropTypes.func, onMuteConversation: PropTypes.func, onPin: PropTypes.func, + onBookmark: PropTypes.func, withDismiss: PropTypes.bool, intl: PropTypes.object.isRequired, }; @@ -114,6 +116,10 @@ class StatusActionBar extends ImmutablePureComponent { window.open(`/interact/${this.props.status.get('id')}?type=${type}`, 'mastodon-intent', 'width=445,height=600,resizable=no,menubar=no,status=no,scrollbars=yes'); } + handleBookmarkClick = () => { + this.props.onBookmark(this.props.status); + } + handleDeleteClick = () => { this.props.onDelete(this.props.status, this.context.router.history); } @@ -253,6 +259,7 @@ class StatusActionBar extends ImmutablePureComponent { {shareButton} +
diff --git a/app/javascript/mastodon/containers/status_container.js b/app/javascript/mastodon/containers/status_container.js index fb22676e0..16ba02e12 100644 --- a/app/javascript/mastodon/containers/status_container.js +++ b/app/javascript/mastodon/containers/status_container.js @@ -9,8 +9,10 @@ import { import { reblog, favourite, + bookmark, unreblog, unfavourite, + unbookmark, pin, unpin, } from '../actions/interactions'; @@ -90,6 +92,14 @@ const mapDispatchToProps = (dispatch, { intl }) => ({ } }, + onBookmark (status) { + if (status.get('bookmarked')) { + dispatch(unbookmark(status)); + } else { + dispatch(bookmark(status)); + } + }, + onPin (status) { if (status.get('pinned')) { dispatch(unpin(status)); diff --git a/app/javascript/mastodon/features/bookmarked_statuses/index.js b/app/javascript/mastodon/features/bookmarked_statuses/index.js new file mode 100644 index 000000000..c37cb9176 --- /dev/null +++ b/app/javascript/mastodon/features/bookmarked_statuses/index.js @@ -0,0 +1,104 @@ +import React from 'react'; +import { connect } from 'react-redux'; +import PropTypes from 'prop-types'; +import ImmutablePropTypes from 'react-immutable-proptypes'; +import { fetchBookmarkedStatuses, expandBookmarkedStatuses } from '../../actions/bookmarks'; +import Column from '../ui/components/column'; +import ColumnHeader from '../../components/column_header'; +import { addColumn, removeColumn, moveColumn } from '../../actions/columns'; +import StatusList from '../../components/status_list'; +import { defineMessages, injectIntl, FormattedMessage } from 'react-intl'; +import ImmutablePureComponent from 'react-immutable-pure-component'; +import { debounce } from 'lodash'; + +const messages = defineMessages({ + heading: { id: 'column.bookmarks', defaultMessage: 'Bookmarks' }, +}); + +const mapStateToProps = state => ({ + statusIds: state.getIn(['status_lists', 'bookmarks', 'items']), + isLoading: state.getIn(['status_lists', 'bookmarks', 'isLoading'], true), + hasMore: !!state.getIn(['status_lists', 'bookmarks', 'next']), +}); + +export default @connect(mapStateToProps) +@injectIntl +class Bookmarks extends ImmutablePureComponent { + + static propTypes = { + dispatch: PropTypes.func.isRequired, + shouldUpdateScroll: PropTypes.func, + statusIds: ImmutablePropTypes.list.isRequired, + intl: PropTypes.object.isRequired, + columnId: PropTypes.string, + multiColumn: PropTypes.bool, + hasMore: PropTypes.bool, + isLoading: PropTypes.bool, + }; + + componentWillMount () { + this.props.dispatch(fetchBookmarkedStatuses()); + } + + handlePin = () => { + const { columnId, dispatch } = this.props; + + if (columnId) { + dispatch(removeColumn(columnId)); + } else { + dispatch(addColumn('BOOKMARKS', {})); + } + } + + handleMove = (dir) => { + const { columnId, dispatch } = this.props; + dispatch(moveColumn(columnId, dir)); + } + + handleHeaderClick = () => { + this.column.scrollTop(); + } + + setRef = c => { + this.column = c; + } + + handleLoadMore = debounce(() => { + this.props.dispatch(expandBookmarkedStatuses()); + }, 300, { leading: true }) + + render () { + const { intl, shouldUpdateScroll, statusIds, columnId, multiColumn, hasMore, isLoading } = this.props; + const pinned = !!columnId; + + const emptyMessage = ; + + return ( + + + + + + ); + } + +} diff --git a/app/javascript/mastodon/features/getting_started/index.js b/app/javascript/mastodon/features/getting_started/index.js index 67ec7665b..adbc147d1 100644 --- a/app/javascript/mastodon/features/getting_started/index.js +++ b/app/javascript/mastodon/features/getting_started/index.js @@ -22,6 +22,7 @@ const messages = defineMessages({ settings_subheading: { id: 'column_subheading.settings', defaultMessage: 'Settings' }, community_timeline: { id: 'navigation_bar.community_timeline', defaultMessage: 'Local timeline' }, direct: { id: 'navigation_bar.direct', defaultMessage: 'Direct messages' }, + bookmarks: { id: 'navigation_bar.bookmarks', defaultMessage: 'Bookmarks' }, preferences: { id: 'navigation_bar.preferences', defaultMessage: 'Preferences' }, follow_requests: { id: 'navigation_bar.follow_requests', defaultMessage: 'Follow requests' }, favourites: { id: 'navigation_bar.favourites', defaultMessage: 'Favourites' }, @@ -126,11 +127,12 @@ class GettingStarted extends ImmutablePureComponent { navItems.push( , + , , ); - height += 48*3; + height += 48*4; if (myAccount.get('locked') || unreadFollowRequests > 0) { navItems.push(); diff --git a/app/javascript/mastodon/features/status/components/action_bar.js b/app/javascript/mastodon/features/status/components/action_bar.js index 3e511b7a6..1b81cd245 100644 --- a/app/javascript/mastodon/features/status/components/action_bar.js +++ b/app/javascript/mastodon/features/status/components/action_bar.js @@ -17,6 +17,7 @@ const messages = defineMessages({ cancel_reblog_private: { id: 'status.cancel_reblog_private', defaultMessage: 'Unboost' }, cannot_reblog: { id: 'status.cannot_reblog', defaultMessage: 'This post cannot be boosted' }, favourite: { id: 'status.favourite', defaultMessage: 'Favourite' }, + bookmark: { id: 'status.bookmark', defaultMessage: 'Bookmark' }, mute: { id: 'status.mute', defaultMessage: 'Mute @{name}' }, muteConversation: { id: 'status.mute_conversation', defaultMessage: 'Mute conversation' }, unmuteConversation: { id: 'status.unmute_conversation', defaultMessage: 'Unmute conversation' }, @@ -43,6 +44,7 @@ class ActionBar extends React.PureComponent { onReply: PropTypes.func.isRequired, onReblog: PropTypes.func.isRequired, onFavourite: PropTypes.func.isRequired, + onBookmark: PropTypes.func.isRequired, onDelete: PropTypes.func.isRequired, onDirect: PropTypes.func.isRequired, onMention: PropTypes.func.isRequired, @@ -67,6 +69,10 @@ class ActionBar extends React.PureComponent { this.props.onFavourite(this.props.status); } + handleBookmarkClick = (e) => { + this.props.onBookmark(this.props.status, e); + } + handleDeleteClick = () => { this.props.onDelete(this.props.status, this.context.router.history); } @@ -198,6 +204,7 @@ class ActionBar extends React.PureComponent {
{shareButton} +
diff --git a/app/javascript/mastodon/features/status/index.js b/app/javascript/mastodon/features/status/index.js index 029057d40..9fb3fe305 100644 --- a/app/javascript/mastodon/features/status/index.js +++ b/app/javascript/mastodon/features/status/index.js @@ -13,6 +13,8 @@ import Column from '../ui/components/column'; import { favourite, unfavourite, + bookmark, + unbookmark, reblog, unreblog, pin, @@ -232,6 +234,14 @@ class Status extends ImmutablePureComponent { } } + handleBookmarkClick = (status) => { + if (status.get('bookmarked')) { + this.props.dispatch(unbookmark(status)); + } else { + this.props.dispatch(bookmark(status)); + } + } + handleDeleteClick = (status, history, withRedraft = false) => { const { dispatch, intl } = this.props; @@ -499,6 +509,7 @@ class Status extends ImmutablePureComponent { onReply={this.handleReplyClick} onFavourite={this.handleFavouriteClick} onReblog={this.handleReblogClick} + onBookmark={this.handleBookmarkClick} onDelete={this.handleDeleteClick} onDirect={this.handleDirectClick} onMention={this.handleMentionClick} diff --git a/app/javascript/mastodon/features/ui/components/columns_area.js b/app/javascript/mastodon/features/ui/components/columns_area.js index f31425c70..8bc4bfc0e 100644 --- a/app/javascript/mastodon/features/ui/components/columns_area.js +++ b/app/javascript/mastodon/features/ui/components/columns_area.js @@ -21,6 +21,7 @@ import { HashtagTimeline, DirectTimeline, FavouritedStatuses, + BookmarkedStatuses, ListTimeline, Directory, } from '../../ui/util/async-components'; @@ -40,6 +41,7 @@ const componentMap = { 'HASHTAG': HashtagTimeline, 'DIRECT': DirectTimeline, 'FAVOURITES': FavouritedStatuses, + 'BOOKMARKS': BookmarkedStatuses, 'LIST': ListTimeline, 'DIRECTORY': Directory, }; diff --git a/app/javascript/mastodon/features/ui/components/navigation_panel.js b/app/javascript/mastodon/features/ui/components/navigation_panel.js index 51e3ec037..0c12852f5 100644 --- a/app/javascript/mastodon/features/ui/components/navigation_panel.js +++ b/app/javascript/mastodon/features/ui/components/navigation_panel.js @@ -17,6 +17,7 @@ const NavigationPanel = () => ( + {profile_directory && } diff --git a/app/javascript/mastodon/features/ui/index.js b/app/javascript/mastodon/features/ui/index.js index a45ba91eb..b0e38c5cb 100644 --- a/app/javascript/mastodon/features/ui/index.js +++ b/app/javascript/mastodon/features/ui/index.js @@ -41,6 +41,7 @@ import { FollowRequests, GenericNotFound, FavouritedStatuses, + BookmarkedStatuses, ListTimeline, Blocks, DomainBlocks, @@ -190,6 +191,7 @@ class SwitchingColumnsArea extends React.PureComponent { + diff --git a/app/javascript/mastodon/features/ui/util/async-components.js b/app/javascript/mastodon/features/ui/util/async-components.js index bb0fcb859..986efda1e 100644 --- a/app/javascript/mastodon/features/ui/util/async-components.js +++ b/app/javascript/mastodon/features/ui/util/async-components.js @@ -90,6 +90,10 @@ export function FavouritedStatuses () { return import(/* webpackChunkName: "features/favourited_statuses" */'../../favourited_statuses'); } +export function BookmarkedStatuses () { + return import(/* webpackChunkName: "features/bookmarked_statuses" */'../../bookmarked_statuses'); +} + export function Blocks () { return import(/* webpackChunkName: "features/blocks" */'../../blocks'); } diff --git a/app/javascript/mastodon/reducers/status_lists.js b/app/javascript/mastodon/reducers/status_lists.js index 6c5f33557..9f8f28dee 100644 --- a/app/javascript/mastodon/reducers/status_lists.js +++ b/app/javascript/mastodon/reducers/status_lists.js @@ -6,6 +6,14 @@ import { FAVOURITED_STATUSES_EXPAND_SUCCESS, FAVOURITED_STATUSES_EXPAND_FAIL, } from '../actions/favourites'; +import { + BOOKMARKED_STATUSES_FETCH_REQUEST, + BOOKMARKED_STATUSES_FETCH_SUCCESS, + BOOKMARKED_STATUSES_FETCH_FAIL, + BOOKMARKED_STATUSES_EXPAND_REQUEST, + BOOKMARKED_STATUSES_EXPAND_SUCCESS, + BOOKMARKED_STATUSES_EXPAND_FAIL, +} from '../actions/bookmarks'; import { PINNED_STATUSES_FETCH_SUCCESS, } from '../actions/pin_statuses'; @@ -13,6 +21,8 @@ import { Map as ImmutableMap, List as ImmutableList } from 'immutable'; import { FAVOURITE_SUCCESS, UNFAVOURITE_SUCCESS, + BOOKMARK_SUCCESS, + UNBOOKMARK_SUCCESS, PIN_SUCCESS, UNPIN_SUCCESS, } from '../actions/interactions'; @@ -23,6 +33,11 @@ const initialState = ImmutableMap({ loaded: false, items: ImmutableList(), }), + bookmarks: ImmutableMap({ + next: null, + loaded: false, + items: ImmutableList(), + }), pins: ImmutableMap({ next: null, loaded: false, @@ -71,10 +86,24 @@ export default function statusLists(state = initialState, action) { return normalizeList(state, 'favourites', action.statuses, action.next); case FAVOURITED_STATUSES_EXPAND_SUCCESS: return appendToList(state, 'favourites', action.statuses, action.next); + case BOOKMARKED_STATUSES_FETCH_REQUEST: + case BOOKMARKED_STATUSES_EXPAND_REQUEST: + return state.setIn(['bookmarks', 'isLoading'], true); + case BOOKMARKED_STATUSES_FETCH_FAIL: + case BOOKMARKED_STATUSES_EXPAND_FAIL: + return state.setIn(['bookmarks', 'isLoading'], false); + case BOOKMARKED_STATUSES_FETCH_SUCCESS: + return normalizeList(state, 'bookmarks', action.statuses, action.next); + case BOOKMARKED_STATUSES_EXPAND_SUCCESS: + return appendToList(state, 'bookmarks', action.statuses, action.next); case FAVOURITE_SUCCESS: return prependOneToList(state, 'favourites', action.status); case UNFAVOURITE_SUCCESS: return removeOneFromList(state, 'favourites', action.status); + case BOOKMARK_SUCCESS: + return prependOneToList(state, 'bookmarks', action.status); + case UNBOOKMARK_SUCCESS: + return removeOneFromList(state, 'bookmarks', action.status); case PINNED_STATUSES_FETCH_SUCCESS: return normalizeList(state, 'pins', action.statuses, action.next); case PIN_SUCCESS: diff --git a/app/javascript/mastodon/reducers/statuses.js b/app/javascript/mastodon/reducers/statuses.js index 372673bc0..772f98bcb 100644 --- a/app/javascript/mastodon/reducers/statuses.js +++ b/app/javascript/mastodon/reducers/statuses.js @@ -4,6 +4,8 @@ import { FAVOURITE_REQUEST, FAVOURITE_FAIL, UNFAVOURITE_SUCCESS, + BOOKMARK_REQUEST, + BOOKMARK_FAIL, } from '../actions/interactions'; import { STATUS_MUTE_SUCCESS, @@ -43,6 +45,10 @@ export default function statuses(state = initialState, action) { return state.setIn([action.status.get('id'), 'favourites_count'], favouritesCount - 1); case FAVOURITE_FAIL: return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'favourited'], false); + case BOOKMARK_REQUEST: + return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'bookmarked'], true); + case BOOKMARK_FAIL: + return state.get(action.status.get('id')) === undefined ? state : state.setIn([action.status.get('id'), 'bookmarked'], false); case REBLOG_REQUEST: return state.setIn([action.status.get('id'), 'reblogged'], true); case REBLOG_FAIL: diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index ded668aea..98ffe1c20 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -1592,6 +1592,10 @@ a.account__display-name { color: $gold-star; } +.bookmark-icon.active { + color: $red-bookmark; +} + .no-reduce-motion .icon-button.star-icon { &.activate { & > .fa-star { diff --git a/app/javascript/styles/mastodon/variables.scss b/app/javascript/styles/mastodon/variables.scss index a82c44229..8602c3dde 100644 --- a/app/javascript/styles/mastodon/variables.scss +++ b/app/javascript/styles/mastodon/variables.scss @@ -6,6 +6,8 @@ $error-red: #df405a !default; // Cerise $warning-red: #ff5050 !default; // Sunset Orange $gold-star: #ca8f04 !default; // Dark Goldenrod +$red-bookmark: $warning-red; + // Values from the classic Mastodon UI $classic-base-color: #282c37; // Midnight Express $classic-primary-color: #9baec8; // Echo Blue diff --git a/app/models/bookmark.rb b/app/models/bookmark.rb new file mode 100644 index 000000000..01dc48ee7 --- /dev/null +++ b/app/models/bookmark.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: bookmarks +# +# id :integer not null, primary key +# created_at :datetime not null +# updated_at :datetime not null +# account_id :integer not null +# status_id :integer not null +# + +class Bookmark < ApplicationRecord + include Paginable + + update_index('statuses#status', :status) if Chewy.enabled? + + belongs_to :account, inverse_of: :bookmarks + belongs_to :status, inverse_of: :bookmarks + + validates :status_id, uniqueness: { scope: :account_id } + + before_validation do + self.status = status.reblog if status&.reblog? + end +end diff --git a/app/models/concerns/account_associations.rb b/app/models/concerns/account_associations.rb index c9cc5c610..499edbf4e 100644 --- a/app/models/concerns/account_associations.rb +++ b/app/models/concerns/account_associations.rb @@ -13,6 +13,7 @@ module AccountAssociations # Timelines has_many :statuses, inverse_of: :account, dependent: :destroy has_many :favourites, inverse_of: :account, dependent: :destroy + has_many :bookmarks, inverse_of: :account, dependent: :destroy has_many :mentions, inverse_of: :account, dependent: :destroy has_many :notifications, inverse_of: :account, dependent: :destroy has_many :conversations, class_name: 'AccountConversation', dependent: :destroy, inverse_of: :account diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index ad2909d91..f27d39483 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -186,6 +186,10 @@ module AccountInteractions status.proper.favourites.where(account: self).exists? end + def bookmarked?(status) + status.proper.bookmarks.where(account: self).exists? + end + def reblogged?(status) status.proper.reblogs.where(account: self).exists? end diff --git a/app/models/status.rb b/app/models/status.rb index 0c01a5389..1cb381400 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -54,6 +54,7 @@ class Status < ApplicationRecord belongs_to :reblog, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblogs, optional: true has_many :favourites, inverse_of: :status, dependent: :destroy + has_many :bookmarks, inverse_of: :status, dependent: :destroy has_many :reblogs, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblog, dependent: :destroy has_many :replies, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :thread has_many :mentions, dependent: :destroy, inverse_of: :status @@ -302,6 +303,10 @@ class Status < ApplicationRecord Favourite.select('status_id').where(status_id: status_ids).where(account_id: account_id).each_with_object({}) { |f, h| h[f.status_id] = true } end + def bookmarks_map(status_ids, account_id) + Bookmark.select('status_id').where(status_id: status_ids).where(account_id: account_id).map { |f| [f.status_id, true] }.to_h + end + def reblogs_map(status_ids, account_id) unscoped.select('reblog_of_id').where(reblog_of_id: status_ids).where(account_id: account_id).each_with_object({}) { |s, h| h[s.reblog_of_id] = true } end diff --git a/app/presenters/status_relationships_presenter.rb b/app/presenters/status_relationships_presenter.rb index b04e10e2f..64e688d87 100644 --- a/app/presenters/status_relationships_presenter.rb +++ b/app/presenters/status_relationships_presenter.rb @@ -7,6 +7,7 @@ class StatusRelationshipsPresenter if current_account_id.nil? @reblogs_map = {} @favourites_map = {} + @bookmarks_map = {} @mutes_map = {} @pins_map = {} else @@ -17,6 +18,7 @@ class StatusRelationshipsPresenter @reblogs_map = Status.reblogs_map(status_ids, current_account_id).merge(options[:reblogs_map] || {}) @favourites_map = Status.favourites_map(status_ids, current_account_id).merge(options[:favourites_map] || {}) + @bookmarks_map = Status.bookmarks_map(status_ids, current_account_id).merge(options[:bookmarks_map] || {}) @mutes_map = Status.mutes_map(conversation_ids, current_account_id).merge(options[:mutes_map] || {}) @pins_map = Status.pins_map(pinnable_status_ids, current_account_id).merge(options[:pins_map] || {}) end diff --git a/app/serializers/rest/status_serializer.rb b/app/serializers/rest/status_serializer.rb index 2dc4a1b61..08bc4d82a 100644 --- a/app/serializers/rest/status_serializer.rb +++ b/app/serializers/rest/status_serializer.rb @@ -9,6 +9,7 @@ class REST::StatusSerializer < ActiveModel::Serializer attribute :favourited, if: :current_user? attribute :reblogged, if: :current_user? attribute :muted, if: :current_user? + attribute :bookmarked, if: :current_user? attribute :pinned, if: :pinnable? attribute :content, unless: :source_requested? @@ -93,6 +94,14 @@ class REST::StatusSerializer < ActiveModel::Serializer end end + def bookmarked + if instance_options && instance_options[:bookmarks] + instance_options[:bookmarks].bookmarks_map[object.id] || false + else + current_user.account.bookmarked?(object) + end + end + def pinned if instance_options && instance_options[:relationships] instance_options[:relationships].pins_map[object.id] || false diff --git a/app/services/backup_service.rb b/app/services/backup_service.rb index 12e4fa8b4..fe26d7aa0 100644 --- a/app/services/backup_service.rb +++ b/app/services/backup_service.rb @@ -45,6 +45,7 @@ class BackupService < BaseService dump_media_attachments!(tar) dump_outbox!(tar) dump_likes!(tar) + dump_bookmarks!(tar) dump_actor!(tar) end end @@ -85,6 +86,7 @@ class BackupService < BaseService actor[:image][:url] = 'header' + File.extname(actor[:image][:url]) if actor[:image] actor[:outbox] = 'outbox.json' actor[:likes] = 'likes.json' + actor[:bookmarks] = 'bookmarks.json' download_to_tar(tar, account.avatar, 'avatar' + File.extname(account.avatar.path)) if account.avatar.exists? download_to_tar(tar, account.header, 'header' + File.extname(account.header.path)) if account.header.exists? @@ -115,6 +117,25 @@ class BackupService < BaseService end end + def dump_bookmarks!(tar) + collection = serialize(ActivityPub::CollectionPresenter.new(id: 'bookmarks.json', type: :ordered, size: 0, items: []), ActivityPub::CollectionSerializer) + + Status.reorder(nil).joins(:bookmarks).includes(:account).merge(account.bookmarks).find_in_batches do |statuses| + statuses.each do |status| + collection[:totalItems] += 1 + collection[:orderedItems] << ActivityPub::TagManager.instance.uri_for(status) + end + + GC.start + end + + json = Oj.dump(collection) + + tar.add_file_simple('bookmarks.json', 0o444, json.bytesize) do |io| + io.write(json) + end + end + def collection_presenter ActivityPub::CollectionPresenter.new( id: 'outbox.json', diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 914b3c001..a5c9caa4a 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -58,6 +58,7 @@ Doorkeeper.configure do optional_scopes :write, :'write:accounts', :'write:blocks', + :'write:bookmarks', :'write:conversations', :'write:favourites', :'write:filters', @@ -71,6 +72,7 @@ Doorkeeper.configure do :read, :'read:accounts', :'read:blocks', + :'read:bookmarks', :'read:favourites', :'read:filters', :'read:follows', diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml index d9b7c2c8e..4e9c83a8f 100644 --- a/config/locales/doorkeeper.en.yml +++ b/config/locales/doorkeeper.en.yml @@ -125,6 +125,7 @@ en: read: read all your account's data read:accounts: see accounts information read:blocks: see your blocks + read:bookmarks: see your bookmarks read:favourites: see your favourites read:filters: see your filters read:follows: see your follows @@ -137,6 +138,7 @@ en: write: modify all your account's data write:accounts: modify your profile write:blocks: block accounts and domains + write:bookmarks: bookmark statuses write:favourites: favourite statuses write:filters: create filters write:follows: follow people diff --git a/config/routes.rb b/config/routes.rb index e43e201a5..5411cff58 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -289,6 +289,9 @@ Rails.application.routes.draw do resource :favourite, only: :create post :unfavourite, to: 'favourites#destroy' + resource :bookmark, only: :create + post :unbookmark, to: 'bookmarks#destroy' + resource :mute, only: :create post :unmute, to: 'mutes#destroy' @@ -324,6 +327,7 @@ Rails.application.routes.draw do resources :blocks, only: [:index] resources :mutes, only: [:index] resources :favourites, only: [:index] + resources :bookmarks, only: [:index] resources :reports, only: [:create] resources :trends, only: [:index] resources :filters, only: [:index, :create, :show, :update, :destroy] diff --git a/db/migrate/20180831171112_create_bookmarks.rb b/db/migrate/20180831171112_create_bookmarks.rb new file mode 100644 index 000000000..27c7339c9 --- /dev/null +++ b/db/migrate/20180831171112_create_bookmarks.rb @@ -0,0 +1,17 @@ +class CreateBookmarks < ActiveRecord::Migration[5.1] + def change + create_table :bookmarks do |t| + t.references :account, null: false + t.references :status, null: false + + t.timestamps + end + + safety_assured do + add_foreign_key :bookmarks, :accounts, column: :account_id, on_delete: :cascade + add_foreign_key :bookmarks, :statuses, column: :status_id, on_delete: :cascade + end + + add_index :bookmarks, [:account_id, :status_id], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 1bcc003a5..84c76e4ce 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -217,6 +217,16 @@ ActiveRecord::Schema.define(version: 2019_10_31_163205) do t.index ["target_account_id"], name: "index_blocks_on_target_account_id" end + create_table "bookmarks", force: :cascade do |t| + t.bigint "account_id", null: false + t.bigint "status_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["account_id", "status_id"], name: "index_bookmarks_on_account_id_and_status_id", unique: true + t.index ["account_id"], name: "index_bookmarks_on_account_id" + t.index ["status_id"], name: "index_bookmarks_on_status_id" + end + create_table "conversation_mutes", force: :cascade do |t| t.bigint "conversation_id", null: false t.bigint "account_id", null: false @@ -834,6 +844,8 @@ ActiveRecord::Schema.define(version: 2019_10_31_163205) do add_foreign_key "backups", "users", on_delete: :nullify add_foreign_key "blocks", "accounts", column: "target_account_id", name: "fk_9571bfabc1", on_delete: :cascade add_foreign_key "blocks", "accounts", name: "fk_4269e03e65", on_delete: :cascade + add_foreign_key "bookmarks", "accounts", on_delete: :cascade + add_foreign_key "bookmarks", "statuses", on_delete: :cascade add_foreign_key "conversation_mutes", "accounts", name: "fk_225b4212bb", on_delete: :cascade add_foreign_key "conversation_mutes", "conversations", on_delete: :cascade add_foreign_key "custom_filters", "accounts", on_delete: :cascade diff --git a/spec/controllers/api/v1/bookmarks_controller_spec.rb b/spec/controllers/api/v1/bookmarks_controller_spec.rb new file mode 100644 index 000000000..79601b6e6 --- /dev/null +++ b/spec/controllers/api/v1/bookmarks_controller_spec.rb @@ -0,0 +1,78 @@ +require 'rails_helper' + +RSpec.describe Api::V1::BookmarksController, type: :controller do + render_views + + let(:user) { Fabricate(:user) } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:bookmarks') } + + describe 'GET #index' do + context 'without token' do + it 'returns http unauthorized' do + get :index + expect(response).to have_http_status :unauthorized + end + end + + context 'with token' do + context 'without read scope' do + before do + allow(controller).to receive(:doorkeeper_token) do + Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: '') + end + end + + it 'returns http forbidden' do + get :index + expect(response).to have_http_status :forbidden + end + end + + context 'without valid resource owner' do + before do + token = Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read') + user.destroy! + + allow(controller).to receive(:doorkeeper_token) { token } + end + + it 'returns http unprocessable entity' do + get :index + expect(response).to have_http_status :unprocessable_entity + end + end + + context 'with read scope and valid resource owner' do + before do + allow(controller).to receive(:doorkeeper_token) do + Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read') + end + end + + it 'shows bookmarks owned by the user' do + bookmarked_by_user = Fabricate(:bookmark, account: user.account) + bookmarked_by_others = Fabricate(:bookmark) + + get :index + + expect(assigns(:statuses)).to match_array [bookmarked_by_user.status] + end + + it 'adds pagination headers if necessary' do + bookmark = Fabricate(:bookmark, account: user.account) + + get :index, params: { limit: 1 } + + expect(response.headers['Link'].find_link(['rel', 'next']).href).to eq "http://test.host/api/v1/bookmarks?limit=1&max_id=#{bookmark.id}" + expect(response.headers['Link'].find_link(['rel', 'prev']).href).to eq "http://test.host/api/v1/bookmarks?limit=1&since_id=#{bookmark.id}" + end + + it 'does not add pagination headers if not necessary' do + get :index + + expect(response.headers['Link']).to eq nil + end + end + end + end +end diff --git a/spec/controllers/api/v1/statuses/bookmarks_controller_spec.rb b/spec/controllers/api/v1/statuses/bookmarks_controller_spec.rb new file mode 100644 index 000000000..b79853718 --- /dev/null +++ b/spec/controllers/api/v1/statuses/bookmarks_controller_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Api::V1::Statuses::BookmarksController do + render_views + + let(:user) { Fabricate(:user, account: Fabricate(:account, username: 'alice')) } + let(:app) { Fabricate(:application, name: 'Test app', website: 'http://testapp.com') } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'write:bookmarks', application: app) } + + context 'with an oauth token' do + before do + allow(controller).to receive(:doorkeeper_token) { token } + end + + describe 'POST #create' do + let(:status) { Fabricate(:status, account: user.account) } + + before do + post :create, params: { status_id: status.id } + end + + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'updates the bookmarked attribute' do + expect(user.account.bookmarked?(status)).to be true + end + + it 'return json with updated attributes' do + hash_body = body_as_json + + expect(hash_body[:id]).to eq status.id.to_s + expect(hash_body[:bookmarked]).to be true + end + end + + describe 'POST #destroy' do + let(:status) { Fabricate(:status, account: user.account) } + + before do + Bookmark.find_or_create_by!(account: user.account, status: status) + post :destroy, params: { status_id: status.id } + end + + it 'returns http success' do + expect(response).to have_http_status(:success) + end + + it 'updates the bookmarked attribute' do + expect(user.account.bookmarked?(status)).to be false + end + end + end +end diff --git a/spec/fabricators/bookmark_fabricator.rb b/spec/fabricators/bookmark_fabricator.rb new file mode 100644 index 000000000..12cbc5bfa --- /dev/null +++ b/spec/fabricators/bookmark_fabricator.rb @@ -0,0 +1,4 @@ +Fabricator(:bookmark) do + account + status +end -- cgit From d14e74eff52352f1a2fb4bc2053bbb28c1aa29e0 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 17 Nov 2019 18:40:33 +0100 Subject: Add cache for OEmbed endpoints to avoid extra HTTP requests (#12403) * add youtube oembed endpoint * add check for oembed endpoint * change unless for a more readable if * clear blank lines * endpoint via https * Fix string literal in condition * use cache for endpoints * use cache for endpoints * clean up and adding check * clean up and remove redundant return * add html check * add false to return * use double quotes * use double quotes * Clean up --- app/services/fetch_link_card_service.rb | 23 +++++++++++++++------- app/services/fetch_oembed_service.rb | 31 +++++++++++++++++++++++++++++- spec/services/fetch_oembed_service_spec.rb | 18 +++++++++++++++++ 3 files changed, 64 insertions(+), 8 deletions(-) (limited to 'app/services') diff --git a/app/services/fetch_link_card_service.rb b/app/services/fetch_link_card_service.rb index 640c60fd4..29880e8d8 100644 --- a/app/services/fetch_link_card_service.rb +++ b/app/services/fetch_link_card_service.rb @@ -39,6 +39,12 @@ class FetchLinkCardService < BaseService def process_url @card ||= PreviewCard.new(url: @url) + attempt_oembed || attempt_opengraph + end + + def html + return @html if defined?(@html) + Request.new(:get, @url).perform do |res| if res.code == 200 && res.mime_type == 'text/html' @html = res.body_with_limit @@ -48,10 +54,6 @@ class FetchLinkCardService < BaseService @html_charset = nil end end - - return if @html.nil? - - attempt_oembed || attempt_opengraph end def attach_card @@ -88,12 +90,17 @@ class FetchLinkCardService < BaseService end def attempt_oembed - service = FetchOEmbedService.new - embed = service.call(@url, html: @html) - url = Addressable::URI.parse(service.endpoint_url) + service = FetchOEmbedService.new + url_domain = Addressable::URI.parse(@url).normalized_host + cached_endpoint = Rails.cache.read("oembed_endpoint:#{url_domain}") + + embed = service.call(@url, cached_endpoint: cached_endpoint) unless cached_endpoint.nil? + embed ||= service.call(@url, html: html) unless html.nil? return false if embed.nil? + url = Addressable::URI.parse(service.endpoint_url) + @card.type = embed[:type] @card.title = embed[:title] || '' @card.author_name = embed[:author_name] || '' @@ -127,6 +134,8 @@ class FetchLinkCardService < BaseService end def attempt_opengraph + return if html.nil? + detector = CharlockHolmes::EncodingDetector.new detector.strip_tags = true diff --git a/app/services/fetch_oembed_service.rb b/app/services/fetch_oembed_service.rb index 10176cfb9..4f8498c62 100644 --- a/app/services/fetch_oembed_service.rb +++ b/app/services/fetch_oembed_service.rb @@ -1,13 +1,20 @@ # frozen_string_literal: true class FetchOEmbedService + ENDPOINT_CACHE_EXPIRES_IN = 24.hours.freeze + attr_reader :url, :options, :format, :endpoint_url def call(url, options = {}) @url = url @options = options - discover_endpoint! + if @options[:cached_endpoint] + parse_cached_endpoint! + else + discover_endpoint! + end + fetch! end @@ -32,10 +39,32 @@ class FetchOEmbedService return if @endpoint_url.blank? @endpoint_url = (Addressable::URI.parse(@url) + @endpoint_url).to_s + + cache_endpoint! rescue Addressable::URI::InvalidURIError @endpoint_url = nil end + def parse_cached_endpoint! + cached = @options[:cached_endpoint] + + return if cached[:endpoint].nil? || cached[:format].nil? + + @endpoint_url = Addressable::Template.new(cached[:endpoint]).expand(url: @url).to_s + @format = cached[:format] + end + + def cache_endpoint! + url_domain = Addressable::URI.parse(@url).normalized_host + + endpoint_hash = { + endpoint: @endpoint_url.gsub(URI.encode_www_form_component(@url), '{url}'), + format: @format, + } + + Rails.cache.write("oembed_endpoint:#{url_domain}", endpoint_hash, expires_in: ENDPOINT_CACHE_EXPIRES_IN) + end + def fetch! return if @endpoint_url.blank? diff --git a/spec/services/fetch_oembed_service_spec.rb b/spec/services/fetch_oembed_service_spec.rb index 5789fb53b..a4262b040 100644 --- a/spec/services/fetch_oembed_service_spec.rb +++ b/spec/services/fetch_oembed_service_spec.rb @@ -113,6 +113,24 @@ describe FetchOEmbedService, type: :service do end + context 'when endpoint is cached' do + before do + stub_request(:get, 'http://www.youtube.com/oembed?format=json&url=https://www.youtube.com/watch?v=dqwpQarrDwk').to_return( + status: 200, + headers: { 'Content-Type': 'text/html' }, + body: request_fixture('oembed_json_empty.html') + ) + end + + it 'returns new provider without fetching original URL first' do + subject.call('https://www.youtube.com/watch?v=dqwpQarrDwk', cached_endpoint: { endpoint: 'http://www.youtube.com/oembed?format=json&url={url}', format: :json }) + expect(a_request(:get, 'https://www.youtube.com/watch?v=dqwpQarrDwk')).to_not have_been_made + expect(subject.endpoint_url).to eq 'http://www.youtube.com/oembed?format=json&url=https%3A%2F%2Fwww.youtube.com%2Fwatch%3Fv%3DdqwpQarrDwk' + expect(subject.format).to eq :json + expect(a_request(:get, 'http://www.youtube.com/oembed?format=json&url=https%3A%2F%2Fwww.youtube.com%2Fwatch%3Fv%3DdqwpQarrDwk')).to have_been_made + end + end + context 'when status code is not 200' do before do stub_request(:get, 'https://host.test/oembed.html').to_return( -- cgit