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.

CommentFileSizeAuthor
#9 0001-Comment-about-deprecated-hook_services_resources_pos.patch1.1 KBygerasimov
#7 0001-Comment-abou-deprecated-hook_services_resources_post.patch986 bytesygerasimov

Comments

marcingy’s picture

marcingy commented

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.

ygerasimov’s picture

ygerasimov commented

@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?

marcingy’s picture

marcingy commented
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.

eg

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.

ygerasimov’s picture

ygerasimov commented

Alright. Lets postpone this issue.

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

marcingy’s picture

marcingy commented

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.

marcingy’s picture

marcingy commented

May the answer for now is mark one as deprecated, so as it warns people it is going to be removed.

ygerasimov’s picture

ygerasimov commented
Status: Postponed » Needs review
StatusFileSize
new 0001-Comment-abou-deprecated-hook_services_resources_post.patch986 bytes

Alright. Lets mark hook_services_resources_post_processing_alter as deprecated as the other one is used by rest server.

marcingy’s picture

marcingy commented
Status: Needs review » Needs work

I'd get rid of this part of the comment otherwise it is good (called about 20 lines up).

ygerasimov’s picture

ygerasimov commented
Status: Needs work » Needs review
StatusFileSize
new 0001-Comment-about-deprecated-hook_services_resources_pos.patch1.1 KB

Here is updated patch.

marcingy’s picture

marcingy commented
Status: Needs review » Reviewed & tested by the community
marcingy’s picture

marcingy commented
Status: Reviewed & tested by the community » Fixed

Committed to d6 and d7

Status: Fixed » Closed (fixed)

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