Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Steps to Reproduce
- Install the current stable version of Drupal (8.1.x)
- Create a custom rest resource (example code here)
- Send a GET request to your new rest resource
- See that the request succeeds
- Upgrade to Drupal 8.2.x
- Send another GET request to your rest resource
- See that the request fails with 404 and the error message:
{ "message": "No route found for \"GET /todo/\"" }
Expected result
That custom rest resources would continue to work after upgrading to Drupal 8.2.x
Actual result
Custom REST resource now responds with a 404 and the error message:
{
"message": "No route found for \"GET /todo/\""
}
Video demonstration
https://www.youtube.com/watch?v=mdR8peJIu_A
Notes
I verified this experimentally with 8.2.0-beta2 and 8.2.0-dev on September 20, 12:30pm EDT. I suspect it is related to #2308745: Remove rest.settings.yml, use rest_resource config entities.
Comment | File | Size | Author |
---|---|---|---|
#18 | terminal_output2.txt | 2.33 KB | MKorostoff |
#12 | 2803179-12.patch | 1.47 KB | Wim Leers |
Comments
Comment #2
mikeryanI'm also having this problem, in a fresh 8.2.x install (no upgrade from 8.1.x), with http://cgit.drupalcode.org/migrate_plus/tree/migrate_example_advanced/mi...
I've also enabled the /dblog/{id} endpoint through restui, and can't get that to work either.
Comment #3
cilefen CreditAttribution: cilefen commentedThe video shows you had an EntityStorageException that you didn't mention in the issue summary. Could you add that please? @mikeryan: Same or similar exception?
Comment #4
cilefen CreditAttribution: cilefen commented@mikeryan: Oh you didn't upgrade.
Comment #5
MKorostoff CreditAttribution: MKorostoff commented@cilefen sure, sorry, I meant to include this with the original post. It's attached.
I'm reasonably confident that the terminal error is due to the fact that this was my second or third upgrade attempt. I upgraded once, and after I noticed my resource wasn't responding, I downgraded so as to capture the issue on video. The rest module attempts to create a config entity called rest_resource_config.entity.node. Because I had already run the upgrade once, the entity already existed, hence the error. This is all to say, I don't believe the error in my terminal is related to the issue discussed in this thread, but I could be wrong.
Comment #6
cilefen CreditAttribution: cilefen commentedThat and the fact that #2 wasn't an upgrade...
Comment #7
Wim LeersHow can you run the update again? Drupal prevents update functions (to migrate data from one version to the next) to be run multiple times, precisely for this reason.
Comment #8
Wim LeersAre you seeing this error?
#2308745-253: Remove rest.settings.yml, use rest_resource config entities:
Which was reported again at #2721595-61: Simplify REST configuration:
(We forgot to open a follow-up for that.)
It'd be great if you could try to reproduce this. If so, this will become a critical issue, and one that will have to be fixed before 8.2.0 is released.
Comment #9
Wim LeersOh wait, #2 is indeed not an update. So that can't be the cause.
#2: After deploying the config, did you rebuild your routes?
Comment #10
BerdirThis happens when you have warm caches when running the update, I just reproduced this when updating to 8.2.0-rc1, I don't think something changes since then.
An explicit call to clearCachedDefinitions() on the entity type manager should fix this before calling getDefinition() should fix this.
Comment #11
Wim LeersComment #12
Wim LeersBerdir at #10:
dawehner at #2308745-254: Remove rest.settings.yml, use rest_resource config entities:
Both achieve the same result, but the latter seems the correct one to me. If the
rest_resource_config
entity type definition changes at a later time, then this particular update function will install the definition as it is at that later time, rather than at the time of this particular update function. Which means subsequent update functions will try to update it to the then-newer version, but it'll already be at that version.Therefore, I'm going to go with @dawehner's proposal.
P.S.: alexpott raised this very concern at #2308745-245: Remove rest.settings.yml, use rest_resource config entities, but dawehner felt there was no harm because it's just a "create" operation, and I agreed. We were wrong.
This patch takes the config entity type definition in the annotation:
… and maps that to a corresponding
new ConfigEntityType([…])
instantiation, so that it no longer relies on the entity type manager. Which means no more caches to be cleared (as Berdir says), nor any chance of using not the appropriate version of that definition (as dawehner says).Comment #13
BerdirWorks for me. This is pretty much a no-op anyway for config entity. entityDefinitionUpdateManager() doesn't care about them and doesn't do anything for them, but complains if they don't exist.
Comment #14
dawehnerWFM +1
Comment #17
catchTheoretically we could have update tests for this, but it'd be a lot of work compared to what is equivalent to "use the literal schema definition instead of hook_schema()". So opened #2803865: Additional test coverage for rest_update_8201() as a follow-up and committed to 8.3.x and 8.2.x, thanks!
Comment #18
MKorostoff CreditAttribution: MKorostoff commentedHey guys, after testing the patch in #12 I got the exact same result as the initial ticket description. Here is a recording of my test http://www.youtube.com/watch?v=md8Z55-VxQw
Comment #19
mikeryanHeh, Matt got in while I was adding my comment:
So, should I close that one while we take it back up here? Seems the update issue was a red herring...
Comment #20
Berdir@MKorostoff: That is a different error. It complains that the entity already exists. That means you already did run the updates before, at least partially. that isn't supported. You have to manually delete those configs or you need to restore a db dump from before you started to update.
Comment #21
catchLet's discuss Mike's issue in #2803905: Custom REST resource no longer accessible after upgrading to Drupal 8.2.x , the update change here makes sense in itself.
Comment #22
Wim LeersThanks, Berdir, for responding to #18.
Thanks, catch, for responding to #19.
Comment #23
MKorostoff CreditAttribution: MKorostoff commented@Berdir, that's the exact same error message—if you compare terminal_output.txt and terminal_output2.txt you'll see that the two messages are identical verbatim. I also offered that same theory (that the update had already run) in #5, but Wim Leers felt that was unlikely in #7.
In any event, after deleting
rest.resource.entity.node
from the config table, I was able to run the upgrade successfully and my service worked! So thanks all around!So, to summarize for anyone who finds this thread with a similar issue later, here's what I think happened in my case:
So the question remains: why did upgrade attempt 1 fail? The failure occurred in
RestResourceConfig::normalizeRestMethod()
. This function is (I think) responsible for migrating validating the HTTP verbs on each rest resource as they are migrated from the rest.settings config entity to the rest.resource.my_custom_resource_id config entity. The problem is that my custom rest resource implemented aPUT
method, which is not valid in Drupal rest. What made this particularly sticky is that, even AFTER I removed thePUT
method from my class, Drupal continued to "remember" that it had been there previously. I'm really not sure how or why Drupal "remembered" that I used to have aPUT
method, because thePUT
method was not referenced by the rest.settings config (I also cleared caches about a million times). I never did figure out how to make Drupal "forget" about my invalidPUT
method. In the end, I just changedRestResourceConfig::normalizeRestMethod()
from this:To this:
Then ran the upgrades, and then changed the code back. Now everything works. Thanks everyone!