#695 ✓resolved
Shane Hanna

attributes= now respects method visibility change breaks Model#copy

Reported by Shane Hanna | December 4th, 2008 @ 07:57 PM

http://github.com/sam/dm-core/co...

Model#copy does a create(resource.attributes). #attributes returns an attribute when the reader (not writer) is public.

This means you get an error on Model#copy now when your reader is public but your writer isn't.

I'm not sure what to fix so no patch or spec this time.

Comments and changes to this ticket

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) December 4th, 2008 @ 11:54 PM

    • State changed from “unconfirmed” to “confirmed”

    I'm thinking that since Model#copy is a special case, we shouldn't use Model#create(resource.attributes) and should instead do it the (slightly) longer form:

    
    def copy(source, destination, query = {})
      properties = properties(source)
    
      repository(destination) do
        all(:repository => source).each do |resource|
          copy = new
          properties.each { |p| p.set!(copy, p.get!(resource)) }
          copy.save
        end
      end
    end
    

    This is actually probably better anyway. I don't like to have methods access the read_many method in the adapter anyway. It's preferable to route all requests for it through Model#all anyway.

    Shane, would you mind putting together a patch for this with specs, if it's not too much trouble? Probably the biggest reason for this breaking is that the Model#copy method is pretty much unspecced. It would be a good idea to spec normal behavior, as well as making sure that private attributes are copied over (the above code should work for privates), so this regression doesn't happen again. At the same time, I wouldn't spend a ton of time on the specs, just a few of the common cases, since we're updating Model#copy in dkubb/dm-core, and rewriting the specs using a specific style that probably doesn't mesh well with the sam/dm-core specs.

  • Shane Hanna

    Shane Hanna December 5th, 2008 @ 12:16 AM

    Yep. It's already passed knock-off time here though and I'm off for a couple cold ones down the pub.

    If it's not patched by the time I sober up I'll do it :)

  • Shane Hanna

    Shane Hanna December 8th, 2008 @ 01:31 AM

    I'm a little unsure if the tests are any good or even in the right spot.

    I've never really used RSpec before (the indirect .should calls smell funny to me) plus private attribute support for #copy is only implied by the other tests success.

    http://github.com/shanna/dm-core...

    Shane.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) January 4th, 2009 @ 11:38 PM

    • State changed from “confirmed” to “resolved”

    Ticket #742 is a dupe of this, and is now resolved.

  • Shane Hanna

    Shane Hanna January 5th, 2009 @ 09:40 PM

    • Assigned user changed from “Jonathan Stott (namelessjon)” to “Dan Kubb (dkubb)”

    My mistake, ticket #742 doesn't resolve the protected attributes issue I don't think.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) January 5th, 2009 @ 11:34 PM

    • State changed from “resolved” to “accepted”

    Shane, I mispoke and I think you are right. Let me see what I can do to make it totally bypass private and protected accessors and mutators.

  • Jonathan Stott (namelessjon)

    Jonathan Stott (namelessjon) February 22nd, 2009 @ 11:50 AM

    • Tag changed from dm-core to dm-core, one_file_test

    This is still an issue as of 02a93b11ce576361789340f78babf7bd4f796161. See attached one file test.

  • Dan Kubb (dkubb)

    Dan Kubb (dkubb) November 10th, 2009 @ 02:00 AM

    • State changed from “accepted” to “resolved”

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Attachments

Referenced by

Pages