Make remote_data_updated mechanism usable for connectors

Connectors were missing access to the modified_at field of synchronizations;
for non-connector runner it was passed through the downloader.

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
 

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

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

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

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
+end

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