Trying to figure out what happens in _services_process_resource() is very hard - it sends in two variables by reference of which the $controllers one is only changed but never used in its caller _services_build_resources().

Are the changes made in _services_build_resources() reflected in $resources due to the data in $controllers being references to data in $resources? I actually don't know - might be - or might be something else weird that is going on.

CommentFileSizeAuthor
#1 1020414_services_overkill_reference_usage.patch1.24 KBvoxpelli

Comments

voxpelli’s picture

voxpelli commented
Status: Active » Needs review
StatusFileSize
new 1020414_services_overkill_reference_usage.patch1.24 KB
voxpelli’s picture

voxpelli commented

That was just a small fix - the big issue here was that it's very hard to understand what's happening and after digging through history and talking to Hugo I've concluded that the entire $controllers array in _services_process_resource() mostly is created to support an old 2.x services oriented services_auth_invoke('alter_methods', $methods); and could probably be removed and replaced by resources alone if all other API:s are updated.

kylebrowning’s picture

kylebrowning commented

I think this needs further discussion. After looking at this extensively it seems like this $controllers array is made so that it easy to loop through everything that exists and verify certain things about it. If you look at the resources array everything is broken up into its respective retrieve, actions, relationships etc. Retrieve should never have multiple controls, but actions,relationships, and targeted actions will. The _services_process_resource() builds thats controller array for us so that we can easily loop through We can definitely lose the reference, but I think removing this function for 3.x might not be needed just yet. One thing is for sure, this code is used to simplify and easily loop through every method that exists and determine certain this like access callbacks work, and files are using the prefered arrangement for module_load_include

Voxpelli, lets open a separate issue, and mark it postponed for 4.x.

Im running the tests on this right now and hopefully they all pass, if they doo ill port to 6.x commit and push, and then close this ticket.

marcingy’s picture

marcingy commented

I agree lets fix the what we have now and look at refractoring in 4.x, where hopefully we can simply the code.

kylebrowning’s picture

kylebrowning commented
Status: Needs review » Fixed

This has been fixed in 6.x and 7.x, both in 3.x

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.