Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.NetConnection calls pass the arguments as an indexed array, eg. [value1, value2,… ]
whilst the AmfServerServicePoxy::execute() method checks for an associative array [key1 : value1, key2 : value2, …]
This causes a couple of issues:
1. optional arguments are added twice:
eg. arg1 (required), arg2 (optional), arg3 (optional)
when the call to the server has [arg1_value, arg2_value] (one required and one optional argument)
the argument array passed to the resource is:
arg[0] = arg1_value
arg[1] = arg2_value
arg[arg2_key] = arg2_default_value
arg[arg3_key] = arg3_default_value
when this is passed to the resource callback:
callback(arg1, arg2 = default_value, arg3 = default_value);
the actual arguments being fetched are:
callback(arg1_value, arg2_value, arg2_default_value);
2. certain resources expect an associative array, so an error occurs as it cannot find the required arguments
this has probably gone unnoticed due to the fact that resources anticipate on this, and check if the arguments are an indexed array
(as is the case for the node_resource access_callback)
the services_views module does not anticipate on this, and throws errors because it expects an associative array
…the underlying issue here may be that it is not clear what the server actually should fetch the resources…
anyway, the patch in attachment resolves this issue with the amfserver, it accepts indexed or associative arguments from the call, and passes the resource an associative array
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | amfserver_7.x-3.0-dev__arguments_handling.patch | 21.95 KB | g10 |
| amfserver_7.x-3.0-rc1_args-to-named-array.patch | 1.71 KB | g10 |
Comments
Comment #1
rolf vreijdenberger commentedthanks g10. good description, thanks for the patch. I'm planning an update beginning next week and will take this patch with it.
Comment #2
g10 commentedI guess the patch will have to be re-evaluated:
on one hand it fixes a bug where the arguments array became a mixed array with indexed and named items (because of the way the default value was checked)… so this part is still valid
on the other hand it changes the arguments array to a named array, which solved an issue with certain access callbacks (specifically the services_views one, the
node_resource access callback anticipates on an indexed array but in a not so elegant way, and services_menu expects an indexed one at the moment)… more detail here #1217602: Uniform service arguments declaration for services, for services_menu: #1222098: Invitation to merge with branch on GitHub
so the question of an indexed vs associative one remains… which in fact only causes an issue in the access callback of the resources
personally I would go for the associative one (think this is the type that the rest_server provides)
Comment #3
g10 commentedAfter re-evaluating the situation, and comparing with the rest_server (+ some pointers from kylebrowning #1217602: Uniform service arguments declaration)… I noticed that core resources are using an indexed array to declare the arguments and services_views + amfserver resources are using a named array, depending on what the resources defined, they expect an indexed or named array in their callback… thus both should be handled correctly
The original issue with the argument handling of the amfserver remains (mixed indexed with named), but changing this to a named array did not solve the issue entirely… therefore an implementation in line with the rest_server getControllerArguments was needed (argument array keeps the format as it was defined : indexed or named)
Patch in attachment implements this (tested w/ core + services_views + services_menu for all situations)
+ a bit of refactoring: resources.inc split in 3 files:
- resources.inc : actual resources (which I assume are solely for testing purposes)
- server.inc : actual server implementation (AmfServerServer and AmfServerServiceProxy)
- vo.inc : the user value object / class
Comment #4
g10 commentedComment #5
rolf vreijdenberger commentedHi g10,
thanks for your work, great that you have made a fix.
I would however like to ask you to do a fix only. Can you provide a patch for that?
A refactoring like you did will force me into doing a thorough review and might add complexity for me to maintain the project (because of time constraints)
Your assumptions are correct, resources and value object are purely for testing/examples.
At the moment, there is no need to separate it all, since it's a pretty simple module. Therefore, I'd like to keep it as simple _for me_ as possible.
I think you'll understand. Again, I appreciate what you did!
cheers
Comment #6
g10 commentedHi Rolf,
the refactoring is not much, if you would do a diff you would notice it is simply the same code split in 3 files…
(I'm a having the same issue, not having any spare time :/
Comment #7
rolf vreijdenberger commentedg10, thanks, issue is fixed. I gave you credit for the authorship, but have implemented some code myself based on yours.
great work!
version 7.x-3.03 released
Comment #8
rolf vreijdenberger commented