Readd support for rich options for resources - needed by auth mechanism

Actually I'm not so sure this is true. You now set controller settings via the authentication tab.
I tested this with oath and it worked as expected

Can you refer me to the code where it WAS working that way?

I went through all the way back to last Summer(July 1st) and scoured through the code to find a working version of this. I know you mentioned it was before the Resource UI switch, but I havnt been able to see this.

The patch that you posted seems to add a CONTROLLER SETTINGS to each endpoint. If you look at services_oauth the controller settings refer to this

function _services_oauth_controller_settings($settings, $controller, $endpoint, $class, $name) {
  $form = array();

  $cc = $controller['endpoint']['services_oauth'];
  $auth_levels = array();
  $context = oauth_common_context_load($settings['oauth_context']);
  foreach ($context->authorization_levels as $name => $level) {
    $auth_levels[$name] = t($level['title']);
  }

  $form['credentials'] = array(
    '#type'          => 'select',
    '#options'       => array(
      'none'              => t('None'),
      'unsigned_consumer' => t('Unsigned with consumer key'),
      'consumer'          => t('Consumer key'),
      'token'             => t('Consumer key and access token'),
    ),
    '#default_value' => !empty($cc['credentials']) ? $cc['credentials'] : 'token',
    '#title'         => t('Required authentication'),
    '#description'   => t('Authorization levels will <em>not</em> be applied if the consumer isn\'t required to supply a access token.'),
  );

  $form['authorization'] = array(
    '#type'          => 'select',
    '#options'       => $auth_levels,
    '#default_value' => !empty($cc['authorization']) ? $cc['authorization'] : '*',
    '#title'         => t('Required authorization'),
  );

  return $form;
}

So, your adding settings to every single method about whether it should be authorized or not? Thats what confused and made me think that that code should be on the Authroization settings tab, not per resource method.

I could be wrong and am happy to get this in before the next RC, i just need to know what specifically it is that is needed by services_oauth.

Can you help me help you?

Ok heres an updated patch that brings the functionality full circle and fixes the settings for services oauth.
I have no idea if services authenticated call is working, but that to me is a separate bug/patch.

The reason you see changes to the services_oauth.inc is because I wanted to prove the settings save and show per endpoint.

Doesn't apply, but this is pretty major for oauth to work

Heres an updated version of this that i THINK fully covers the scope of this task.

Please review

I can't apply the patch. Also some trailing spaces have been added.

error: patch failed: plugins/export_ui/services_ctools_export_ui.class.php:232
error: plugins/export_ui/services_ctools_export_ui.class.php: patch does not apply
error: patch failed: services.admin.inc:12
error: services.admin.inc: patch does not apply

Forgive me this patch is for 6.x right now as oAuth really only works in 6.x

#11: 0001-Rich-settings-per-endpoint.patch queued for re-testing.

updated patch.

The default values for the resources form don't seem to be loading correctly. When I hit admin/build/services/list//resources, none of the checkboxes are checked. If I make changes to the form and hit Save, none of my changes are reflected when the page reloads.

Ok ill spend a bit more time on this later, anyone else who wants to join in feel free.

I'm not entirely sure what it's supposed to do, but I've folded some of the old form into services/authentication form on http://drupal.org/node/1336974#comment-5260612

So, pardon me for being out of the loop - but what is the intent with the above? (I'll gladly help if I understood what it's supposed to do)

Basically, it adds rich settings/options to each Resource Method on the REsources page and allows other mothers like oauth to hook into these.

Ok boys heres a newly updating patch. AGain this does NOT actually work, it just adds the forms back in.

Ok heres a new patch.

This should fix everything and bring it home. Services_oauth still needs work but thats a separate issue.

This is just a code review not an actual test...

function services_get_resource_operation_info($resource, $class, $name = NULL) {
+  $op = NULL;
+  $classes = array(
+    'actions'          => t('Action'),
+    'targeted_actions' => t('Targeted Action'),
+    'relationships'    => t('Relationship'),
+  );
+  if (isset($resource[$class])) {
+    $op = $resource[$class];
+    if (!empty($name)) {
+      $op = isset($op[$name]) ? $op[$name] : NULL;
+    }
+  }
+
+  return $op;
+}

The $classes array is not actually used and wondering if this easier to read

function services_get_resource_operation_info($resource, $class, $name = NULL) {
+  $op = isset($resource[$class]) ? $resource[$class] : NULL;
+  if (!empty($op) && !empty($name)) {
+    $op = isset($op[$name]) ? $op[$name] : NULL;
+  }
+
+  return $op;
+}

This can also be simplified

if (isset($rop['help'])) {
+      $description = $rop['help'];
+    }
+    else {
+      $description = t('No description is available');
+    }

to

$description = isset($rop['help']) ? $rop['help'] : t('No description is available');

Extra line that is not needed

+        //$settings_form['#tree'] = TRUE;

Here is simply rerolled patch of #23. I hold on my testing until Hugo finishes refactoring.

I've kicked off a feature branch instead of doing all this incessant re-rolling: http://drupalcode.org/project/services.git/shortlog/refs/heads/feature/r...

The current state of feature/rich-options-for-resources. Changing component from "User interface" to "Code" as the scope has changed a bit, one could argue that (re)adding configuration options to controllers was beyond the scope of "User interface" to begin with. Instead of normalizing the resource definitions on the fly for the admin the resource definition format has been updated to be consistent both with the endpoint format and between all operation classes.

