diff --git a/app/models/synchronization/member.rb b/app/models/synchronization/member.rb index 8d30a1cc20..f30a54b97d 100644 --- a/app/models/synchronization/member.rb +++ b/app/models/synchronization/member.rb @@ -288,12 +288,15 @@ module CartoDB end def get_connector + # get_runner passes then syncrhonization modified_at to the downloader to the runner, + # but here we must pass it directly to the connector runner CartoDB::Importer2::ConnectorRunner.check_availability!(user) CartoDB::Importer2::ConnectorRunner.new( service_item_id, user: user, pg: pg_options, - log: log + log: log, + modified_at: modified_at ) end diff --git a/lib/carto/connector.rb b/lib/carto/connector.rb index ccd3de7f00..ffc24ddacb 100644 --- a/lib/carto/connector.rb +++ b/lib/carto/connector.rb @@ -65,6 +65,10 @@ module Carto @provider.remote_data_updated? end + def last_modified + @provider.last_modified + end + def table_name @provider.table_name end diff --git a/lib/carto/connector/provider.rb b/lib/carto/connector/provider.rb index 922aed2b1d..f2d44cbb97 100644 --- a/lib/carto/connector/provider.rb +++ b/lib/carto/connector/provider.rb @@ -34,10 +34,12 @@ module Carto true end - def initialize(parameters: {}, user: nil, logger: nil) + def initialize(parameters: {}, user: nil, logger: nil, modified_at: nil) @params = Parameters.new(parameters, required: required_parameters + [:provider], optional: optional_parameters) @user = user @logger = logger + # modified_at is the time at which previous synchronization data was modified + @modified_at = modified_at end def errors(only_for: nil) @@ -82,7 +84,13 @@ module Carto end def remote_data_updated? - must_be_defined_in_derived_class + # By default connectors don't check for updated data + true + end + + def last_modified + # By default connectors don't support last modified time + nil end # Name of the table to be imported diff --git a/services/importer/lib/importer/connector_runner.rb b/services/importer/lib/importer/connector_runner.rb index 5c05c3e2e5..366a8495f6 100644 --- a/services/importer/lib/importer/connector_runner.rb +++ b/services/importer/lib/importer/connector_runner.rb @@ -26,6 +26,7 @@ module CartoDB @log = options[:log] || new_logger @job = options[:job] || new_job(@log, @pg_options) @user = options[:user] + @previous_modified_at = options[:modified_at] @collision_strategy = options[:collision_strategy] @georeferencer = options[:georeferencer] || new_georeferencer(@job) @@ -33,7 +34,7 @@ module CartoDB @unique_suffix = @id.delete('-') @json_params = JSON.parse(connector_source) extract_params - @connector = Carto::Connector.new(parameters: @params, user: @user, logger: @log) + @connector = Carto::Connector.new(parameters: @params, user: @user, logger: @log, modified_at: modified_at) @results = [] @tracker = nil @stats = {} @@ -103,6 +104,7 @@ module CartoDB def last_modified # This method is needed to make the interface of ConnectorRunner compatible with Runner, # but we have no meaningful data to return here. + @connector.last_modified end def provider_name diff --git a/services/importer/spec/doubles/connector.rb b/services/importer/spec/doubles/connector.rb index cd59a3aefd..7bdc9ca13f 100644 --- a/services/importer/spec/doubles/connector.rb +++ b/services/importer/spec/doubles/connector.rb @@ -86,10 +86,22 @@ class DummyConnectorProvider < Carto::Connector::Provider end end +class DummyConnectorProviderWithModifiedDate < DummyConnectorProvider + metadata id: 'dummy_with_modified_date', name: 'DummyWithModifiedDate' + LAST_MODIFIED = Time.new(2020, 6, 16) + def remote_data_updated? + @modified_at.blank? || last_modified > @modified_at + end + + def last_modified + LAST_MODIFIED + end +end + def dummy_connector_provider_with_id(id, name=nil, features=DummyConnectorProvider::DEFAULT_FEATURES) Class.new(DummyConnectorProvider) do metadata id: id, name: name || id @copies = [] define_method(:features_information){ features } end -end \ No newline at end of file +end diff --git a/services/importer/spec/unit/connector_spec.rb b/services/importer/spec/unit/connector_spec.rb index 6afd65ef54..d91d61294e 100644 --- a/services/importer/spec/unit/connector_spec.rb +++ b/services/importer/spec/unit/connector_spec.rb @@ -15,6 +15,7 @@ describe Carto::Connector do Carto::Connector::PROVIDERS << DummyConnectorProvider Carto::Connector::PROVIDERS << dummy_connector_provider_with_id('another_dummy') Carto::Connector::PROVIDERS << dummy_connector_provider_with_id('third_dummy') + Carto::Connector::PROVIDERS << DummyConnectorProviderWithModifiedDate Carto::Connector.providers.keys.each do |provider_name| Carto::ConnectorProvider.create! name: provider_name end @@ -41,6 +42,7 @@ describe Carto::Connector do Carto::Connector.providers(user: @user).should == { "dummy" => { name: "Dummy", enabled: true, description: nil }, "another_dummy" => { name: "another_dummy", enabled: false, description: nil }, + "dummy_with_modified_date" => { name: "DummyWithModifiedDate", enabled: false, description: nil }, "third_dummy" => { name: "third_dummy", enabled: false, description: nil } } end @@ -58,6 +60,7 @@ describe Carto::Connector do Carto::Connector.providers(user: @user).should == { "dummy" => { name: "Dummy", enabled: true, description: nil }, "another_dummy" => { name: "another_dummy", enabled: false, description: nil }, + "dummy_with_modified_date" => { name: "DummyWithModifiedDate", enabled: false, description: nil }, "third_dummy" => { name: "third_dummy", enabled: false, description: nil } } end @@ -115,4 +118,43 @@ describe Carto::Connector do Carto::Connector.new(parameters: parameters, user: @user, logger: @fake_log) }.to raise_error(Carto::Connector::InvalidParametersError) end + + it 'By default providers consider data modified' do + parameters = { + provider: 'dummy', + table: 'thetable', + req1: 'a', + req2: 'b', + opt1: 'c' + } + future = Time.now + 1 + past = Time.new(0) + connector = Carto::Connector.new(parameters: parameters, user: @user, logger: @fake_log) + connector.remote_data_updated?.should eq true + connector = Carto::Connector.new(parameters: parameters, user: @user, logger: @fake_log, modified_at: future) + connector.remote_data_updated?.should eq true + connector = Carto::Connector.new(parameters: parameters, user: @user, logger: @fake_log, modified_at: past) + connector.remote_data_updated?.should eq true + end + + it 'Providers can detect data modifications' do + parameters = { + provider: DummyConnectorProviderWithModifiedDate.provider_id, + table: 'thetable', + req1: 'a', + req2: 'b', + opt1: 'c' + } + same_date = DummyConnectorProviderWithModifiedDate::LAST_MODIFIED + prior_date = same_date - 1 + posterior_date = same_date + 1 + connector = Carto::Connector.new(parameters: parameters, user: @user, logger: @fake_log) + connector.remote_data_updated?.should eq true + connector = Carto::Connector.new(parameters: parameters, user: @user, logger: @fake_log, modified_at: prior_date) + connector.remote_data_updated?.should eq true + connector = Carto::Connector.new(parameters: parameters, user: @user, logger: @fake_log, modified_at: posterior_date) + connector.remote_data_updated?.should eq false + connector = Carto::Connector.new(parameters: parameters, user: @user, logger: @fake_log, modified_at: same_date) + connector.remote_data_updated?.should eq false + end end