Merge pull request opf#16780 from opf/bug/55393-500-when-filtering-by…

…-date-field-and-specifying-too-big-number

Bug/55393 500 when filtering by date field and specifying too big number

app/models/queries/operators.rb

@@ -27,7 +27,7 @@
 #++
 
 module Queries::Operators
-  operators = [
+  OPERATORS = [
     Queries::Operators::GreaterOrEqual,
     Queries::Operators::LessOrEqual,
     Queries::Operators::Equals,
@@ -61,7 +61,5 @@ module Queries::Operators
     Queries::Operators::Parent,
     Queries::Operators::Children,
     Queries::Operators::Child
-  ]
-
-  OPERATORS = Hash[*(operators.map { |o| [o.symbol.to_s, o] }).flatten].freeze
+  ].index_by { |o| o.symbol.to_s }.freeze
 end

app/models/queries/operators/ago.rb

@@ -34,10 +34,9 @@ class Ago < Base
     extend DateRangeClauses
 
     def self.sql_for_field(values, db_table, db_field)
-      relative_date_range_clause(db_table,
-                                 db_field,
-                                 - values.first.to_i,
-                                 - values.first.to_i)
+      days = values.first.to_i
+
+      relative_date_range_clause(db_table, db_field, -days, -days)
     end
   end
 end

app/models/queries/operators/between_date.rb

@@ -34,7 +34,7 @@ class BetweenDate < Base
     extend DateRangeClauses
 
     def self.sql_for_field(values, db_table, db_field)
-      lower_boundary, upper_boundary = values.map { |v| v.blank? ? nil : Date.parse(v) }
+      lower_boundary, upper_boundary = values.map { |v| Date.parse(v) if v.present? }
 
       date_range_clause(db_table, db_field, lower_boundary, upper_boundary)
     end

app/models/queries/operators/between_date_time.rb

@@ -34,12 +34,9 @@ class BetweenDateTime < Base
     extend DatetimeRangeClauses
 
     def self.sql_for_field(values, db_table, db_field)
-      lower_boundary, upper_boundary = values.map { |v| v.blank? ? nil : DateTime.parse(v) }
+      lower_boundary, upper_boundary = values.map { |v| DateTime.parse(v) if v.present? }
 
-      datetime_range_clause(db_table,
-                            db_field,
-                            lower_boundary,
-                            upper_boundary)
+      datetime_range_clause(db_table, db_field, lower_boundary, upper_boundary)
     end
   end
 end

app/models/queries/operators/date_limits.rb

@@ -0,0 +1,48 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+module Queries::Operators
+  module DateLimits
+    # Technically dates in PostgreSQL can be up to 5874897 AD, but limit to
+    # timestamp range, as dates are used to query for timestamps too.
+    # Date.new BCE years are counted astronomically, so 0 is 1 BC.
+    # Minimal allowed date is Date.new(-4713, 11, 24), but specification says 4713 BC (-4712).
+    #
+    # https://www.postgresql.org/docs/current/datatype-datetime.html
+    PG_DATE_FROM = Date.new(-4712, 1, 1)
+    PG_DATE_TO_EXCLUSIVE = Date.new(294276 + 1, 1, 1)
+
+    def date_too_small?(date)
+      date < PG_DATE_FROM
+    end
+
+    def date_too_big?(date)
+      date >= PG_DATE_TO_EXCLUSIVE
+    end
+  end
+end

app/models/queries/operators/date_range_clauses.rb

@@ -28,31 +28,45 @@
 
 module Queries::Operators
   module DateRangeClauses
+    include DateLimits
+
     # Returns a SQL clause for a date or datetime field for a relative range from
     # the end of the day of yesterday + from until the end of today + to.
     def relative_date_range_clause(table, field, from, to)
-      if from
-        from_date = Date.today + from
-      end
-      if to
-        to_date = Date.today + to
-      end
+      today = Time.zone.today
+
+      from_date = today + from if from
+      to_date = today + to if to
+
       date_range_clause(table, field, from_date, to_date)
     end
 
     # Returns a SQL clause for date or datetime field for an exact range starting
     # at the beginning of the day of from until the end of the day of to
     def date_range_clause(table, field, from, to)
       s = []
+
       if from
-        s << ("#{table}.#{field} > '%s'" % [quoted_date_from_utc(from.yesterday)])
+        return "1 <> 1" if date_too_big?(from)
+
+        unless date_too_small?(from)
+          s << "#{table}.#{field} > '#{quoted_date_from_utc(from.yesterday)}'"
+        end
       end
+
       if to
-        s << ("#{table}.#{field} <= '%s'" % [quoted_date_from_utc(to)])
+        return "1 <> 1" if date_too_small?(to)
+
+        unless date_too_big?(to)
+          s << "#{table}.#{field} <= '#{quoted_date_from_utc(to)}'"
+        end
       end
-      s.join(" AND ")
+
+      s.join(" AND ").presence || "1 = 1"
     end
 
+    private
+
     def quoted_date_from_utc(value)
       connection.quoted_date(value.to_time(:utc).end_of_day)
     end

app/models/queries/operators/datetime_range_clauses.rb

@@ -28,15 +28,28 @@
 
 module Queries::Operators
   module DatetimeRangeClauses
+    include DateLimits
+
     def datetime_range_clause(table, field, from, to)
       s = []
+
       if from
-        s << ("#{table}.#{field} >= '%s'" % [connection.quoted_date(from)])
+        return "1 <> 1" if date_too_big?(from)
+
+        unless date_too_small?(from)
+          s << "#{table}.#{field} >= '#{connection.quoted_date(from)}'"
+        end
       end
