Fastly module breaks interface contract

[ihor-sviziev]

Hi,

This is follow up to the magento/magento2#33468 (comment)

Pre-history: In Magento at some point was delivered a fix for incorrect return type for \Magento\PageCache\Model\Config::getType() in magento/magento2@b59dc72#diff-1281464bcdc813265cb8b0e9822464bd0504743ca81e31b00d46a8b600c9b770R124

Later on, as Magento Cloud using Fastly as CDN provider, this change was partly reverted in magento/magento2@1778c0f, which caused another issue - more details see in magento/magento2#33468.

Root cause analysis shown, that issue coming to the Fastly extension, as it introduced value "fastly" (string) for the method "getType", then converting it to integer (as method designed to return integer) just breaks Fastly extension.

fastly-magento2/Observer/FlushAllCacheObserver.php

Line 61 in 707ffb3

if ($this->config->getType() == Config::FASTLY && $this->config->isEnabled()) {

fastly-magento2/Model/Config.php

Line 48 in 707ffb3

const FASTLY = 'fastly';

Suggestion:

1. Change "type" from "fastly" to some integer value
2. Write a data upgrade script that will convert the old value to new one.

/CC @hostep

[ihor-sviziev]

@vvuksan, could you review this issue?

[dpotkoc]

Hello @ihor-sviziev we are aware of issue and we are fine with your suggestion.

If we change "type" from "fastly" to some integer, which value is fine?

Magento is using next values:
const BUILT_IN = 1;
const VARNISH = 2;

I mean that we should use some number greater then 3, because Magento can use 3 for some other type of cache and then we will have issue / conflict.

What do you mean?

Thnx
Domagoj

[ihor-sviziev]

@dpotkoc, I think you can use some value greater than 10, maybe 100 is good enough.

[dpotkoc]

@ihor-sviziev thnx for answer, we will make modification.

Domagoj

[IgorVitol]

@ihor-sviziev @dpotkoc @vvuksan Updated fastly extension today and noted that it is no longer backward compatible. Our cloud stage env for some reason switched back to Built-in cache. Root cause is because of "fastly" was changed to "42".

Even with this, seems like image optimizations stopped to work:

if ($this->_scopeConfig->getValue(PageCacheConfig::XML_PAGECACHE_TYPE) !== Config::FASTLY) {
// ...
}

Such block doesn't work properly, since "42" !== 42 is true.

As a temporary solution we have to downgrade back, to the older version.

[dpotkoc]

Hello @IgorVitol thnx for reporting, we will need to fix issue for image processing. I agree with you.

Regarding your claim "Our cloud stage env for some reason switched back to Built-in cache. Root cause is because of "fastly" was changed to "42"."
It will be good if you can send us steps how to reproduce.

I think that Fastly worked fine but you had old value "fastly" exported in config.php file and dropdown in Magento admin section was not be able to preselect right option with fastly cache.

If you exported config to files we can't change file on your disc

We have plugin which is fixing issue if you have old vaule in config:

fastly-magento2/Model/PageCache/ConfigPlugin.php

Line 77 in 86e0fa7

public function aroundGetType(Config $config, callable $proceed)

Thnx
Domagoj