Move boilerplate required field None checks into ProviderDataIngester

[stacimc]

Current Situation

ProviderDataIngester child classes currently all must implement logic in get_record_data to return early when one of the required fields (foreign_landing_url, image/audio_url, foreign_identifier, license_info) is None. Example:

        if (foreign_identifier := data.get("foreign_id")) is None:
            return None

        if (foreign_landing_url := data.get("url")) is None:
            return None

        # ...and so on

This logic is repeated in each of our provider scripts, causing code duplication. There's also a risk that a new contributor might forget to implement this logic in a new provider script.

Suggested Improvement

We should try to move this logic into the base class. One idea is, instead of having a single abstract get_record_data method, to have an abstract method for each required field as well as a get_optional_fields method. Then get_record_data could contain the shared logic:

@abstractmethod
def get_foreign_identifier(self, data:dict) -> str | int:
    pass

@abstractmethod 
def get_optional_fields(self, data:dict) -> dict:
    pass

def get_record_data(self, data: dict) -> dict | list[dict] | None:
    if (foreign_identifier := self.get_foreign_identifier(data)) is None:
        return None

    # and so on for other required fields...

    optional_fields = self.get_optional_fields(data)

    return {
       "foreign_identifier": foreign_identifier,
       # etc...
       **optional_fields
    }

^ Just a starting point. For example, this does not support returning lists yet.

See the implementation of checks in Europeana for another idea: WordPress/openverse-catalog#821 (comment) using decorators. This approach is a bit cleaner especially in the event that the required fields list expands, and it also adds extra logging.

Some things we need to keep in mind:

• We still need to support returning a list of data from get_record_data. See SMK for an example of a complex use case
• Changes should ideally be backwards compatible with currently refactored providers so they don't need to be updated all at once. Consider refactoring Europeana as part of this ticket.
• We should update the template as part of this work

Benefit

• Reduce code duplication
• Makes it clearer which fields are required without relying on docs
• Ensures new providers will not miss this functionality

Additional context

This was discussed on WordPress/openverse-catalog#821 in this comment thread.

Implementation

• 🙋 I would be interested in implementing this feature.