From 8d8a56e4930445e6909399b9c61294495d2cd722 Mon Sep 17 00:00:00 2001 From: andresgutgon Date: Thu, 11 Oct 2018 14:13:18 +0200 Subject: [PATCH 1/2] Implement database adapters to be able to have diferent ways of implementing the same feature --- README.md | 5 +- lib/active_reporting.rb | 5 ++ lib/active_reporting/configuration.rb | 17 ++++++ .../database_adapters/base.rb | 33 +++++++++++ .../database_adapters/factory.rb | 27 +++++++++ .../database_adapters/mysql_adapter.rb | 56 +++++++++++++++++++ .../database_adapters/postgresql_adapter.rb | 38 +++++++++++++ .../database_adapters/sqlite_adapter.rb | 8 +++ lib/active_reporting/reporting_dimension.rb | 27 ++++----- test/active_reporting/report_test.rb | 20 ++++++- .../reporting_dimension_test.rb | 2 +- test/fact_models.rb | 1 + test/seed.rb | 1 + test/test_helper.rb | 11 +++- 14 files changed, 229 insertions(+), 22 deletions(-) create mode 100644 lib/active_reporting/database_adapters/base.rb create mode 100644 lib/active_reporting/database_adapters/factory.rb create mode 100644 lib/active_reporting/database_adapters/mysql_adapter.rb create mode 100644 lib/active_reporting/database_adapters/postgresql_adapter.rb create mode 100644 lib/active_reporting/database_adapters/sqlite_adapter.rb diff --git a/README.md b/README.md index c582469..07a2ab4 100644 --- a/README.md +++ b/README.md @@ -142,6 +142,8 @@ ActiveReporting::Configuration.setting = value `metric_lookup_class` - The name of a constant used to lookup prebuilt `Reporting::Metric` objects by name. The constant should define a class method called `#lookup` which can take a string or symbol of the metric name. (Default: `::Metric`) +`db_adapter` - Each database has a diferent way of doing the same thing on SQL. This is an abstraction layer on each database. For example `PostgreSQL` have a native `date_trunc` while `MySQL` doesn't so we do the same in another way. Make sure to choose your database's adapter (Default: `:sqlite3`). If you want MySQL put `:mysql2` and if you want PostGreSQL put `:postgresql`. + ## ActiveReporting::FactModel @@ -219,7 +221,7 @@ class PhoneFactModel < ActiveReporting::FactModel end ``` -### Implicit hierarchies with datetime columns (PostgreSQL support only) +### Implicit hierarchies with datetime columns (PostgreSQL and MySQL support only) The fastest approach to group by certain date metrics is to create so-called "date dimensions". For those Postgres users that are restricted from organizing their data in this way, Postgres provides @@ -235,7 +237,6 @@ end When creating a metric, ActiveReporting will recognize implicit hierarchies for this dimension. The hierarchies correspond to the [values](https://www.postgresql.org/docs/8.1/static/functions-datetime.html#FUNCTIONS-DATETIME-TRUNC) supported by PostgreSQL. (See example under the metric section, below.) -*NOTE*: PRs welcomed to support this functionality in other databases. ## Configuring Dimension Filters diff --git a/lib/active_reporting.rb b/lib/active_reporting.rb index 591cf46..ba0ed13 100644 --- a/lib/active_reporting.rb +++ b/lib/active_reporting.rb @@ -1,6 +1,11 @@ # frozen_string_literal: true require 'active_record' +require 'active_reporting/database_adapters/base' +require 'active_reporting/database_adapters/sqlite_adapter' +require 'active_reporting/database_adapters/mysql_adapter' +require 'active_reporting/database_adapters/postgresql_adapter' +require 'active_reporting/database_adapters/factory' require 'active_reporting/active_record_adaptor' require 'active_reporting/configuration' require 'active_reporting/dimension' diff --git a/lib/active_reporting/configuration.rb b/lib/active_reporting/configuration.rb index 14dc11c..6b73d41 100644 --- a/lib/active_reporting/configuration.rb +++ b/lib/active_reporting/configuration.rb @@ -13,6 +13,22 @@ def self.config yield self end + # Set database adapter. There are some SQL functions that are only + # available in some databases. You need to specify adapter + # + # Default db_adapter: SqliteAdapter + def self.db_adapter + @db_adapter ||= DatabaseAdapters::Factory.for_database(:sqlite3) + end + + # Set database adapter. There are some SQL functions that are only + # available in some databases. You need to specify adapter + # + # @param [Symbol] adapter_name + def self.db_adapter=(adapter_name) + @db_adapter = DatabaseAdapters::Factory.for_database(adapter_name) + end + # The default label used by all dimensions if not set otherwise # # Default value is `:name` @@ -57,6 +73,7 @@ def self.ransack_fallback # @return [Boolean] def self.ransack_fallback=(fallback) raise RansackNotAvailable unless ransack_available + @ransack_fallback = fallback end diff --git a/lib/active_reporting/database_adapters/base.rb b/lib/active_reporting/database_adapters/base.rb new file mode 100644 index 0000000..e0c78d2 --- /dev/null +++ b/lib/active_reporting/database_adapters/base.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module ActiveReporting + module DatabaseAdapters + DatabaseNotSupported = Class.new(StandardError) + MethodNotImplemented = Class.new(StandardError) + # Database adapters are here to solve SQL problems in the + # most idiomatic way in each database + class Base + attr_reader :name + + def initialize(name) + @name = name + end + + # @param [Symbol] interval + # @return [Boolean] + def allowed_datetime_hierarchy?(interval) + datetime_hierarchies.include?(interval.try(:to_sym)) + end + + protected + + # Allowed datetime hierchies in each adapter + # By default (:sqlite) there is none + # + # @return [Array] + def datetime_hierarchies + @datetime_hierarchies ||= [] + end + end + end +end diff --git a/lib/active_reporting/database_adapters/factory.rb b/lib/active_reporting/database_adapters/factory.rb new file mode 100644 index 0000000..28b6b57 --- /dev/null +++ b/lib/active_reporting/database_adapters/factory.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module ActiveReporting + module DatabaseAdapters + class Factory + class << self + ADAPTERS = { + sqlite3: SqliteAdapter, + mysql2: MysqlAdapter, + postgresql: PostgresqlAdapter, + postgis: PostgresqlAdapter + }.freeze + # @param [Symbol] + def for_database(adapter_name) + adapter = ADAPTERS[adapter_name] + + return adapter.new(adapter_name) unless adapter.nil? + + raise( + DatabaseNotSupported, + "Database with this #{adapter_name} is not supported by ActiveReporting" + ) + end + end + end + end +end diff --git a/lib/active_reporting/database_adapters/mysql_adapter.rb b/lib/active_reporting/database_adapters/mysql_adapter.rb new file mode 100644 index 0000000..1f6ee47 --- /dev/null +++ b/lib/active_reporting/database_adapters/mysql_adapter.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module ActiveReporting + module DatabaseAdapters + # Database adapters are here to solve SQL problems in the + # most idiomatic way in each database + class MysqlAdapter < Base + # Generate SQL snippet with DATE_TRUNC + # @param [String] interval + # @param [String] field + # @return [String] + def date_trunc(interval, field) + clean_sql( + <<-SQL + DATE_ADD( + "#{super_old_base_date}", + INTERVAL TIMESTAMPDIFF( + #{interval.upcase}, + "#{super_old_base_date}", + #{field} + ) #{interval.upcase} + ) + SQL + ) + end + + protected + + # Remove spaces and put all in one line + def clean_sql(sql) + sql + .strip + .gsub(/\n+/, ' ') + .gsub(/\s+/, ' ') + .gsub(/\(\s+\)/, '(') + .gsub(/\)\s+\)/, ')') + end + + def datetime_hierarchies + @datetime_hierarchies ||= %i[ + year + month + week + ] + end + + # Used to generate a diff when implementing + # datetime truncation + # + # @return [String] + def super_old_base_date + '1900-01-01' + end + end + end +end diff --git a/lib/active_reporting/database_adapters/postgresql_adapter.rb b/lib/active_reporting/database_adapters/postgresql_adapter.rb new file mode 100644 index 0000000..3ba3d9c --- /dev/null +++ b/lib/active_reporting/database_adapters/postgresql_adapter.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module ActiveReporting + module DatabaseAdapters + class PostgresqlAdapter < Base + # Values for the Postgres `date_trunc` method. + # See https://www.postgresql.org/docs/10/static/functions-datetime.html#FUNCTIONS-DATETIME-TRUNC + + # Generate SQL snippet with DATE_TRUNC + # @param [String] interval + # @param [String] field + # @return [String] + def date_trunc(interval, field) + "DATE_TRUNC('#{interval}', #{field})" + end + + protected + + def datetime_hierarchies + @datetime_hierarchies ||= %i[ + microseconds + milliseconds + second + minute + hour + day + week + month + quarter + year + decade + century + millennium + ] + end + end + end +end diff --git a/lib/active_reporting/database_adapters/sqlite_adapter.rb b/lib/active_reporting/database_adapters/sqlite_adapter.rb new file mode 100644 index 0000000..be719fa --- /dev/null +++ b/lib/active_reporting/database_adapters/sqlite_adapter.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +module ActiveReporting + module DatabaseAdapters + class SqliteAdapter < Base + end + end +end diff --git a/lib/active_reporting/reporting_dimension.rb b/lib/active_reporting/reporting_dimension.rb index 73cadce..4a9814a 100644 --- a/lib/active_reporting/reporting_dimension.rb +++ b/lib/active_reporting/reporting_dimension.rb @@ -4,11 +4,6 @@ module ActiveReporting class ReportingDimension extend Forwardable - SUPPORTED_DBS = %w[PostgreSQL PostGIS].freeze - # Values for the Postgres `date_trunc` method. - # See https://www.postgresql.org/docs/10/static/functions-datetime.html#FUNCTIONS-DATETIME-TRUNC - DATETIME_HIERARCHIES = %i[microseconds milliseconds second minute hour day week month quarter year decade - century millennium].freeze def_delegators :@dimension, :name, :type, :klass, :association, :model, :hierarchical?, :datetime? def self.build_from_dimensions(fact_model, dimensions) @@ -108,7 +103,6 @@ def determine_label_name(label_name) def validate_hierarchical_label(hierarchical_label) if datetime? - validate_supported_database_for_datetime_hierarchies validate_against_datetime_hierarchies(hierarchical_label) else validate_dimension_is_hierachical(hierarchical_label) @@ -119,34 +113,35 @@ def validate_hierarchical_label(hierarchical_label) def validate_dimension_is_hierachical(hierarchical_label) return if hierarchical? - raise InvalidDimensionLabel, "#{name} must be hierarchical to use label #{hierarchical_label}" - end - def validate_supported_database_for_datetime_hierarchies - return if SUPPORTED_DBS.include?(model.connection.adapter_name) - raise InvalidDimensionLabel, - "Cannot utilize datetime grouping for #{name}; " \ - "database #{model.connection.adapter_name} is not supported" + raise InvalidDimensionLabel, "#{name} must be hierarchical to use label #{hierarchical_label}" end def validate_against_datetime_hierarchies(hierarchical_label) - return if DATETIME_HIERARCHIES.include?(hierarchical_label.to_sym) + return if Configuration.db_adapter.allowed_datetime_hierarchy?(hierarchical_label) + raise InvalidDimensionLabel, "#{hierarchical_label} is not a valid datetime grouping label in #{name}" end def validate_against_fact_model_properties(hierarchical_label) return if dimension_fact_model.hierarchical_levels.include?(hierarchical_label.to_sym) + raise InvalidDimensionLabel, "#{hierarchical_label} is not a hierarchical label in #{name}" end def degenerate_fragment return "#{name}_#{@label}" if datetime? + "#{model.quoted_table_name}.#{name}" end def degenerate_select_fragment - return "DATE_TRUNC('#{@label}', #{model.quoted_table_name}.#{name}) AS #{name}_#{@label}" if datetime? - "#{model.quoted_table_name}.#{name}" + return "#{model.quoted_table_name}.#{name}" unless datetime? + + date_trunc = Configuration.db_adapter.date_trunc( + @label, "#{model.quoted_table_name}.#{name}" + ) + "#{date_trunc} AS #{name}_#{@label}" end def identifier_fragment diff --git a/test/active_reporting/report_test.rb b/test/active_reporting/report_test.rb index e62d056..106323a 100644 --- a/test/active_reporting/report_test.rb +++ b/test/active_reporting/report_test.rb @@ -32,8 +32,23 @@ def test_report_runs_with_an_aggregate_other_than_count assert data.all? { |r| r.key?('a_metric') } end - def test_report_runs_with_a_date_grouping - if ENV['DB'] == 'pg' + def test_report_runs_with_week_date_grouping + if valid_db_adapter? + metric = ActiveReporting::Metric.new(:a_metric, fact_model: SaleFactModel, dimensions: [{created_at: :week}]) + report = ActiveReporting::Report.new(metric) + data = report.run + assert data.all? { |r| r.key?('created_at_week') } + assert data.size == 4 + else + assert_raises ActiveReporting::InvalidDimensionLabel do + metric = ActiveReporting::Metric.new(:a_metric, fact_model: SaleFactModel, dimensions: [{created_at: :week}]) + report = ActiveReporting::Report.new(metric) + end + end + end + + def test_report_runs_with_month_date_grouping + if valid_db_adapter? metric = ActiveReporting::Metric.new(:a_metric, fact_model: UserFactModel, dimensions: [{created_at: :month}]) report = ActiveReporting::Report.new(metric) data = report.run @@ -46,4 +61,5 @@ def test_report_runs_with_a_date_grouping end end end + end diff --git a/test/active_reporting/reporting_dimension_test.rb b/test/active_reporting/reporting_dimension_test.rb index a3aae6d..68dcdee 100644 --- a/test/active_reporting/reporting_dimension_test.rb +++ b/test/active_reporting/reporting_dimension_test.rb @@ -106,7 +106,7 @@ def test_label_can_be_passed_in_if_dimension_is_herarchical def test_label_can_be_passed_in_if_dimension_is_datetime refute @user_dimension.hierarchical? assert @user_dimension.type == ActiveReporting::Dimension::TYPES[:degenerate] - if ENV['DB'] == 'pg' + if valid_db_adapter? ActiveReporting::ReportingDimension.new(@user_dimension, label: :year) else assert_raises ActiveReporting::InvalidDimensionLabel do diff --git a/test/fact_models.rb b/test/fact_models.rb index e81f755..cb81275 100644 --- a/test/fact_models.rb +++ b/test/fact_models.rb @@ -45,6 +45,7 @@ class UserFactModel < ActiveReporting::FactModel class SaleFactModel < ActiveReporting::FactModel self.measure= :total + dimension :created_at dimension :placed_at dimension :item end diff --git a/test/seed.rb b/test/seed.rb index 7d53e20..d102e56 100644 --- a/test/seed.rb +++ b/test/seed.rb @@ -23,6 +23,7 @@ base_price: base, taxes: tax, total: base + tax, + created_at: Time.now - i.days, placed_at: DateDimension.find("201603#{(rand(30)+1).to_s.rjust(2, '0')}") ) end diff --git a/test/test_helper.rb b/test/test_helper.rb index e4609a2..baf4d84 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -12,6 +12,7 @@ require 'minitest/pride' db = ENV['DB'] || 'sqlite' +db_user = ENV['DB_USER'] case db when 'pg' ActiveRecord::Base.establish_connection( @@ -21,12 +22,15 @@ # password: 'postgres', # Uncomment if you need this min_messages: 'warning' ) + ActiveReporting::Configuration.db_adapter = :postgresql when 'mysql' ActiveRecord::Base.establish_connection( adapter: 'mysql2', database: 'active_reporting_test', - encoding: 'utf8' + encoding: 'utf8', + username: db_user ) + ActiveReporting::Configuration.db_adapter = :mysql2 when 'sqlite' ActiveRecord::Base.establish_connection( adapter: 'sqlite3', @@ -36,6 +40,11 @@ raise "Unknown ENV['DB']: '#{db}'" end +# Check in spec if it's a valid adapter +def valid_db_adapter? + ENV['DB'] == 'pg' || ENV['DB'] == 'mysql' +end + require 'schema' require 'models' require 'fact_models' From 4553066bd7953215979134bd0a50eb8d7933f675 Mon Sep 17 00:00:00 2001 From: andresgutgon Date: Mon, 15 Oct 2018 11:23:49 +0200 Subject: [PATCH 2/2] Fixed bug introduced on PR: 16 --- lib/active_reporting/reporting_dimension.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_reporting/reporting_dimension.rb b/lib/active_reporting/reporting_dimension.rb index 4a9814a..ff8fb33 100644 --- a/lib/active_reporting/reporting_dimension.rb +++ b/lib/active_reporting/reporting_dimension.rb @@ -26,7 +26,7 @@ def self.label_config(label) return { label: label } unless label.is_a?(Hash) { - label: label[:field], + label: label[:label], label_name: label[:name] } end