+
       if to
-        s << ("#{table}.#{field} <= '%s'" % [connection.quoted_date(to)])
+        return "1 <> 1" if date_too_small?(to)
+
+        unless date_too_big?(to)
+          s << "#{table}.#{field} <= '#{connection.quoted_date(to)}'"
+        end
       end
-      s.join(" AND ")
+
+      s.join(" AND ").presence || "1 = 1"
     end
   end
 end

app/models/queries/operators/in.rb

@@ -34,7 +34,9 @@ class In < Base
     extend DateRangeClauses
 
     def self.sql_for_field(values, db_table, db_field)
-      relative_date_range_clause(db_table, db_field, values.first.to_i, values.first.to_i)
+      days = values.first.to_i
+
+      relative_date_range_clause(db_table, db_field, days, days)
     end
   end
 end

app/models/queries/operators/in_less_than.rb

@@ -34,7 +34,9 @@ class InLessThan < Base
     extend DateRangeClauses
 
     def self.sql_for_field(values, db_table, db_field)
-      relative_date_range_clause(db_table, db_field, 0, values.first.to_i)
+      days = values.first.to_i
+
+      relative_date_range_clause(db_table, db_field, 0, days)
     end
   end
 end

app/models/queries/operators/in_more_than.rb

@@ -34,7 +34,9 @@ class InMoreThan < Base
     extend DateRangeClauses
 
     def self.sql_for_field(values, db_table, db_field)
-      relative_date_range_clause(db_table, db_field, values.first.to_i, nil)
+      days = values.first.to_i
+
+      relative_date_range_clause(db_table, db_field, days, nil)
     end
   end
 end

app/models/queries/operators/less_than_ago.rb

@@ -34,7 +34,9 @@ class LessThanAgo < Base
     extend DateRangeClauses
 
     def self.sql_for_field(values, db_table, db_field)
-      relative_date_range_clause(db_table, db_field, - values.first.to_i, 0)
+      days = values.first.to_i
+
+      relative_date_range_clause(db_table, db_field, -days, 0)
     end
   end
 end

app/models/queries/operators/more_than_ago.rb

@@ -34,7 +34,9 @@ class MoreThanAgo < Base
     extend DateRangeClauses
 
     def self.sql_for_field(values, db_table, db_field)
-      relative_date_range_clause(db_table, db_field, nil, - values.first.to_i)
+      days = values.first.to_i
+
+      relative_date_range_clause(db_table, db_field, nil, -days)
     end
   end
 end

app/models/queries/operators/on_date.rb

@@ -34,10 +34,9 @@ class OnDate < Base
     extend DateRangeClauses
 
     def self.sql_for_field(values, db_table, db_field)
-      date_range_clause(db_table,
-                        db_field,
-                        Date.parse(values.first),
-                        Date.parse(values.first))
+      date = Date.parse(values.first)
+
+      date_range_clause(db_table, db_field, date, date)
     end
   end
 end

app/models/queries/operators/on_date_time.rb

@@ -39,10 +39,7 @@ def self.sql_for_field(values, db_table, db_field)
       lower_boundary = datetime
       upper_boundary = datetime + 24.hours
 
-      datetime_range_clause(db_table,
-                            db_field,
-                            lower_boundary,
-                            upper_boundary)
+      datetime_range_clause(db_table, db_field, lower_boundary, upper_boundary)
     end
   end
 end

app/models/queries/filters/strategies/validations.rb → lib/api/helpers/raise_query_errors.rb

@@ -26,26 +26,16 @@
 # See COPYRIGHT and LICENSE files for more details.
 #++
 
-module Queries::Filters::Strategies
-  module Validations
-    private
+module API
+  module Helpers
+    module RaiseQueryErrors
+      def raise_query_errors(object)
+        api_errors = object.errors.full_messages.map do |message|
+          ::API::Errors::InvalidQuery.new(message)
+        end
 
-    def date?(str)
-      true if Date.parse(str)
-    rescue StandardError
-      false
-    end
-
-    def validate
-      unless values.all? { |value| value.blank? || date?(value) }
-        errors.add(:values, I18n.t("activerecord.errors.messages.not_a_date"))
+        raise ::API::Errors::MultipleErrors.create_if_many api_errors
       end
     end
-
-    def integer?(str)
-      true if Integer(str)
-    rescue StandardError
-      false
-    end
   end
 end

lib/api/root_api.rb

@@ -48,6 +48,8 @@ class RootAPI < Grape::API
 
     helpers API::Caching::Helpers
     module Helpers
+      include ::API::Helpers::RaiseQueryErrors
+
       def current_user
         User.current
       end
@@ -257,14 +259,6 @@ def authorize_logged_in
         authorize_by_with_raise((current_user.logged? && current_user.active?) || current_user.is_a?(SystemUser))
       end
 
-      def raise_query_errors(object)
-        api_errors = object.errors.full_messages.map do |message|
-          ::API::Errors::InvalidQuery.new(message)
-        end
-
-        raise ::API::Errors::MultipleErrors.create_if_many api_errors
-      end
-
       def raise_invalid_query_on_service_failure
         service = yield
 

lib/api/v3/utilities/params_to_query.rb

@@ -31,6 +31,8 @@ module V3
     module Utilities
       class ParamsToQuery
         class << self
+          include ::API::Helpers::RaiseQueryErrors
+
           def collection_response(scope, current_user, params, representer: nil, self_link: nil)
             model = model_class(scope)