Function _services_build_resources excessive drupal_alter() [#1242008]

In function _services_build_resources we have two drupal_alter's that do completely the same:

drupal_alter('services_resources', $resources, $endpoint);

and about 20 lines below

drupal_alter('services_resources_post_processing', $resources, $endpoint);

As we don't change resources between these two calls I can't see reason to keep both hooks. Lets decide which one is not needed.

Comment	File	Size	Author
#9	0001-Comment-about-deprecated-hook_services_resources_pos.patch	1.1 KB	ygerasimov
#9
#7	0001-Comment-abou-deprecated-hook_services_resources_post.patch	986 bytes	ygerasimov
#7

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

marcingy CreditAttribution: marcingy commented 7 August 2011 at 15:35

Resources are altered in this code block as $resource is passed by reference.

// Process the resources, and collect all controllers in the process
  $controllers = array();
  foreach ($resources as $name => &$resource) {
    $controllers = _services_process_resource($name, $resource, $controllers);
  }

So I don't think anything needs to change here are the resource can change between the 2 alter calls.

Comment #2

ygerasimov CreditAttribution: ygerasimov commented 7 August 2011 at 16:31

@marcingy yes, I agree that we pass it by reference, but in function _services_process_resource we only add 'name' property to each of $resources. So not much things added at all.

Can we think about use case of both hooks? Or when we should use one or the other?

Comment #3

marcingy CreditAttribution: marcingy commented 9 August 2011 at 01:40

Status:

Active

» Postponed

The first alter allows me to amend resources before the controller get populated as the values in resource affects what the controller element looks like so it makes it simplier to manipulate.

if (isset($resource['actions'])) {

In the first alter I can simply unset $resource['actions'] in the second I can achieve the same thing but I also have to manipulate the controller array. This is ineffect an api change so it couldn't go into this version as we are now at RC and trying to get a release out so it has to be postpond.

Comment #4

ygerasimov CreditAttribution: ygerasimov commented 9 August 2011 at 06:17

Alright. Lets postpone this issue.

But @marcingy can you update documentation about these hooks? Specially difference between them.

Comment #5

marcingy CreditAttribution: marcingy commented 9 August 2011 at 15:29

My point is that someone could be using either hook therefore we can not remove them as it is an api - I personally can't provide docs re when to use one or the other because the answer is that either of them could be used for the same thing we are simple to far the rc process to rationalise them.