Fix: recipe scraper image cleaning

[michael-genson]

What type of PR is this?

(REQUIRED)

• bug

What this PR does / why we need it:

(REQUIRED)

The recipe scraper wasn't accounting for nested dictionaries of image urls, as mentioned in #2087. I found that the scraper actually wasn't cleaning images at all, so I enabled that.

Which issue(s) this PR fixes:

(REQUIRED)

resolves #2087

Release Notes

(REQUIRED)

recipe scraper has a higher chance of finding an image when scraping

[fleshgolem]

Is it correct for this case to be completely removed though?

[michael-genson]

I just moved it slightly down so it's easier to read, it's not gone

[fleshgolem]

Right, it seems like i am a bit blind

[fleshgolem]

Right, it seems like i am a bit blind

[fleshgolem]

I am not a 100% sure about this, but as far as i remember this will ultimately get used by the recipe_data_service to retrieve the image. That service however also accepts a list of image-urls and will use that to determine the largest one, so it would make sense here to not just return the first hit but the full list of urls so the best one can decided on further down the chain

[hay-kot]

@fleshgolem I was also looking at this and confused on why this returns a string. We do have logic that fetches the largest image so I'm wondering if this should really just return a list to begin with?

@michael-genson I'm also not sure we need to nested match statement here - this is sort of what I was thinking

match image:  # noqa - match statement not supported
    case str(image):
        return [image]
    case [str(_), *_]:
        return image
    case [{"url": str(_)}, *_]:
        return [x["url"] for x in image]
    case {"url": str(image)}:
        return [image]
    case _:
        raise TypeError(f"Unexpected type for image: {type(image)}, {image}")

That would catch all cases we're looking at now right? This returns the list, but could also just return the first resulting match as needed. I don't think we need recursion like I initially thought though.

[michael-genson]

LGTM. I'll update it and make sure it passes the original issue's scrape

[michael-genson]

Per discussion I refactored clean_image to return a list of strings, rather than a single string. At the final step (cleaning before loading data into the database) we select the first image in the list as the recipe image. Since clean_image always returns a non-empty list (using a default string), this will never produce an error.

There's something to be said about the image tag being cleaned twice (@fleshgolem raised this in discord), but I wasn't comfortable dropping it in either place since we're mixing two different workflows (scraping and migrations) and I don't think both workflows use the cleaning functions the same way.