Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When using recurse_over_arrays true, adding and populating hash in an array doesn't work in some cases #29

Open
abonas opened this issue Apr 10, 2015 · 3 comments
Labels

Comments

@abonas
Copy link

abonas commented Apr 10, 2015

Even when defining the object with recurse_over_arrays set to true, certain ways of filling the data in an array don't work. (the nicer , more object oriented ones)

Option A - works
obj.spec = {}
obj.spec.ports = [{ 'port' => 3000,
'targetPort' => 'http-server',
'protocol' => 'TCP'
}]

Option B - doesn't work
obj.spec = {}
obj.spec.ports = []
obj.spec.ports << { 'port' => 3000,
'targetPort' => 'http-server',
'protocol' => 'TCP'
}

Option C - doesn't work
obj.spec = {}
obj.spec.ports = []
obj.spec.ports[0]= {}
obj.spec.ports[0].port = 3000

@aetherknight
Copy link
Owner

Could you provide examples what the expected results are for each of your Options, and how they break down?

At present, the way recurse_over_arrays works is that the first time you call an accessor method that refers to an Array, it memoizes a copy of that array where each hash within the original array is replaced with an ROS. If you use << to mutate such an array, any hashes already in the original array will be converted to ROS, but any new hashes will only be placed in the memoized array, and they won't be converted into ROSes.

I believe the original purpose of the memoization was to ensure that two accessor method calls will return the same ROS object instead of returning a different one each time.

One way of solving Option B would be to subclass Array for use within ROS (to reimplement methods like <<), but I'm concerned that this will open yet another can of worms.

Another way might be to check the memoized array to see if it contains any hashes that have not been replaced yet.

I also suspect that modifications to the memoized array may not show up under certain circumstances when .to_h is called on the parent ROS object, or when .dup is used to create a deep-duplicate of the object.

@abonas
Copy link
Author

abonas commented Apr 12, 2015

@aetherknight, my expectation would be that option B and option C will work without getting a "no method error", so the user can build those structures in a nice object oriented way.

Consider a use case where the user constructs ROS without having a json file, but simply one attribute at a time, just as populating another class in an object oriented way.

@aetherknight
Copy link
Owner

I made a change to how the getter works so that it will always recompute the array that might contain ROS objects, and tested it against the 3 use-cases mentioned above. However, it does not modify the underlying hash @table such that the parent ROS objects @tables are properly modified. This means that to_hash on a parent ROS object is broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants