feat(httpcache): souin integration by darkweak · Pull Request #2487 · api-platform/api-platform

darkweak · 2023-07-22T00:25:29Z

Q	A
Branch?	main
Tickets	Closes #1947
License	MIT
Doc PR	api-platform/docs#1720

Obsoletes #2383

api/docker/caddy/Caddyfile

Co-authored-by: Vincent <407859+vincentchalamon@users.noreply.github.com>

vincentchalamon · 2023-08-23T16:06:43Z

The following improvements could be great to add before merge:

enable Souin only on prod Symfony environment
is there a way to add a header on debug mode, for testing purpose?

darkweak · 2023-08-23T16:43:46Z

Hey, do you have two different Caddyfiles (one for prod and another one for dev)? Assuming yes, just compile your docker image with the cache-handler caddy module, set the cache directive in the prod Caddyfile and don't set it in your dev Caddyfile
On debug environment, I suggest to update the Cache-Status name (using the directive cache_name) it will replace the default Souin value (e.g. Souin; hit; ttl="99".. become Your-debug-cache-name; hit; ...). Unfortunately it's not possible to set a debug header.

vincentchalamon · 2023-08-24T08:13:50Z

Is something like that would do the work?

# Caddyfile
{
    order cache before rewrite
    cache {
        api {
            souin
        }
    }
}

{$CADDY_CACHE}

# ...

# docker-compose.prod.yml
services:
    caddy:
        environment:
            CADDY_CACHE: cache

I'm wondering if the caddy.api.souin declaration would cause issues even in dev mode. WDYT?

darkweak · 2023-08-24T08:19:35Z

Why don't you link a Caddyfile.prod in your docker-compose.prod.yml instead of the default Caddyfile? Is it mandatory to have only one Caddyfile?

cache directive will crash if your caddy instance has been built with the cache-handler dependency in dev.

vincentchalamon · 2023-08-24T09:10:48Z

Using a Caddyfile.prod requires to duplicate the content from Caddyfile, or use import feature and juggle with Caddyfiles files such as:

# Caddyfile.prod

import Caddyfile.dev

cache

# Dockerfile

RUN mv /etc/caddy/Caddyfile /etc/caddy/Caddyfile.dev
COPY --link docker/caddy/Caddyfile.prod /etc/caddy/Caddyfile

But I don't find it as a good approach...

I'm working on an implementation here: api-platform/demo@02e5026

EDIT: implementation commit amended: api-platform/demo@73977b3

dunglas · 2023-08-24T09:58:16Z

Having a single Caddyfile will help with maintenance. It's possible to inject any global options in CADDY_DEBUG. I opened #2502 to improve this.

vincentchalamon

Nice integration. Main issue: cache should only be enabled on prod env. Check api-platform/demo@73977b3 for example

api/config/packages/api_platform.yaml

vincentchalamon · 2023-08-24T11:49:54Z

api/config/packages/api_platform.yaml

    defaults:
        stateless: true
        cache_headers:
+            max_age: 0


HTTP Cache Headers should only be set on prod env

Comment still valid for max_age header

api/docker/caddy/Caddyfile

api/Dockerfile

darkweak · 2023-09-12T10:42:24Z

Ping @vincentchalamon

vincentchalamon · 2023-09-12T11:19:01Z

api/config/packages/api_platform.yaml

+        invalidation:
+            enabled: true
+            purger: 'api_platform.http_cache.purger.souin'
+            urls: ['%env(SOUIN_API_URL)%']


Suggested change

urls: ['%env(SOUIN_API_URL)%']

urls: ['%env(SOUIN_API_URL)%']

vincentchalamon · 2023-09-12T11:19:43Z

api/.env

 TRUSTED_PROXIES=127.0.0.0/8,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16
 TRUSTED_HOSTS=^(localhost|caddy)$
+# The api url that is called to invalidate cached resources
+SOUIN_API_URL=http://caddy/souin-api/souin


A declared env var without being used, especially in dev/test environments, will throw an error in Symfony commands

vincentchalamon · 2023-09-12T11:20:18Z

api/config/packages/api_platform.yaml

    defaults:
        stateless: true
        cache_headers:
+            max_age: 0


Comment still valid for max_age header

vincentchalamon · 2023-09-12T11:20:58Z

api/docker/caddy/Caddyfile


 log

+cache


