Merge #176

176: Fix primary_key customization r=brunoocasali a=brunoocasali

Fixes #147 

Co-authored-by: Bruno Casali <[email protected]>

Author: bors[bot]

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2022-07-04 10:10:48 UTC using RuboCop version 1.27.0.
+# on 2022-07-22 20:48:41 UTC using RuboCop version 1.27.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -70,7 +70,7 @@ Metrics/MethodLength:
 # Offense count: 1
 # Configuration parameters: CountComments, CountAsOne.
 Metrics/ModuleLength:
-  Max: 431
+  Max: 441
 
 # Offense count: 9
 # Configuration parameters: IgnoredMethods.
@@ -108,7 +108,7 @@ RSpec/DescribeClass:
   Exclude:
     - 'spec/integration_spec.rb'
 
-# Offense count: 27
+# Offense count: 29
 # Configuration parameters: CountAsOne.
 RSpec/ExampleLength:
   Max: 19
@@ -185,7 +185,7 @@ Style/RescueModifier:
   Exclude:
     - 'lib/meilisearch-rails.rb'
 
-# Offense count: 13
+# Offense count: 15
 # This cop supports safe auto-correction (--auto-correct).
 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
 # URISchemes: http, https

README.md

@@ -318,15 +318,33 @@ end
 
 By default, the primary key is based on your record's id. You can change this behavior by specifying the `primary_key:` option.
 
-Note that the primary key must have a **unique value**.
+Note that the primary key must return a **unique value** otherwise your data could be overwritten.
 
 ```ruby
 class Book < ActiveRecord::Base
   include MeiliSearch::Rails
 
-  meilisearch primary_key: 'ISBN'
+  meilisearch primary_key: :isbn # isbn is a column in your table definition.
 end
 ```
+
+You can also set the `primary_key` as a method, this method will be evaluated in runtime, and its return 
+will be used as the reference to the document when Meilisearch needs it.
+
+```rb
+class Book < ActiveRecord::Base
+  include MeiliSearch::Rails
+
+  meilisearch primary_key: :my_custom_ms_id
+
+  private
+
+  def my_custom_ms_id
+    "isbn_#{primary_key}" # ensure this return is unique, otherwise you'll lose data.
+  end
+end
+```
+
 #### Conditional indexing
 
 You can control if a record must be indexed by using the `if:` or `unless:` options.<br>

lib/meilisearch-rails.rb

@@ -613,16 +613,30 @@ def ms_search(query, params = {})
         #  "processingTimeMs"=>0, "query"=>"iphone"}
         json = ms_raw_search(query, params)
 
-        # Returns the ids of the hits: 13
-        hit_ids = json['hits'].map { |hit| hit[ms_pk(meilisearch_options).to_s] }
-
         # condition_key gets the primary key of the document; looks for "id" on the options
         condition_key = if defined?(::Mongoid::Document) && include?(::Mongoid::Document)
                           ms_primary_key_method.in
                         else
                           ms_primary_key_method
                         end
 
+        # The condition_key must be a valid column otherwise, the `.where` below will not work
+        # Since we provide a way to customize the primary_key value, `ms_pk(meilisearch_options)` may not
+        # respond with a valid database column. The blocks below prevent that from happening.
+        has_virtual_column_as_pk = if defined?(::Sequel::Model) && self < Sequel::Model
+                                     meilisearch_options[:type].columns.map(&:to_s).exclude?(condition_key.to_s)
+                                   else
+                                     meilisearch_options[:type].columns.map(&:name).map(&:to_s).exclude?(condition_key.to_s)
+                                   end
+
+        condition_key = meilisearch_options[:type].primary_key if has_virtual_column_as_pk
+
+        hit_ids = if has_virtual_column_as_pk
+                    json['hits'].map { |hit| hit[condition_key] }
+                  else
+                    json['hits'].map { |hit| hit[ms_pk(meilisearch_options).to_s] }
+                  end
+
         # meilisearch_options[:type] refers to the Model name (e.g. Product)
         # results_by_id creates a hash with the primaryKey of the document (id) as the key and doc itself as the value
         # {"13"=>#<Product id: 13, name: "iphone", href: "apple", tags: nil, type: nil,

spec/integration_spec.rb

@@ -219,7 +219,7 @@ def will_save_change_to_full_name?
 class Cat < ActiveRecord::Base
   include MeiliSearch::Rails
 
-  meilisearch index_uid: safe_index_uid('animals'), id: :ms_id do
+  meilisearch index_uid: safe_index_uid('animals'), synchronous: true, primary_key: :ms_id do
   end
 
   private
@@ -232,7 +232,7 @@ def ms_id
 class Dog < ActiveRecord::Base
   include MeiliSearch::Rails
 
-  meilisearch index_uid: safe_index_uid('animals'), id: :ms_id do
+  meilisearch index_uid: safe_index_uid('animals'), synchronous: true, primary_key: :ms_id do
   end
 
   private
@@ -359,14 +359,14 @@ class Model < ActiveRecord::Base
 class UniqUser < ActiveRecord::Base
   include MeiliSearch::Rails
 
-  meilisearch synchronous: true, index_uid: safe_index_uid('UniqUser'), id: :name do
+  meilisearch synchronous: true, index_uid: safe_index_uid('UniqUser'), primary_key: :name do
   end
 end
 
 class NullableId < ActiveRecord::Base
   include MeiliSearch::Rails
 
-  meilisearch synchronous: true, index_uid: safe_index_uid('NullableId'), id: :custom_id,
+  meilisearch synchronous: true, index_uid: safe_index_uid('NullableId'), primary_key: :custom_id,
               if: :never do
   end
 
@@ -1370,13 +1370,24 @@ class SerializedDocument < ActiveRecord::Base
 end
 
 describe 'Animals' do
+  it 'returns only the requested type' do
+    Dog.create!([{ name: 'Toby the Dog' }, { name: 'Felix the Dog' }])
+    Cat.create!([{ name: 'Toby the Cat' }, { name: 'Felix the Cat' }, { name: 'roar' }])
+
+    expect(Dog.count).to eq(2)
+    expect(Cat.count).to eq(3)
+
+    expect(Cat.search('felix').size).to eq(1)
+    expect(Cat.search('felix').first.name).to eq('Felix the Cat')
+    expect(Dog.search('toby').size).to eq(1)
+    expect(Dog.search('Toby').first.name).to eq('Toby the Dog')
+  end
+
   it 'shares a single index' do
-    Dog.create!(name: 'Toby')
-    Cat.create!(name: 'Felix')
-    index = MeiliSearch::Rails.client.index(safe_index_uid('animals'))
-    index.wait_for_task(index.tasks['results'].first['uid'])
-    docs = index.search('')
-    expect(docs['hits'].size).to eq(2)
+    cat_index = Cat.index.instance_variable_get('@index').uid
+    dog_index = Dog.index.instance_variable_get('@index').uid
+
+    expect(cat_index).to eq(dog_index)
   end
 end