Clarify the finders backend options

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

• Close this issue

Preamble

With !90991 (merged) we are checking if a finder return ActiveRecord::Relation or not. Current offenses are stored in spec/support/finder_collection_allowlist.yml which contains two sections: Permenant excludes and Temporary excludes (aka TODOs).

Problem

It seems that finders are designed specially for retrieving AR relations from databases. Finders with other backends (like memory or file) should not be named "finder" but rather put into lib/.

As per discussion in Slack:

@alexkalderimis I have just added a finder, specific to the quick action interpreter, that returns an array, violating the new rules (see https://docs.gitlab.com/ee/development/reusing_abstractions.html#finders). I could add an exception for this, but that seems messy. What is the best place to put a class that exists to find an array of things?

@ahegyi: I'd vote for the lib folder for now. Maybe we need another module within filters (Array::) to make a distinction between Array-returning (elasticsearch, clickhouse, etc.) and AR::Relation-returning finders?

@splattael: I agree! We should make the intent of finders very clear in the documentation. Personally, I also thought that they are backend-agnostic and a generic concept of how to retrieve objects from a store (database, memory, files). Now, I see it's not the case 😅

Proposed solution

• Highlight that finders are only meant to be used with a AR backends in https://docs.gitlab.com/ee/development/reusing_abstractions.html#finders
• Remove section Permanent excludes from spec/support/finder_collection_allowlist.yml
• Review all current offenses in spec/support/finder_collection_allowlist.yml and
  • decide if "false finders" should be moved to lib/
  • create a follow-up issue to fix all actual offenses