Should be enabled through an env var:

Suggested change

cache

{$CADDY_CACHE}

Then, declare this env var in docker-compose.prod.yml and helm chart to only enable cache on production

vincentchalamon · 2023-09-12T11:21:52Z

docker-compose.yml

      SERVER_NAME: ${SERVER_NAME:-localhost}, caddy:80
      MERCURE_PUBLISHER_JWT_KEY: ${CADDY_MERCURE_JWT_SECRET:-!ChangeThisMercureHubJWTSecretKey!}
      MERCURE_SUBSCRIBER_JWT_KEY: ${CADDY_MERCURE_JWT_SECRET:-!ChangeThisMercureHubJWTSecretKey!}
+      CADDY_GLOBAL_OPTIONS: |


Do not enable cache on dev

vincentchalamon · 2023-09-12T11:23:50Z

I tried to implement it in the demo (cf. api-platform/demo#327) but had to rolled it back because it caused issues. If you want to try and improve this implementation in API Platform, you can test it (locally or with a fork of my branch) with this branch: https://github.com/api-platform/demo/tree/chore/refacto

darkweak · 2023-09-12T11:30:44Z

https://github.com/darkweak/souin/issues/new if you have issues using it.

feat(httpcache): souin integration

136676a

vincentchalamon mentioned this pull request Aug 23, 2023

Update Demo api-platform/demo#327

Merged

18 tasks

vincentchalamon reviewed Aug 23, 2023

View reviewed changes

api/docker/caddy/Caddyfile Outdated Show resolved Hide resolved

darkweak and others added 2 commits August 23, 2023 18:03

Update api/docker/caddy/Caddyfile

16c814f

Co-authored-by: Vincent <407859+vincentchalamon@users.noreply.github.com>

Merge branch 'main' into feat/httpcache/souin-integration

8269002

dunglas mentioned this pull request Aug 24, 2023

feat: rename CADDY_DEBUG to CADDY_GLOBAL_OPTIONS #2502

Merged

vincentchalamon requested changes Aug 24, 2023

View reviewed changes

dunglas reviewed Aug 25, 2023

View reviewed changes

api/Dockerfile Outdated Show resolved Hide resolved

darkweak added 2 commits September 12, 2023 11:59

Merge branch 'main' into feat/httpcache/souin-integration

0a86bc8

fix(review): cleanup Dockerfile, use CADDY_GLOBAL_OPTIONS

7ede32f

darkweak force-pushed the feat/httpcache/souin-integration branch 3 times, most recently from 2ec729b to b80d9ff Compare September 12, 2023 10:26

fix(configuration): enable cache only on prod env

6f0fb64

darkweak force-pushed the feat/httpcache/souin-integration branch from b80d9ff to 6f0fb64 Compare September 12, 2023 10:28

vincentchalamon requested changes Sep 12, 2023

View reviewed changes

	urls: ['%env(SOUIN_API_URL)%']
	urls: ['%env(SOUIN_API_URL)%']

Uh oh!

Conversation

darkweak commented Jul 22, 2023

Uh oh!

Uh oh!

vincentchalamon commented Aug 23, 2023

Uh oh!

darkweak commented Aug 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vincentchalamon commented Aug 24, 2023

Uh oh!

darkweak commented Aug 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vincentchalamon commented Aug 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dunglas commented Aug 24, 2023

Uh oh!

vincentchalamon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vincentchalamon Aug 24, 2023

Choose a reason for hiding this comment

Uh oh!

vincentchalamon Sep 12, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

darkweak commented Sep 12, 2023

Uh oh!

vincentchalamon Sep 12, 2023

Choose a reason for hiding this comment

Uh oh!

vincentchalamon Sep 12, 2023

Choose a reason for hiding this comment

Uh oh!

vincentchalamon Sep 12, 2023

Choose a reason for hiding this comment

Uh oh!

vincentchalamon Sep 12, 2023

Choose a reason for hiding this comment

Uh oh!

vincentchalamon Sep 12, 2023

Choose a reason for hiding this comment

Uh oh!

vincentchalamon commented Sep 12, 2023

Uh oh!

darkweak commented Sep 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

darkweak commented Aug 23, 2023 •

edited

Loading

darkweak commented Aug 24, 2023 •

edited

Loading

vincentchalamon commented Aug 24, 2023 •

edited

Loading