Refactored Resource and Collection reloading

* Refactored Collection#reload
* Refactored Collection#default_attributes
* Updated Resource#reload to only reload a Resource with a key
* Updated Resource#attributes to return a Hash with only the loaded
  properties in a new resource.
* Updated Resource#query to return a Query with the :fields set to all
  the loaded fields in the Resource.
* Removed Collection#properties and Collection#relationships from the
  semipublic api.

lib/dm-core/collection.rb

@@ -75,15 +75,8 @@ def reload(query = nil)
         identity_map[resource.key] = resource
       end
 
-      properties = model.properties(repository.name)
-      fields     = properties.key | query.fields
-
-      if discriminator = properties.discriminator
-        fields |= [ discriminator ]
-      end
-
       # sort fields based on declared order, for more consistent reload queries
-      fields = properties & fields
+      fields = properties & (query.fields | model_key | [ properties.discriminator ].compact)
 
       # replace the list of resources
       replace(all(query.update(:fields => fields, :reload => true)))
@@ -108,7 +101,7 @@ def reload(query = nil)
     #
     # @api public
     def get(*key)
-      key = model.key(repository.name).typecast(key)
+      key = model_key.typecast(key)
 
       @identity_map[key] || if !loaded? && (query.limit || query.offset > 0)
         # current query is exclusive, find resource within the set
@@ -901,27 +894,6 @@ def inspect
       "[#{map { |resource| resource.inspect }.join(', ')}]"
     end
 
-    # Returns the PropertySet representing the fields in the Collection scope
-    #
-    # @return [PropertySet]
-    #   The set of properties this Collection's query will retrieve
-    #
-    # @api semipublic
-    def properties
-      PropertySet.new(query.fields)
-    end
-
-    # Returns the Relationships for the Collection's Model
-    #
-    # @return [Hash]
-    #   The model's relationships, mapping the name to the
-    #   Associations::Relationship object
-    #
-    # @api semipublic
-    def relationships
-      model.relationships(repository.name)
-    end
-
     def hash
       query.hash
     end
@@ -1042,6 +1014,47 @@ def loaded_entries
       loaded? ? self : head + tail
     end
 
+    # Returns the Query Repository name
+    #
+    # @return [Symbol]
+    #   the repository name
+    #
+    # @api private
+    def repository_name
+      repository.name
+    end
+
+    # Returns the model key
+    #
+    # @return [PropertySet]
+    #   the model key
+    #
+    # @api private
+    def model_key
+      model.key(repository_name)
+    end
+
+    # Returns the PropertySet representing the fields in the Collection scope
+    #
+    # @return [PropertySet]
+    #   The set of properties this Collection's query will retrieve
+    #
+    # @api private
+    def properties
+      model.properties(repository_name)
+    end
+
+    # Returns the Relationships for the Collection's Model
+    #
+    # @return [Hash]
+    #   The model's relationships, mapping the name to the
+    #   Associations::Relationship object
+    #
+    # @api private
+    def relationships
+      model.relationships(repository_name)
+    end
+
     # Initializes a new Collection
     #
     # @return [Collection]
@@ -1125,23 +1138,20 @@ def default_attributes
 
       conditions = query.conditions
 
-      if conditions.kind_of?(Query::Conditions::AndOperation)
-        repository_name = repository.name
-        relationships   = self.relationships.values
-        properties      = model.properties(repository_name)
-        key             = model.key(repository_name)
+      if conditions.slug == :and
+        properties = self.properties.dup
 
-        # if all the key properties are included in the conditions,
-        # then do not allow them to be default attributes
-        if query.condition_properties.to_set.superset?(key.to_set)
-          properties -= key
+        if query.condition_properties.to_set.superset?(model_key.to_set)
+          properties -= model_key
         end
 
         conditions.each do |condition|
-          if condition.kind_of?(Query::Conditions::EqualToComparison) &&
-            (properties.include?(condition.subject) || (condition.relationship? && condition.subject.source_model == model))
-            default_attributes[condition.subject] = condition.value
-          end
+          next unless condition.slug == :eql
+
+          subject = condition.subject
+          next unless properties.include?(subject) || (condition.relationship? && subject.source_model == model)
+
+          default_attributes[condition.subject] = condition.value
         end
       end
 

lib/dm-core/resource.rb

@@ -248,9 +248,10 @@ def attribute_set(name, value)
     #
     # @api public
     def attributes(key_on = :name)
-      lazy_load(properties)
       attributes = {}
-      properties.each do |property|
+
+      lazy_load(properties)
+      fields.each do |property|
         if model.public_method_defined?(name = property.name)
           key = case key_on
             when :name  then name
@@ -261,6 +262,7 @@ def attributes(key_on = :name)
           attributes[key] = __send__(name)
         end
       end
+
       attributes
     end
 
