Improve from search prefix error handling (#37)

* Make `from` search prefix more robust (addresses mastodon/mastodon#17941)

* Improve robustness for account string validation

Using unsupported prefixes now reports a 422; searching for posts from an
account the instance is not aware of reports a 404. TODO: The UI for this
on the front end is abysmal.

Searching `from:username@domain` now succeeds when `domain` is the local
domain; searching `from:@username(@Domain)?` now works as expected.

* Satisfy upstream rubcocp

* Unbreak upstream tests

* Make account string validation consistent with mention processing

We previously matched on one-character domains and domains ending with
`[\.-]`, allowing `from:@A@a` and `from:@A@a-` searches to cause an
account lookup. This commit will raise a syntax error in both cases, as
MENTION_RE would never match them.

* Refactor `from` prefix error handling.

Incorporates changes suggested in #37. In doing so, adopts an error
handling style more consistent with the existing codebase (for which I
must thank @ClearlyClaire).

Removes new code no longer in use.

app/controllers/api/v2/search_controller.rb

@@ -11,6 +11,16 @@ class Api::V2::SearchController < Api::BaseController
   def index
     @search = Search.new(search_results)
     render json: @search, serializer: REST::SearchSerializer
+
+  # TODO: in the front end, these will show a toast that is only barely helpful
+  # TODO: semantics?
+
+  # user searched with a prefix that does exist
+  rescue Mastodon::SyntaxError
+    unprocessable_entity
+  # user searched for posts from an account the instance is not aware of
+  rescue ActiveRecord::NotFound
+    not_found
   end
 
   private

app/lib/search_query_transformer.rb

@@ -88,14 +88,12 @@ def initialize(prefix, term)
       case prefix
       when 'from'
         @filter = :account_id
-        username, domain = term.split('@')
-        account = Account.find_remote(username, domain)
-
-        raise "Account not found: #{term}" unless account
-
+        username, domain = Account.validate_account_string!(term)
+        account = Account.find_local_or_remote!(username, domain)
         @term = account.id
+      # TODO: consider, instead of erroring on non-prefixes, treating them as words to search for. motivating case: searching for `https://.*`
       else
-        raise "Unknown prefix: #{prefix}"
+        raise Mastodon::SyntaxError, "Unknown prefix: #{prefix}"
       end
     end
   end

app/models/concerns/account_finder_concern.rb

@@ -12,6 +12,10 @@ def find_remote!(username, domain)
       find_remote(username, domain) || raise(ActiveRecord::RecordNotFound)
     end
 
+    def find_local_or_remote!(username, domain)
+      find_local_or_remote(username, domain) || raise(ActiveRecord::RecordNotFound)
+    end
+
     def representative
       Account.find(-99)
     rescue ActiveRecord::RecordNotFound
@@ -25,8 +29,29 @@ def find_local(username)
     def find_remote(username, domain)
       AccountFinder.new(username, domain).account
     end
+
+    def find_local_or_remote(username, domain)
+      TagManager.instance.local_domain?(domain) ? find_local(username) : find_remote(username, domain)
+    end
+
+    def validate_account_string!(account_string)
+      match = ACCOUNT_STRING_RE.match(account_string)
+      raise Mastodon::SyntaxError if match.nil? || match[:username].nil?
+
+      [match[:username], match[:domain]]
+    end
   end
 
+  # TODO: where should this go?
+  #
+  # this is adapted from MENTION_RE to
+  #   + capture only a mention,
+  #   + not require the mention to begin with an @,
+  #   + not match if there is anything surrounding the mention, and
+  #   + add named subgroup matches
+  # it would be ideal to explicitly refer to MENTION_RE, or a more fundamental regexp that we refactor MENTION_RE to incorporate
+  ACCOUNT_STRING_RE = /^@?(?<username>#{Account::USERNAME_RE})(?:@(?<domain>[[:word:]\.\-]+[[:word:]]+))?$/i
+
   class AccountFinder
     attr_reader :username, :domain
 

lib/exceptions.rb

@@ -10,6 +10,7 @@ class DimensionsValidationError < ValidationError; end
   class StreamValidationError < ValidationError; end
   class RaceConditionError < Error; end
   class RateLimitExceededError < Error; end
+  class SyntaxError < Error; end
 
   class UnexpectedResponseError < Error
     attr_reader :response