Problem/Motivation

REST responses for creating entities with POST add a $response->addCacheableDependency($url); call. But that responses should not be cached at all.

Proposed resolution

Remove that $response->addCacheableDependency($url); call.

Remaining tasks

Write & review patch.

User interface changes

none

API changes

none

Data model changes

none

CommentFileSizeAuthor
#2 rest-post-cache-2571929-2.patch843 bytesklausi

Comments

klausi created an issue. See original summary.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new rest-post-cache-2571929-2.patch843 bytes

Patch attached.

The response is already uncachable, so that call is just completely wrong and does not do anything. Url does not implement CacheableDependencyInterface, so it has no effect.

Status: Needs review » Needs work

The last submitted patch, 2: rest-post-cache-2571929-2.patch, failed testing.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Reviewed & tested by the community
Issue tags: +D8 cacheability
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Looks possible to test.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Category: Task » Bug report

also sounds like a bug

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: REST entity POST request must not be cachable » REST entity POST request is not cacheable: cacheability metadata is unnecessary
Category: Bug report » Task
Priority: Normal » Minor

This is what the actual "problem" is. There is no actual bug.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs tests

Therefore moving back to RTBC. Let's hear what committers think.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 6b0d523 and pushed to 8.0.x and 8.1.x. Thanks!

I had to manually apply the patch to 8.1.x because the patch would not apply - also I fixed a spelling mistake... cachability should be cacheability

  • alexpott committed b2734bb on 8.1.x 
    Issue #2571929 by klausi: REST entity POST request is not cacheable:...

  • alexpott committed 6b0d523 on 8.0.x 
    Issue #2571929 by klausi: REST entity POST request is not cacheable:...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Thanks Alex, and good catch :)

Status: Fixed » Closed (fixed)

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