[STORE] Reduce repeated string instantiation (#813)

Author: estolfo

Diff for: elasticsearch-persistence/lib/elasticsearch/persistence/repository/find.rb

@@ -7,6 +7,20 @@ class DocumentNotFound < StandardError; end
       #
       module Find
 
+        # The default type of document.
+        #
+        ALL = '_all'.freeze
+
+        # The key for accessing the document found and returned from an
+        #   Elasticsearch _mget query.
+        #
+        DOCS = 'docs'.freeze
+
+        # The key for the boolean value indicating whether a particular id
+        #   has been successfully found in an Elasticsearch _mget query.
+        #
+        FOUND = 'found'.freeze
+
         # Retrieve a single object or multiple objects from Elasticsearch by ID or IDs
         #
         # @example Retrieve a single object by ID
@@ -43,14 +57,14 @@ def find(*args)
         # @return [true, false]
         #
         def exists?(id, options={})
-          type     = document_type || (klass ? __get_type_from_class(klass) : '_all')
+          type     = document_type || (klass ? __get_type_from_class(klass) : ALL)
           client.exists( { index: index_name, type: type, id: id }.merge(options) )
         end
 
         # @api private
         #
         def __find_one(id, options={})
-          type     = document_type || (klass ? __get_type_from_class(klass) : '_all')
+          type     = document_type || (klass ? __get_type_from_class(klass) : ALL)
           document = client.get( { index: index_name, type: type, id: id }.merge(options) )
 
           deserialize(document)
@@ -61,10 +75,10 @@ def __find_one(id, options={})
         # @api private
         #
         def __find_many(ids, options={})
-          type     = document_type || (klass ? __get_type_from_class(klass) : '_all')
+          type     = document_type || (klass ? __get_type_from_class(klass) : ALL)
           documents = client.mget( { index: index_name, type: type, body: { ids: ids } }.merge(options) )
 
-          documents['docs'].map { |document| document['found'] ? deserialize(document) : nil }
+          documents[DOCS].map { |document| document[FOUND] ? deserialize(document) : nil }
         end
       end
 

Diff for: elasticsearch-persistence/lib/elasticsearch/persistence/repository/naming.rb

@@ -6,6 +6,10 @@ module Repository
       #
       module Naming
 
+        # The possible keys for a document id.
+        #
+        IDS = [:id, 'id', :_id, '_id'].freeze
+
         # Get or set the class used to initialize domain objects when deserializing them
         #
         def klass name=nil
@@ -89,7 +93,7 @@ def __get_type_from_class(klass)
         # @api private
         #
         def __get_id_from_document(document)
-          document[:id] || document['id'] || document[:_id] || document['_id']
+          document[IDS.find { |id| document[id] }]
         end
 
         # Extract a document ID from the document (assuming Hash or Hash-like object)
@@ -106,7 +110,13 @@ def __get_id_from_document(document)
         # @api private
         #
         def __extract_id_from_document(document)
-          document.delete(:id) || document.delete('id') || document.delete(:_id) || document.delete('_id')
+          IDS.inject(nil) do |deleted, id|
+            if document[id]
+              document.delete(id)
+            else
+              deleted
+            end
+          end
         end
       end
 

Diff for: elasticsearch-persistence/lib/elasticsearch/persistence/repository/response/results.rb

@@ -12,6 +12,18 @@ class Results
 
           attr_reader :repository
 
+          # The key for accessing the results in an Elasticsearch query response.
+          #
+          HITS = 'hits'.freeze
+
+          # The key for accessing the total number of hits in an Elasticsearch query response.
+          #
+          TOTAL = 'total'.freeze
+
+          # The key for accessing the maximum score in an Elasticsearch query response.
+          #
+          MAX_SCORE = 'max_score'.freeze
+
           # @param repository [Elasticsearch::Persistence::Repository::Class] The repository instance
           # @param response   [Hash]  The full response returned from the Elasticsearch client
           # @param options    [Hash]  Optional parameters
@@ -33,25 +45,25 @@ def respond_to?(method_name, include_private = false)
           # The number of total hits for a query
           #
           def total
-            response['hits']['total']
+            response[HITS][TOTAL]
           end
 
           # The maximum score for a query
           #
           def max_score
-            response['hits']['max_score']
+            response[HITS][MAX_SCORE]
           end
 
           # Yields [object, hit] pairs to the block
           #
           def each_with_hit(&block)
-            results.zip(response['hits']['hits']).each(&block)
+            results.zip(response[HITS][HITS]).each(&block)
           end
 
           # Yields [object, hit] pairs and returns the result
           #
           def map_with_hit(&block)
-            results.zip(response['hits']['hits']).map(&block)
+            results.zip(response[HITS][HITS]).map(&block)
           end
 
           # Return the collection of domain objects
@@ -64,7 +76,7 @@ def map_with_hit(&block)
           # @return [Array]
           #
           def results
-            @results ||= response['hits']['hits'].map do |document|
+            @results ||= response[HITS][HITS].map do |document|
               repository.deserialize(document.to_hash)
             end
           end

Diff for: elasticsearch-persistence/lib/elasticsearch/persistence/repository/search.rb

@@ -6,6 +6,10 @@ module Repository
       #
       module Search
 
+        # The key for accessing the count in a Elasticsearch query response.
+        #
+        COUNT = 'count'.freeze
+
         # Returns a collection of domain objects by an Elasticsearch query
         #
         # Pass the query either as a string or a Hash-like object
@@ -86,7 +90,7 @@ def count(query_or_definition=nil, options={})
             raise ArgumentError, "[!] Pass the search definition as a Hash-like object or pass the query as a String, not as [#{query_or_definition.class}]"
           end
 
-          response['count']
+          response[COUNT]
         end
       end
 

Diff for: elasticsearch-persistence/lib/elasticsearch/persistence/repository/serialize.rb

@@ -8,6 +8,16 @@ module Repository
       #
       module Serialize
 
+        # The key for document fields in an Elasticsearch query response.
+        #
+        SOURCE = '_source'.freeze
+
+        # The key for the document type in an Elasticsearch query response.
+        #   Note that it will be removed eventually, as multiple types in a single
+        #   index are deprecated as of Elasticsearch 6.0.
+        #
+        TYPE = '_type'.freeze
+
         # Serialize the object for storing it in Elasticsearch
         #
         # In the default implementation, call the `to_hash` method on the passed object.
@@ -21,11 +31,10 @@ def serialize(document)
         # Use the `klass` property, if defined, otherwise try to get the class from the document's `_type`.
         #
         def deserialize(document)
-          _klass = klass || __get_klass_from_type(document['_type'])
-          _klass.new document['_source']
+          _klass = klass || __get_klass_from_type(document[TYPE])
+          _klass.new document[SOURCE]
         end
       end
-
     end
   end
 end

Diff for: elasticsearch-persistence/test/unit/repository_naming_test.rb

@@ -50,7 +50,7 @@ module ::Foo; class Bar; end; end
     end
 
     context "extract an ID from the document" do
-      should "delete the key from theHash" do
+      should "delete the key from the Hash" do
         d1 = { :id   => 1 }
         d2 = { :_id  => 1 }
         d3 = { 'id'  => 1 }