From 5aa3df017bcdefd15e041ca0a7e428f85887aff2 Mon Sep 17 00:00:00 2001
From: Eugen Rochko <eugen@zeonfederated.com>
Date: Wed, 22 Mar 2017 17:36:34 +0100
Subject: [PATCH] Fix full-text search query quotation, improve tag search
 performance with an index, add ability to open status by URL from search (fix
 #53)

---
 .../components/autosuggest_account.jsx        |  7 ++++++-
 .../compose/components/autosuggest_status.jsx | 15 +++++++++++++++
 .../features/compose/components/search.jsx    |  9 +++++++--
 .../autosuggest_status_container.jsx          | 15 +++++++++++++++
 .../components/reducers/accounts.jsx          |  2 +-
 .../components/reducers/search.jsx            | 19 ++++++++++++++++---
 app/assets/stylesheets/components.scss        | 10 ++++++++++
 app/models/account.rb                         | 10 ++++++----
 app/models/tag.rb                             |  5 +++--
 app/services/fetch_remote_account_service.rb  |  9 +++++++--
 app/services/fetch_remote_resource_service.rb |  4 ++--
 app/services/fetch_remote_status_service.rb   |  9 +++++++--
 ...20170322162804_add_search_index_to_tags.rb |  9 +++++++++
 db/schema.rb                                  |  3 ++-
 14 files changed, 106 insertions(+), 20 deletions(-)
 create mode 100644 app/assets/javascripts/components/features/compose/components/autosuggest_status.jsx
 create mode 100644 app/assets/javascripts/components/features/compose/containers/autosuggest_status_container.jsx
 create mode 100644 db/migrate/20170322162804_add_search_index_to_tags.rb

diff --git a/app/assets/javascripts/components/features/compose/components/autosuggest_account.jsx b/app/assets/javascripts/components/features/compose/components/autosuggest_account.jsx
index 9ea7f190..5591b45c 100644
--- a/app/assets/javascripts/components/features/compose/components/autosuggest_account.jsx
+++ b/app/assets/javascripts/components/features/compose/components/autosuggest_account.jsx
@@ -1,11 +1,16 @@
 import Avatar from '../../../components/avatar';
 import DisplayName from '../../../components/display_name';
+import ImmutablePropTypes from 'react-immutable-proptypes';
 
 const AutosuggestAccount = ({ account }) => (
-  <div style={{ overflow: 'hidden' }}>
+  <div style={{ overflow: 'hidden' }} className='autosuggest-account'>
     <div style={{ float: 'left', marginRight: '5px' }}><Avatar src={account.get('avatar')} size={18} /></div>
     <DisplayName account={account} />
   </div>
 );
 
+AutosuggestAccount.propTypes = {
+  account: ImmutablePropTypes.map.isRequired
+};
+
 export default AutosuggestAccount;
diff --git a/app/assets/javascripts/components/features/compose/components/autosuggest_status.jsx b/app/assets/javascripts/components/features/compose/components/autosuggest_status.jsx
new file mode 100644
index 00000000..08648864
--- /dev/null
+++ b/app/assets/javascripts/components/features/compose/components/autosuggest_status.jsx
@@ -0,0 +1,15 @@
+import { FormattedMessage } from 'react-intl';
+import DisplayName from '../../../components/display_name';
+import ImmutablePropTypes from 'react-immutable-proptypes';
+
+const AutosuggestStatus = ({ status }) => (
+  <div style={{ overflow: 'hidden' }} className='autosuggest-status'>
+    <FormattedMessage id='search.status_by' defaultMessage='Status by {name}' values={{ name: <strong>@{status.getIn(['account', 'acct'])}</strong> }} />
+  </div>
+);
+
+AutosuggestStatus.propTypes = {
+  status: ImmutablePropTypes.map.isRequired
+};
+
+export default AutosuggestStatus;
diff --git a/app/assets/javascripts/components/features/compose/components/search.jsx b/app/assets/javascripts/components/features/compose/components/search.jsx
index c1f23939..a0e8f82f 100644
--- a/app/assets/javascripts/components/features/compose/components/search.jsx
+++ b/app/assets/javascripts/components/features/compose/components/search.jsx
@@ -2,6 +2,7 @@ import PureRenderMixin from 'react-addons-pure-render-mixin';
 import ImmutablePropTypes from 'react-immutable-proptypes';
 import Autosuggest from 'react-autosuggest';
 import AutosuggestAccountContainer from '../containers/autosuggest_account_container';
+import AutosuggestStatusContainer from '../containers/autosuggest_status_container';
 import { debounce } from 'react-decoration';
 import { defineMessages, injectIntl, FormattedMessage } from 'react-intl';
 
@@ -14,8 +15,10 @@ const getSuggestionValue = suggestion => suggestion.value;
 const renderSuggestion = suggestion => {
   if (suggestion.type === 'account') {
     return <AutosuggestAccountContainer id={suggestion.id} />;
+  } else if (suggestion.type === 'hashtag') {
+    return <span>#{suggestion.id}</span>;
   } else {
-    return <span>#{suggestion.id}</span>
+    return <AutosuggestStatusContainer id={suggestion.id} />;
   }
 };
 
@@ -78,8 +81,10 @@ const Search = React.createClass({
   onSuggestionSelected (_, { suggestion }) {
     if (suggestion.type === 'account') {
       this.context.router.push(`/accounts/${suggestion.id}`);
-    } else {
+    } else if(suggestion.type === 'hashtag') {
       this.context.router.push(`/timelines/tag/${suggestion.id}`);
+    } else {
+      this.context.router.push(`/statuses/${suggestion.id}`);
     }
   },
 
diff --git a/app/assets/javascripts/components/features/compose/containers/autosuggest_status_container.jsx b/app/assets/javascripts/components/features/compose/containers/autosuggest_status_container.jsx
new file mode 100644
index 00000000..ef46eb09
--- /dev/null
+++ b/app/assets/javascripts/components/features/compose/containers/autosuggest_status_container.jsx
@@ -0,0 +1,15 @@
+import { connect } from 'react-redux';
+import AutosuggestStatus from '../components/autosuggest_status';
+import { makeGetStatus } from '../../../selectors';
+
+const makeMapStateToProps = () => {
+  const getStatus = makeGetStatus();
+
+  const mapStateToProps = (state, { id }) => ({
+    status: getStatus(state, id)
+  });
+
+  return mapStateToProps;
+};
+
+export default connect(makeMapStateToProps)(AutosuggestStatus);
diff --git a/app/assets/javascripts/components/reducers/accounts.jsx b/app/assets/javascripts/components/reducers/accounts.jsx
index f3938cee..6ce41670 100644
--- a/app/assets/javascripts/components/reducers/accounts.jsx
+++ b/app/assets/javascripts/components/reducers/accounts.jsx
@@ -90,7 +90,6 @@ export default function accounts(state = initialState, action) {
   case REBLOGS_FETCH_SUCCESS:
   case FAVOURITES_FETCH_SUCCESS:
   case COMPOSE_SUGGESTIONS_READY:
-  case SEARCH_SUGGESTIONS_READY:
   case FOLLOW_REQUESTS_FETCH_SUCCESS:
   case FOLLOW_REQUESTS_EXPAND_SUCCESS:
   case BLOCKS_FETCH_SUCCESS:
@@ -98,6 +97,7 @@ export default function accounts(state = initialState, action) {
     return normalizeAccounts(state, action.accounts);
   case NOTIFICATIONS_REFRESH_SUCCESS:
   case NOTIFICATIONS_EXPAND_SUCCESS:
+  case SEARCH_SUGGESTIONS_READY:
     return normalizeAccountsFromStatuses(normalizeAccounts(state, action.accounts), action.statuses);
   case TIMELINE_REFRESH_SUCCESS:
   case TIMELINE_EXPAND_SUCCESS:
diff --git a/app/assets/javascripts/components/reducers/search.jsx b/app/assets/javascripts/components/reducers/search.jsx
index 1767be2c..e95f9ed7 100644
--- a/app/assets/javascripts/components/reducers/search.jsx
+++ b/app/assets/javascripts/components/reducers/search.jsx
@@ -32,7 +32,7 @@ const normalizeSuggestions = (state, value, accounts, hashtags, statuses) => {
       value: `#${item}`
     }));
 
-    if (value.indexOf('@') === -1 && value.indexOf(' ') === -1 && hashtags.indexOf(value) === -1) {
+    if (value.indexOf('@') === -1 && value.indexOf(' ') === -1 && !value.startsWith('http://') && !value.startsWith('https://') && hashtags.indexOf(value) === -1) {
       hashtagItems.unshift({
         type: 'hashtag',
         id: value,
@@ -40,9 +40,22 @@ const normalizeSuggestions = (state, value, accounts, hashtags, statuses) => {
       });
     }
 
+    if (hashtagItems.length > 0) {
+      newSuggestions.push({
+        title: 'hashtag',
+        items: hashtagItems
+      });
+    }
+  }
+
+  if (statuses.length > 0) {
     newSuggestions.push({
-      title: 'hashtag',
-      items: hashtagItems
+      title: 'status',
+      items: statuses.map(item => ({
+        type: 'status',
+        id: item.id,
+        value: item.id
+      }))
     });
   }
 
diff --git a/app/assets/stylesheets/components.scss b/app/assets/stylesheets/components.scss
index 4b1e86ac..057c61f9 100644
--- a/app/assets/stylesheets/components.scss
+++ b/app/assets/stylesheets/components.scss
@@ -1421,3 +1421,13 @@ button.active i.fa-retweet {
     }
   }
 }
+
+.autosuggest-status {
+  overflow: hidden;
+  white-space: nowrap;
+  text-overflow: ellipsis;
+
+  strong {
+    font-weight: 500;
+  }
+}
diff --git a/app/models/account.rb b/app/models/account.rb
index c0cd2ff6..6968607a 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -222,8 +222,9 @@ SQL
     end
 
     def search_for(terms, limit = 10)
+      terms      = Arel.sql(connection.quote(terms.gsub(/['?\\:]/, ' ')))
       textsearch = '(setweight(to_tsvector(\'simple\', accounts.display_name), \'A\') || setweight(to_tsvector(\'simple\', accounts.username), \'B\') || setweight(to_tsvector(\'simple\', coalesce(accounts.domain, \'\')), \'C\'))'
-      query      = 'to_tsquery(\'simple\', \'\'\' \' || ? || \' \'\'\' || \':*\')'
+      query      = 'to_tsquery(\'simple\', \'\'\' \' || ' + terms + ' || \' \'\'\' || \':*\')'
 
       sql = <<SQL
         SELECT
@@ -235,12 +236,13 @@ SQL
         LIMIT ?
 SQL
 
-      Account.find_by_sql([sql, terms, terms, limit])
+      Account.find_by_sql([sql, limit])
     end
 
     def advanced_search_for(terms, account, limit = 10)
+      terms      = Arel.sql(connection.quote(terms.gsub(/['?\\:]/, ' ')))
       textsearch = '(setweight(to_tsvector(\'simple\', accounts.display_name), \'A\') || setweight(to_tsvector(\'simple\', accounts.username), \'B\') || setweight(to_tsvector(\'simple\', coalesce(accounts.domain, \'\')), \'C\'))'
-      query      = 'to_tsquery(\'simple\', \'\'\' \' || ? || \' \'\'\' || \':*\')'
+      query      = 'to_tsquery(\'simple\', \'\'\' \' || ' + terms + ' || \' \'\'\' || \':*\')'
 
       sql = <<SQL
         SELECT
@@ -254,7 +256,7 @@ SQL
         LIMIT ?
 SQL
 
-      Account.find_by_sql([sql, terms, account.id, account.id, terms, limit])
+      Account.find_by_sql([sql, account.id, account.id, limit])
     end
 
     def following_map(target_account_ids, account_id)
diff --git a/app/models/tag.rb b/app/models/tag.rb
index e2ad8e4d..15625ca4 100644
--- a/app/models/tag.rb
+++ b/app/models/tag.rb
@@ -13,8 +13,9 @@ class Tag < ApplicationRecord
 
   class << self
     def search_for(terms, limit = 5)
+      terms      = Arel.sql(connection.quote(terms.gsub(/['?\\:]/, ' ')))
       textsearch = 'to_tsvector(\'simple\', tags.name)'
-      query      = 'to_tsquery(\'simple\', \'\'\' \' || ? || \' \'\'\' || \':*\')'
+      query      = 'to_tsquery(\'simple\', \'\'\' \' || ' + terms + ' || \' \'\'\' || \':*\')'
 
       sql = <<SQL
         SELECT
@@ -26,7 +27,7 @@ class Tag < ApplicationRecord
         LIMIT ?
 SQL
 
-      Tag.find_by_sql([sql, terms, terms, limit])
+      Tag.find_by_sql([sql, limit])
     end
   end
 end
diff --git a/app/services/fetch_remote_account_service.rb b/app/services/fetch_remote_account_service.rb
index baefa3a8..6a6a696d 100644
--- a/app/services/fetch_remote_account_service.rb
+++ b/app/services/fetch_remote_account_service.rb
@@ -1,8 +1,13 @@
 # frozen_string_literal: true
 
 class FetchRemoteAccountService < BaseService
-  def call(url)
-    atom_url, body = FetchAtomService.new.call(url)
+  def call(url, prefetched_body = nil)
+    if prefetched_body.nil?
+      atom_url, body = FetchAtomService.new.call(url)
+    else
+      atom_url = url
+      body     = prefetched_body
+    end
 
     return nil if atom_url.nil?
     process_atom(atom_url, body)
diff --git a/app/services/fetch_remote_resource_service.rb b/app/services/fetch_remote_resource_service.rb
index 80aa7436..2185ceb2 100644
--- a/app/services/fetch_remote_resource_service.rb
+++ b/app/services/fetch_remote_resource_service.rb
@@ -10,9 +10,9 @@ class FetchRemoteResourceService < BaseService
     xml.encoding = 'utf-8'
 
     if xml.root.name == 'feed'
-      FetchRemoteAccountService.new.call(atom_url)
+      FetchRemoteAccountService.new.call(atom_url, body)
     elsif xml.root.name == 'entry'
-      FetchRemoteStatusService.new.call(atom_url)
+      FetchRemoteStatusService.new.call(atom_url, body)
     end
   end
 end
diff --git a/app/services/fetch_remote_status_service.rb b/app/services/fetch_remote_status_service.rb
index 7063231e..e2d18572 100644
--- a/app/services/fetch_remote_status_service.rb
+++ b/app/services/fetch_remote_status_service.rb
@@ -1,8 +1,13 @@
 # frozen_string_literal: true
 
 class FetchRemoteStatusService < BaseService
-  def call(url)
-    atom_url, body = FetchAtomService.new.call(url)
+  def call(url, prefetched_body = nil)
+    if prefetched_body.nil?
+      atom_url, body = FetchAtomService.new.call(url)
+    else
+      atom_url = url
+      body     = prefetched_body
+    end
 
     return nil if atom_url.nil?
     process_atom(atom_url, body)
diff --git a/db/migrate/20170322162804_add_search_index_to_tags.rb b/db/migrate/20170322162804_add_search_index_to_tags.rb
new file mode 100644
index 00000000..415dff9a
--- /dev/null
+++ b/db/migrate/20170322162804_add_search_index_to_tags.rb
@@ -0,0 +1,9 @@
+class AddSearchIndexToTags < ActiveRecord::Migration[5.0]
+  def up
+    execute 'CREATE INDEX hashtag_search_index ON tags USING gin(to_tsvector(\'simple\', tags.name));'
+  end
+
+  def down
+    remove_index :tags, name: :hashtag_search_index
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 44b52c22..2457b523 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170322143850) do
+ActiveRecord::Schema.define(version: 20170322162804) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -259,6 +259,7 @@ ActiveRecord::Schema.define(version: 20170322143850) do
     t.string   "name",       default: "", null: false
     t.datetime "created_at",              null: false
     t.datetime "updated_at",              null: false
+    t.index "to_tsvector('simple'::regconfig, (name)::text)", name: "hashtag_search_index", using: :gin
     t.index ["name"], name: "index_tags_on_name", unique: true, using: :btree
   end