Model#copy bypasses private method visibility

[#695 state:resolved]

lib/dm-core/model.rb

@@ -416,9 +416,9 @@ def create!(attributes = {})
 
     # Copy a set of records from one repository to another.
     #
-    # @param [String] source
+    # @param [String] source_repository_name
     #   The name of the Repository the resources should be copied _from_
-    # @param [String] destination
+    # @param [String] target_repository_name
     #   The name of the Repository the resources should be copied _to_
     # @param [Hash] query
     #   The conditions with which to find the records to copy. These
@@ -428,16 +428,23 @@ def create!(attributes = {})
     #   A Collection of the Resource instances created in the operation
     #
     # @api public
-    def copy(source, destination, query = {})
+    def copy(source_repository_name, target_repository_name, query = {})
+      target_properties = properties(target_repository_name)
 
-      # get the list of properties that exist in the source and destination
-      destination_properties = properties(destination)
-      fields = query[:fields] ||= properties(source).select { |property| destination_properties.include?(property) }
+      query[:fields] ||= properties(source_repository_name).select do |property|
+        target_properties.include?(property)
+      end
+
+      repository(target_repository_name) do |repository|
+        resources = []
 
-      repository(destination) do
-        all(query.merge(:repository => source)).map do |resource|
-          create(fields.map { |property| [ property.name, property.get(resource) ] }.to_hash)
+        all(query.merge(:repository => source_repository_name)).each do |resource|
+          new_resource = new
+          query[:fields].each { |property| property.set(new_resource, property.get(resource)) }
+          resources << new_resource if new_resource.save
         end
+
+        all(Query.target_query(repository, self, resources))
       end
     end
 

lib/dm-core/property.rb

@@ -904,7 +904,6 @@ def determine_visibility
       @writer_visibility = @options[:writer] || @options[:accessor] || :public
     end
 
-
     # Typecast a value to an Integer
     #
     # @param [#to_str, #to_i] value

spec/public/model_spec.rb

@@ -7,11 +7,11 @@ module ::Blog
       class Article
         include DataMapper::Resource
 
-        property :id,      Serial
-        property :title,   String, :nullable => false
-        property :content, Text
+        property :id,       Serial
+        property :title,    String, :nullable => false
+        property :content,  Text,                       :writer => :private, :default => lambda { |resource, property| resource.title }
         property :subtitle, String
-        property :author,  String, :nullable => false
+        property :author,   String, :nullable => false
 
         belongs_to :original, self, :nullable => true
         has n, :revisions, self, :child_key => [ :original_id ]
@@ -26,9 +26,9 @@ class Article
     before :all do
       @author = 'Dan Kubb'
 
-      @original = @article_model.create(:title => 'Original Article',                                               :author => @author)
-      @article  = @article_model.create(:title => 'Sample Article',   :content => 'Sample', :original => @original, :author => @author)
-      @other    = @article_model.create(:title => 'Other Article',    :content => 'Other',                          :author => @author)
+      @original = @article_model.create(:title => 'Original Article',                         :author => @author)
+      @article  = @article_model.create(:title => 'Sample Article',   :original => @original, :author => @author)
+      @other    = @article_model.create(:title => 'Other Article',                            :author => @author)
     end
 
     it { @article_model.should respond_to(:copy) }
@@ -40,8 +40,8 @@ class Article
             @return = @resources = @article_model.copy(@repository.name, @alternate_adapter.name)
           end
 
-          it 'should return an Enumerable' do
-            @return.should be_a_kind_of(Enumerable)
+          it 'should return a Collection' do
+            @return.should be_a_kind_of(DataMapper::Collection)
           end
 
           it 'should return Resources' do
@@ -84,8 +84,8 @@ class Article
             @return = @resources = @article_model.copy(@alternate_adapter.name, :default)
           end
 
-          it 'should return an Enumerable' do
-            @return.should be_a_kind_of(Enumerable)
+          it 'should return a Collection' do
+            @return.should be_a_kind_of(DataMapper::Collection)
           end
 
           it 'should return Resources' do