http://drupalcode.org/project/services.git/commit/70916426aafca7fcf52f6b...

Updated resource definition format and added versioning of the format for automatic upgrades of old declarations.

The RPC translation layer and the REST server have been updated to work with the new format. The RPC translation functions also logs errors when there are naming collisions between operations in different classes.

Added proper default value handling for the auth module settings.

Updated documentation for hook_services_resources().

Updated documentation for hook_services_authentication_info() with the new properties. It's still somewhat lacking though.

heres an updated patch.

Re-queue testing.

Lets try this one DRUPAL!

Alrighty trying this one more time.

As part of the backport to 6.x-3.x (which is in progress), I'd like to change '#api_version' to remove the octothorpe. It's the only key in the resource array that has a leading octothorpe, and I don't see any reason for the inconsistency. Since there hasn't been a release with this patch, hopefully this change can happen without breaking anything important.

Do not mistake the recent #-purge in services for a ban on hash signs. The old hashes were removed because they didn't have any function, they were simply there because the services API mimicked the menu API. The hash you want to remove now does, however, have a semantic function. It separates the attribute api_version from its siblings that are arbitrarily named elemements representing resources. Without the hash sign we would have to add the restriction that resources cannot be named api_version; or we'd have to add another level to our data structure where resources are nested under a "resources" element. The first solution lacks elegance and is potentially backwards incompatible. The second solution would be hard to make backwards compatible and really qualifies as an ugly structure in my eyes.

So, keep the semantic meaning of the # in mind: it separates the attributes of an element from its child elements. If there's no need for such a separation we shouldn't go around adding hashes willy nilly (as was previously done). Neither should you reflexively remove hashes when you encounter them, as some kind of coding style review.

here you go, a 6.x version.

Wow! Thanks Kyle!

yeah big patches, now to get versions in core so we can start making changes to resources!

Unfortunately, the code doesn't work, resource options aren't passed to the authenticate_call callback. You only get the general, endpoint settings.
You can easily confirm this with the current OAuth integration, set "authorization" to "" and "credentials" to "token" in the endpoint settings.
Then, for the current resource, set "authorization" to "basic" (or some other authorization level) and "credentials" to "consumer".
In the authenticate_call (_services_oauth_authenticate_call), you will only get access to the endpoint settings.

I tried to patch this, but couldn't find a clean way of fetching what I need from the $endpoint object in _services_authenticate_user().

Here's a patch.

Rerolled.

Ignore it, it's broken.

works for me - thanks!

RE-upload of patch.

Is this patch safe to use? It's not passing testing.

55: fix_controller_settings-1154420-51.patch queued for re-testing.

We have it in production.

I am using this patch (https://drupal.org/node/1154420#comment-7842205) and it works very well. Damn the automatic testers, they are not always right, sometimes they may need to be fixed themselves. I think that this patch should be applied.

These two errors fail locally with the exact same result as the test bot.

How come that I have applied the patch and everything seems to work fine?
I did not blame the bot, I blamed the tests that are giving these errors.

You can login, and log out again, using standard session authentication? Thats what appears to be breaking in the bot.

First, I have applied the patch against the latest stable version (7.x-3.5), this issue is filed against the 7.x-3.x-dev.

Second, I am using this module (services) to define my own (custom) web resources. I neither need nor use the 'user' resource. For that matter, I don't need or use the other predefined resources as well. So, as far as I am concerned, it is not a problem for me if 'user/logout' resource does not work.

In my opinion, predefined resources should belong to a separate, optional module, because not everybody needs them.

We have a ticket for that. but just because it doesn't break for you doesn't mean it works.

Give us time to make sure it all works before we just commit it.

Ill try and figurer out whats going on tomorrow.

There is no hurry, I am using it as a patch. But just it should not be pending forever.

@dashohoxha if you want this patch committing help fix the test because unless the tests are fixed the patch will never be committed :( And even if the resources were in a module this would still be the case.

@marcingly I am not a maintainer of 'services', I am not a maintainer of 'oauth2_server', I am not the author of the patch, why is it me that should fix the tests? It would take me too much time and I have already spent too much time to fix modules that do not work. I tried to help by giving my testimony that I am using it and in my experience everything is working well. Did you try to use this patch and did it create any problems for you?

So, I find your challenge unjust and dishonest. I may try to fix the tests, but not before you give up being a maintainer of 'services'.

dashohoxha, Im already working on it, but it very well may be that the tests are accurate.

Lets move the topic of conversation back to the patch and fixes, rather who should fix what.

I appreciate any time and input you have going forward dashohoxha

Heres an updated patch, I hope everyone who's uses it can test as I had to add an if(is array())

There error in question when using regular services authentication is....
<b>Fatal error</b>: Unsupported operand types in <b>/Users/kyle.browning/Sites/d7/sites/all/modules/services/includes/services.runtime.inc</b> on line <b>418</b><br />

This patch address that, and all tests seem to pass now.

Can we verify the settings still work?

kylebrowning, thanks for fixing it and for your constructive comments.

I have applied the latest patch to 7.x-3.5 and so far everything seems OK.
I think that a positive review from bojanz will definitely validate it, since I don't really understand what this patch does. I am just interested in using services with oauth2_server. Thanks again for being responsive.

Commit Reference: http://drupalcode.org/project/services.git/commit/7b85300aea5dafe82b03ba...

Thanks kylebrowning, works nicely.