thegreenwebfoundation / greencheck-api

The green web foundation API
https://www.thegreenwebfoundation.org/
Apache License 2.0
9 stars 3 forks source link

WIP - get the API under CI too #14

Closed mrchrisadams closed 4 years ago

mrchrisadams commented 5 years ago

We have the greencheck library under CI now, but we'd need to check that the the API works as expected as we're changing the flow for fetching data to get a result from redis first, then running our check now.

mrchrisadams commented 5 years ago

Sigh, this is turning out to be a pain. Neither the RedisClient, nor PredisClient seem to write json how I would expect them to, which is making pulling it out in the API a pain.

Using Predis instead of Redis

We're assuming use of the whole DoctrineCache interface in other parts of the code base, anf making various caching calls to use it, but annoyingly while RedisCache gives you access to the underlying Redis client, Predis doesn't:

It looks like Redis gives you access to the client: https://github.com/doctrine/cache/blob/master/lib/Doctrine/Common/Cache/RedisCache.php#L37

But PredisCache doesn't: https://github.com/doctrine/cache/blob/master/lib/Doctrine/Common/Cache/PredisCache.php#L21

So, I've tried to take the approach of using the API offered by PredisCache instead, using save, and fetch, as we're not doing using much else, and presumably this would honour the calls to namespacing too, which I assume is important:

Here's the Redis session:

127.0.0.1:6379> keys *domains*
1) "tgwf_greencheck[domains:www.nu.nl][4963]"

// a load more more keys matching left out 

When we look at the key returned, we see that:

there's still all this cruft before we get to the actual JSON encoded content, - he have this key listing before the json O:28:\"TGWF\\Greencheck\\LatestResult\":6 , in addition to that extra [4963] appended to the key name itself, as well as a load what seems to be type information for each property of the datastructure:

"O:28:\"TGWF\\Greencheck\\LatestResult\":6:{s:4:\"date\";s:25:\"2019-05-10T09:15:31+00:00\";s:3:\"url\";s:13:\"www.xs4all.nl\";s:17:\"hostingProviderId\";i:42002;s:18:\"hostingProviderUrl\";s:20:\"http://www.xs4all.nl\";s:19:\"hostingProviderName\";s:6:\"Xs4all\";s:5:\"green\";b:1;}"

In this case here, I'd be much happier just getting this back as just the JSON, and just using a key without any automatic prefixing. So having a key like this:

domains:www.thegreenwebfoundation.org

And JSON like this (linebreaks added for readbility):

{
  "green": true,
  "url": "www.thegreenwebfoundation.org",
  "data": true,
  "hostedby": "LeaseWeb",
  "hostedbyid": 156,
  "hostedbywebsite": "www.leaseweb.com",
}

Deciding the trade-off

I can rewrite this to only use the Predis Client, so on the front end we don't have this weird key, suffix, and serialisation behaviour but we'd then need to check elsewhere in the codebase, to make sure we're making direct calls to redis rather than going through these interfaces.

I'm not familiar enough with Symfomy or Doctrine to make a call here - if we just want to write JSON to a redis key, then pull it out in another process, so we can rehydrate that JSON without needing everyone know the internals of Symfony or Doctrine, what's the simplest approach we can take?

My current thinking is to literally write the strings, and do without all this cruft, as the API isn't using Doctrine either, but I don't know what we lose by doing this.

@arendjantetteroo we're pretty settled on Redis - are you okay with me using the client direct, and skipping all this extra DoctrineCache stuff ?

I can't see where we're using it in the API section, and not being so coupled to Doctrine gives us more options later too.

mrchrisadams commented 5 years ago

Okay after a quick chat, here's the resolution.

Recognise we have two kinds of caching really:

1) internal use, where there's no need to worry about other libraries or frameworks needing to access the underlying data 2) external use where more than one system might use it

In our case here, while we have a load of caching libraries already, the act of building this longer lasting hash, to we can take fast CSV dumps, and likely from inside other languages or frameworks lends itself to us using the Predis Client directly in the logger, rather than relying on all the existing Doctrine Caching code we have in place.

So, we'll rely on the DoctrineCache interfaces for all 'internal' use, and use the PredisClient for 'external; use.

mrchrisadams commented 5 years ago

okay, I think I've found and cause for this to be breaking:

Here's the error I get in the app when consuming this the greencheck library

[Symfony\Component\Debug\Exception\FatalThrowableError]
  Argument 1 passed to TGWF\Greencheck\Sitecheck\Aschecker::__construct() must implement interface Symfony\Contracts\Cache\CacheInterface, instance of TGWF\Greencheck\Sitecheck\C
  ache given, called in /Users/chrisadams/Code/tgwf/thegreenwebfoundation/packages/greencheck/src/Sitecheck.php on line 504

So, TGWF\Greencheck\Sitecheck\Cache needs to conform to the Symfony\Contracts\Cache\CacheInterface interface.

We're getting somewhere!

mrchrisadams commented 5 years ago

No, that bug was user error on my end. Two different branches running. Disregard…

arendjantetteroo commented 4 years ago

@mrchrisadams still relevant. will close for now. Feel free to reopen when it's becoming relevant again.