Rest module currently attempts to interface with rest.yml, however config files must have not only a module namespace but a config name. This should become rest.resources.yml. Additionally, no default configuration is supplied, it is just written out on the first submission of its settings form. A default configuration file should be supplied representing the initial configuration state (even in this case where nothing should be enabled by default.)

Patch to come.

Files: 
CommentFileSizeAuthor
#10 1858676-rest-config-10.patch3.15 KBheyrocker
PASSED: [[SimpleTest]]: [MySQL] 50,434 pass(es). View
#9 1858676-rest-config.patch2.92 KBDamien Tournoud
PASSED: [[SimpleTest]]: [MySQL] 50,148 pass(es). View
#7 rest.config.7.patch2.34 KBsun
FAILED: [[SimpleTest]]: [MySQL] 49,363 pass(es), 41 fail(s), and 1 exception(s). View
#4 1858676-rest-config-4.patch2.34 KBheyrocker
FAILED: [[SimpleTest]]: [MySQL] 48,899 pass(es), 26 fail(s), and 0 exception(s). View
#1 1858676-rest-config-1.patch1.71 KBheyrocker
FAILED: [[SimpleTest]]: [MySQL] 48,817 pass(es), 60 fail(s), and 15 exception(s). View

Comments

heyrocker’s picture

Status: Active » Needs review
FileSize
1.71 KB
FAILED: [[SimpleTest]]: [MySQL] 48,817 pass(es), 60 fail(s), and 15 exception(s). View

Woop!

tstoeckler’s picture

+++ b/core/modules/rest/config/rest.settings.yml
@@ -0,0 +1 @@
+resources: null

+++ b/core/modules/rest/rest.admin.inc
@@ -26,7 +26,7 @@ function rest_admin_form($form, &$form_state) {
+  $enabled_resources = config('rest.settings')->get('resources') ?: array();

Won't the call to config return 'null' as a string, and, hence, cast to TRUE? I think we could just put an empty array directly in the default config and then kill the ternary completely. Considering that I've not spent that much time knee-deep in CMI yet, I'm probably missing something but I wanted to bring it up just in case.

Status: Needs review » Needs work

The last submitted patch, 1858676-rest-config-1.patch, failed testing.

heyrocker’s picture

Status: Needs work » Needs review
FileSize
2.34 KB
FAILED: [[SimpleTest]]: [MySQL] 48,899 pass(es), 26 fail(s), and 0 exception(s). View

It helps if I convert the tests as well. This passes locally for me now.

As far as the YAML, this string null does actually convert to the value NULL as long as it isn't quoted, however I do agree that an empty array would probably be a better default value. I'm about to run but I'll get to that in a followup.

Status: Needs review » Needs work

The last submitted patch, 1858676-rest-config-4.patch, failed testing.

xjm’s picture

I downloaded this patch and issue to look at offline.

sun’s picture

Status: Needs work » Needs review
FileSize
2.34 KB
FAILED: [[SimpleTest]]: [MySQL] 49,363 pass(es), 41 fail(s), and 1 exception(s). View

The only reason for the test failures seems to be the bogus default value for resources in the default config file.

Status: Needs review » Needs work

The last submitted patch, rest.config.7.patch, failed testing.

Damien Tournoud’s picture

Status: Needs work » Needs review
FileSize
2.92 KB
PASSED: [[SimpleTest]]: [MySQL] 50,148 pass(es). View

Just keeping the ball rolling. The config call in Drupal\rest\EventSubscriber\RouteSubscriber was not converted.

heyrocker’s picture

FileSize
3.15 KB
PASSED: [[SimpleTest]]: [MySQL] 50,434 pass(es). View

Last patch was missing the default config file.

sun’s picture

Thanks + ouch @ #9... The good thing about procedural wrappers is that we can consistently grep the entire code base for existing instances — the more stuff we inject, the more inconsistent + ungreppable becomes our code base. :-/ I hope we'll be able to address this somehow prior to release.

xjm’s picture

Status: Needs review » Reviewed & tested by the community

Yep.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

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