fix: _insert_record failed for other adapters (#298)

* fix: _insert_record failed for other adapters

The _insert_record method would fail for adapters other than Spanner,
as it would call super with two arguments. This is now changed so the
method checks whether the current connection uses Spanner or some other
adapter, and based on that calls the super method with 1 or 2 arguments.

Fixes #292

* chore: fix rubocop issue

* fix: also call correct super for spanner adapter

Gemfile

@@ -8,6 +8,8 @@ gem "minitest", "~> 5.20.0"
 gem "minitest-rg", "~> 5.3.0"
 gem "pry", "~> 0.13.0"
 gem "pry-byebug", "~> 3.9.0"
+# Add sqlite3 for testing for compatibility with other adapters.
+gem "sqlite3"
 
 # Required for samples and testing.
 install_if -> { ENV.fetch("AR_VERSION", "~> 6.1.6.1").dup.to_s.sub("~>", "").strip < "7.1.0" && !ENV["SKIP_COMPOSITE_PK"] } do

lib/activerecord_spanner_adapter/base.rb

@@ -49,7 +49,10 @@ def self.buffered_mutations?
     end
 
     def self._insert_record values, returning = []
-      return super unless buffered_mutations? || (primary_key && values.is_a?(Hash))
+      if !(buffered_mutations? || (primary_key && values.is_a?(Hash))) || !spanner_adapter?
+        return super values if ActiveRecord.gem_version < VERSION_7_1
+        return super
+      end
 
       # Mutations cannot be used in combination with a sequence, as mutations do not support a THEN RETURN clause.
       if buffered_mutations? && sequence_name
@@ -59,6 +62,10 @@ def self._insert_record values, returning = []
 
       return _buffer_record values, :insert, returning if buffered_mutations?
 
+      _insert_record_dml values, returning
+    end
+
+    def self._insert_record_dml values, returning
       primary_key_value = _set_primary_key_value values
       if ActiveRecord::VERSION::MAJOR >= 7
         im = Arel::InsertManager.new arel_table

test/activerecord_spanner_mock_server/models/other_adapter.rb

@@ -0,0 +1,20 @@
+# Copyright 2024 Google LLC
+#
+# Use of this source code is governed by an MIT-style
+# license that can be found in the LICENSE file or at
+# https://opensource.org/licenses/MIT.
+
+module MockServerTests
+  class System < ActiveRecord::Base
+    has_many :projects
+  end
+
+  class Plan < ActiveRecord::Base
+    has_many :projects
+  end
+
+  class Project < ActiveRecord::Base
+    belongs_to :system
+    belongs_to :plan
+  end
+end

test/activerecord_spanner_mock_server/spanner_active_record_with_mock_server_test.rb

@@ -6,7 +6,9 @@
 #
 # frozen_string_literal: true
 
+require "sqlite3"
 require_relative "./base_spanner_mock_server_test"
+require_relative "models/other_adapter"
 
 module MockServerTests
   CommitRequest = Google::Cloud::Spanner::V1::CommitRequest
@@ -21,6 +23,35 @@ def test_insert
       assert_raises(NotImplementedError) { Singer.insert(singer) }
     end
 
+    def test_insert_other_adapter
+      ActiveRecord::Base.establish_connection(
+        adapter: "sqlite3",
+        database: ":memory:",
+      )
+      ActiveRecord::Base.logger = nil
+      ActiveRecord::Schema.define(version: 1) do
+        create_table :systems do |t|
+          t.string  :name
+        end
+
+        create_table :plans do |t|
+          t.string  :name
+        end
+
+        # simulate a join table with no primary key
+        create_table :projects, id: false do |t|
+          t.integer  :plan_id
+          t.integer  :system_id
+          t.index [:plan_id, :system_id], unique: true
+        end
+      end
+
+      system = System.create(name: "Baroque")
+      plan = Plan.create(name: "Music")
+      # This would previously fail, as the table has no primary key.
+      Project.create(plan_id: plan.id, system_id: system.id)
+    end
+
     def test_insert!
       singer = { first_name: "Alice", last_name: "Ecila" }
       singer = Singer.insert! singer