@@ -296,7 +298,7 @@ def attributes=(attributes)
     # @api public
     def reload
       if saved?
-        eager_load(loaded_properties)
+        eager_load(fields)
         child_collections.each { |children| children.reload }
       end
 
@@ -741,34 +743,36 @@ def identity_map
     #   names of attributes that have been loaded
     #
     # @api private
-    def loaded_properties
-      properties.select { |property| property.loaded?(self) }
+    def fields
+      properties.select do |property|
+        property.loaded?(self) || (new? && property.default?)
+      end
     end
 
     # Lazy loads attributes not yet loaded
     #
-    # @param [Array<Property>] fields
+    # @param [Array<Property>] properties
     #   the properties to reload
     #
     # @return [self]
     #
     # @api private
-    def lazy_load(fields)
-      eager_load(fields - loaded_properties)
+    def lazy_load(properties)
+      eager_load(properties - fields)
     end
 
     # Reloads specified attributes
     #
-    # @param [Array<Property>] fields
+    # @param [Array<Property>] properties
     #   the properties to reload
     #
     # @return [Resource]
     #   the receiver, the current Resource instance
     #
     # @api private
-    def eager_load(fields)
-      unless fields.empty? || new?
-        collection.reload(:fields => fields)
+    def eager_load(properties)
+      unless properties.empty? || key.nil?
+        collection.reload(:fields => properties)
       end
 
       self
@@ -781,7 +785,7 @@ def eager_load(fields)
     #
     # @api private
     def query
-      Query.new(repository, model, model.key_conditions(repository, key))
+      Query.new(repository, model, model.key_conditions(repository, key).update(:fields => fields))
     end
 
     # Return a collection including the current resource only

spec/public/shared/resource_shared_spec.rb

@@ -187,11 +187,21 @@
   it { @user.should respond_to(:attributes) }
 
   describe '#attributes' do
+    describe 'with a new resource' do
+      before :all do
+        @user = @user.model.new
+      end
 
-    it 'should return the expected values' do
-      @user.attributes.only(:name, :description, :age).should == { :name => 'dbussink', :description => 'Test', :age => 25 }
+      it 'should return the expected values' do
+        @user.attributes.should == {}
+      end
     end
 
+    describe 'with a saved resource' do
+      it 'should return the expected values' do
+        @user.attributes.only(:name, :description, :age).should == { :name => 'dbussink', :description => 'Test', :age => 25 }
+      end
+    end
   end
 
   it { @user.should respond_to(:attributes=) }
@@ -500,8 +510,7 @@
   it { @user.should respond_to(:reload) }
 
   describe '#reload' do
-    describe 'for a single resource' do
-
+    describe 'on a resource' do
       before :all do
         rescue_if @skip do
           @user.name = 'dkubb'
@@ -517,7 +526,7 @@
       end
     end
 
-    describe 'for when the resource is changed outside another resource' do
+    describe 'on a resource that is changed outside another resource' do
       before :all do
         rescue_if @skip do
           @user2 = @user.dup
@@ -531,6 +540,29 @@
         @user.description.should eql('Changed')
       end
     end
+
+    describe 'on an anonymous resource' do
+      before :all do
+        rescue_if @skip do
+          @user = @user.model.first(:fields => [ :description ])
+          @user.description.should == 'Test'
+
+          @return = @user.reload
+        end
+      end
+
+      it 'should return a Resource' do
+        @return.should be_kind_of(DataMapper::Resource)
+      end
+
+      it 'should return the expected value' do
+        @return.should equal(@user)
+      end
+
+      it 'should not reload any other attributes' do
+        @user.attributes.should == { :description => 'Test' }
+      end
+    end
   end
 
   it { @user.should respond_to(:readonly?) }

spec/semipublic/collection_spec.rb

@@ -74,22 +74,6 @@ class Article
         end
       end
 
-      it { @articles.should respond_to(:properties) }
-
-      describe '#properties' do
-        before :all do
-          @return = @properties = @articles.properties
-        end
-
-        it 'should return a PropertySet' do
-          @return.should be_kind_of(DataMapper::PropertySet)
-        end
-
-        it 'should be expected properties' do
-          @properties.to_a.should == @articles_query.fields
-        end
-      end
-
       it { @articles.should respond_to(:query) }
 
       describe '#query' do
@@ -106,22 +90,6 @@ class Article
         end
       end
 
-      it { @articles.should respond_to(:relationships) }
-
-      describe '#relationships' do
-        before :all do
-          @return = @relationships = @articles.relationships
-        end
-
-        it 'should return a Hash' do
-          @return.should be_kind_of(Hash)
-        end
-
-        it 'should return expected Hash' do
-          @return.should equal(@article_model.relationships(@article_repository.name))
-        end
-      end
-
       it { @articles.should respond_to(:repository) }
 
       describe '#repository' do