Redesign Lookup Provider architecture for extensibility and type safety

.github/workflows/baseline.xml

@@ -141,32 +141,15 @@
       <code>$value</code>
       <code>$value</code>
     </PossiblyNullArgument>
-    <UndefinedConstant occurrences="9">
+    <UndefinedConstant occurrences="1">
       <code>DEFAULT_USE_REDIS</code>
-      <code>LOOKUP_ID_ALBERTHEIJN</code>
-      <code>LOOKUP_ID_FEDERATION</code>
-      <code>LOOKUP_ID_JUMBO</code>
-      <code>LOOKUP_ID_OPENFOODFACTS</code>
-      <code>LOOKUP_ID_OPENGTINDB</code>
-      <code>LOOKUP_ID_PLUS</code>
-      <code>LOOKUP_ID_UPCDATABASE</code>
-      <code>LOOKUP_ID_UPCDB</code>
     </UndefinedConstant>
   </file>
   <file src="../../incl/lookupProviders/BarcodeLookup.class.php">
     <PossiblyNullArgument occurrences="1">
       <code>$config["LOOKUP_ORDER"]</code>
     </PossiblyNullArgument>
-    <UndefinedConstant occurrences="8">
-      <code>LOOKUP_ID_ALBERTHEIJN</code>
-      <code>LOOKUP_ID_FEDERATION</code>
-      <code>LOOKUP_ID_JUMBO</code>
-      <code>LOOKUP_ID_OPENFOODFACTS</code>
-      <code>LOOKUP_ID_OPENGTINDB</code>
-      <code>LOOKUP_ID_PLUS</code>
-      <code>LOOKUP_ID_UPCDATABASE</code>
-      <code>LOOKUP_ID_UPCDB</code>
-    </UndefinedConstant>
+
   </file>
   <file src="../../incl/modules/barcodeFederation.php">
     <InvalidNullableReturnType occurrences="1">

config-dist.php

@@ -113,6 +113,20 @@
                                          // "LOOKUP_UPC_DATABASE_KEY"    => null
                                          );
 
+// List of available lookup providers
+// Format: "ID" => ["class" => "ClassName", "file" => "filename.php"]
+const LOOKUP_PROVIDERS = array(
+    "1" => ["class" => "ProviderOpenFoodFacts", "file" => "ProviderOpenFoodFacts.php"],
+    "2" => ["class" => "ProviderUpcDb", "file" => "ProviderUpcDb.php"],
+    "3" => ["class" => "ProviderUpcDatabase", "file" => "ProviderUpcDatabase.php"],
+    "4" => ["class" => "ProviderAlbertHeijn", "file" => "ProviderAlbertHeijn.php"],
+    "5" => ["class" => "ProviderJumbo", "file" => "ProviderJumbo.php"],
+    "6" => ["class" => "ProviderOpengtindb", "file" => "ProviderOpengtindb.php"],
+    "7" => ["class" => "ProviderFederation", "file" => "ProviderFederation.php"],
+    "8" => ["class" => "ProviderPlusSupermarkt", "file" => "ProviderPlusSupermarkt.php"],
+    "9" => ["class" => "ProviderDiscogs", "file" => "ProviderDiscogs.php"]
+);
+
 
 // Currently not in use
 const IS_DOCKER                    = false;

incl/db.inc.php

@@ -44,16 +44,6 @@
 const SECTION_LOGS             = "log";
 
 
-const LOOKUP_ID_OPENFOODFACTS = "1";
-const LOOKUP_ID_UPCDB         = "2";
-const LOOKUP_ID_UPCDATABASE   = "3";
-const LOOKUP_ID_ALBERTHEIJN   = "4";
-const LOOKUP_ID_JUMBO         = "5";
-const LOOKUP_ID_OPENGTINDB    = "6";
-const LOOKUP_ID_FEDERATION    = "7";
-const LOOKUP_ID_PLUS          = "8";
-const LOOKUP_ID_DISCOGS       = "9";
-
 /**
  * Dockerfile changes this to "1", so that the default is true
  * For non-docker this should be false ("0").
@@ -119,15 +109,7 @@ class DatabaseConnection {
         "BBUDDY_SERVER_ENABLED" => "0",
         "BBUDDY_SERVER_POPUPSHOWN" => "0",
         "BBUDDY_SERVER_NEXTSYNC" => "0",
-        "LOOKUP_ORDER" => LOOKUP_ID_OPENFOODFACTS . "," .
-            LOOKUP_ID_UPCDB . "," .
-            LOOKUP_ID_UPCDATABASE . "," .
-            LOOKUP_ID_ALBERTHEIJN . "," .
-            LOOKUP_ID_PLUS . "," .
-            LOOKUP_ID_JUMBO . "," .
-            LOOKUP_ID_OPENGTINDB . "," .
-            LOOKUP_ID_DISCOGS . "," .
-            LOOKUP_ID_FEDERATION);
+        "LOOKUP_ORDER" => "1,2,3,4,8,5,6,9,7");
 
 
     const DB_INT_VALUES = array("REVERT_TIME");

incl/lookupProviders/BarcodeLookup.class.php

@@ -23,18 +23,6 @@ class BarcodeLookup {
 
     private const USE_DEBUG_PROVIDER = false;
 
-    private static $providers = array(
-        LOOKUP_ID_OPENFOODFACTS => "ProviderOpenFoodFacts",
-        LOOKUP_ID_UPCDB         => "ProviderUpcDb",
-        LOOKUP_ID_UPCDATABASE   => "ProviderUpcDatabase",
-        LOOKUP_ID_ALBERTHEIJN   => "ProviderAlbertHeijn",
-        LOOKUP_ID_PLUS          => "ProviderPlusSupermarkt",
-        LOOKUP_ID_JUMBO         => "ProviderJumbo",
-        LOOKUP_ID_OPENGTINDB    => "ProviderOpengtindb",
-        LOOKUP_ID_DISCOGS       => "ProviderDiscogs",
-        LOOKUP_ID_FEDERATION    => "ProviderFederation"
-    );
-
     /**
      * Look up a barcode using providers
      * @param string $barcode Input barcode
@@ -46,11 +34,52 @@ public static function lookUp(string $barcode): ?array {
         }
         $config       = BBConfig::getInstance();
         $orderAsArray = explode(",", $config["LOOKUP_ORDER"]);
+        $providers    = self::getProviders();
+
         foreach ($orderAsArray as $orderId) {
-            $result = (new self::$providers[$orderId]())->lookupBarcode($barcode);
-            if ($result != null)
-                return $result;
+            if (isset($providers[$orderId])) {
+                $result = $providers[$orderId]->lookupBarcode($barcode);
+                if ($result != null)
+                    return $result;
+            }
         }
         return null;
     }
+
+    /**
+     * @return LookupProvider[]
+     */
+    public static function getProviders(): array {
+        $providers = [];
+        if (!defined('LOOKUP_PROVIDERS')) {
+            return [];
+        }
+
+        foreach (LOOKUP_PROVIDERS as $id => $providerConfig) {
+            $className = $providerConfig['class'];
+            $fileName  = $providerConfig['file'];
+            $filePath  = __DIR__ . "/" . $fileName;
+
+            if (file_exists($filePath)) {
+                require_once $filePath;
+                if (class_exists($className)) {
+                    $provider = new $className();
+                    if ($provider instanceof LookupProvider) {
+                        $provider->setId((string)$id);
+                        $providers[$id] = $provider;
+                    }
+                }
+            }
+        }
+        return $providers;
+    }
+
+    /**
+     * @param string $id
+     * @return LookupProvider|null
+     */
+    public static function getProvider(string $id): ?LookupProvider {
+        $providers = self::getProviders();
+        return $providers[$id] ?? null;
+    }
 }

incl/lookupProviders/LookupProvider.class.php

@@ -15,17 +15,6 @@
  */
 
 
-require_once __DIR__ . "/ProviderOpenFoodFacts.php";
-require_once __DIR__ . "/ProviderUpcDb.php";
-require_once __DIR__ . "/ProviderJumbo.php";
-require_once __DIR__ . "/ProviderUpcDatabase.php";
-require_once __DIR__ . "/ProviderDebug.php";
-require_once __DIR__ . "/ProviderAlbertHeijn.php";
-require_once __DIR__ . "/ProviderPlusSupermarkt.php";
-require_once __DIR__ . "/ProviderOpengtindb.php";
-require_once __DIR__ . "/ProviderDiscogs.php";
-require_once __DIR__ . "/ProviderFederation.php";
-
 abstract class LookupProviderType
 {
     const OpenFoodFacts = 0;
@@ -39,26 +28,17 @@ abstract class LookupProviderType
     const Discogs = 8;
 }
 
-class LookupProvider {
+abstract class LookupProvider {
 
     protected $useGenericName;
     protected $apiKey;
-    protected $providerName;
     protected $ignoredResultCodes = null;
-    protected $providerConfigKey = null;
+    protected $id;
 
     function __construct(string $apiKey = null) {
-        $this->useGenericName = BBConfig::getInstance()["USE_GENERIC_NAME"];
-        $this->apiKey         = $apiKey;
-    }
-
-    /**
-     * @return bool
-     */
-    protected function isProviderEnabled(): bool {
-        if ($this->providerConfigKey == null)
-            throw new Exception('providerConfigKey needs to be overriden!');
-        return BBConfig::getInstance()[$this->providerConfigKey] == "1";
+        $this->apiKey = $apiKey;
+        $this->useGenericName = (BBConfig::getInstance()["USE_GENERIC_NAME"] == "1");
+        $this->ignoredResultCodes = array();
     }
 
     /**
@@ -71,6 +51,77 @@ public function lookupBarcode(string $barcode): ?array {
         throw new Exception('lookupBarcode needs to be overriden!');
     }
 
+    /**
+     * Sets the unique ID of the provider
+     * @param string $id
+     * @return void
+     */
+    public function setId(string $id): void {
+        $this->id = $id;
+    }
+
+    /**
+     * Returns the unique ID of the provider
+     * @return string
+     */
+    public function getId(): string {
+        return $this->id;
+    }
+
+    /**
+     * Returns the human readable name of the provider
+     * @return string
+     */
+    abstract public function getName(): string;
+
+    /**
+     * Returns the description shown below the provider name in settings
+     * @return string
+     */
+    abstract public function getDescription(): string;
+
+    /**
+     * Returns the configuration key for enabling/disabling the provider
+     * @return string
+     */
+    abstract public function getConfigKey(): string;
+
+    /**
+     * Generates the configuration UI for this provider (excluding enablement checkbox)
+     * Override this method if your provider needs custom configuration fields
+     * @param UiEditor $html
+     * @return string
+     */
+    public function getConfigHtml(UiEditor $html): string {
+        return "";
+    }
+
+    /**
+     * Returns true if the provider is enabled
+     * @return bool
+     */
+    public function isEnabled(): bool {
+        return BBConfig::getInstance()[$this->getConfigKey()] == "1";
+    }
+
+    /**
+     * Returns the ID of the main configuration field (e.g. API key) to be toggled
+     * @return string|null
+     */
+    public function getConfigFieldId(): ?string {
+        return null;
+    }
+
+    /**
+     * Saves the settings from the POST data
+     * Override this method if your provider needs custom save logic
+     * @param array $postData
+     * @return void
+     */
+    public function saveSettings(array $postData): void {
+        // Default implementation does nothing - generic saver handles standard fields
+    }
+
     /**
      * Returns the generic or product name, depending what user set in config or if
      * a product / generic name is available
@@ -120,31 +171,31 @@ protected function execute(string $url, string $method = METHOD_GET, array $form
             $class = get_class($e);
             switch ($class) {
                 case 'InvalidServerResponseException':
-                    API::logError("Could not connect to " . $this->providerName . ".", false);
+                    API::logError("Could not connect to " . $this->getName() . ".", false);
                     return null;
                 case 'UnauthorizedException':
-                    API::logError("Could not connect to " . $this->providerName . " - unauthorized");
+                    API::logError("Could not connect to " . $this->getName() . " - unauthorized");
                     return null;
                 case 'InvalidJsonResponseException':
-                    API::logError("Error parsing " . $this->providerName . " response: " . $e->getMessage(), false);
+                    API::logError("Error parsing " . $this->getName() . " response: " . $e->getMessage(), false);
                     return null;
                 case 'InvalidSSLException':
-                    API::logError("Could not connect to " . $this->providerName . " - invalid SSL certificate");
+                    API::logError("Could not connect to " . $this->getName() . " - invalid SSL certificate");
                     return null;
                 case 'InvalidParameterException':
-                    API::logError("Internal error: Invalid parameter passed to " . $this->providerName . ".");
+                    API::logError("Internal error: Invalid parameter passed to " . $this->getName() . ".");
                     return null;
                 case 'NotFoundException':
-                    API::logError("Server " . $this->providerName . " reported path not found.");
+                    API::logError("Server " . $this->getName() . " reported path not found.");
                     return null;
                 case 'LimitExceededException':
-                    API::logError("Connection limits exceeded for " . $this->providerName . ".");
+                    API::logError("Connection limits exceeded for " . $this->getName() . ".");
                     return null;
                 case 'InternalServerErrorException':
-                    API::logError($this->providerName . " reported internal error.");
+                    API::logError($this->getName() . " reported internal error.");
                     return null;
                 default:
-                    API::logError("Unknown error with " . $this->providerName . ": " . $e->getMessage());
+                    API::logError("Unknown error with " . $this->getName() . ": " . $e->getMessage());
                     return null;
             }
         }

incl/lookupProviders/ProviderAlbertHeijn.php

@@ -24,19 +24,34 @@ class ProviderAlbertHeijn extends LookupProvider {
 
     function __construct(string $apiKey = null) {
         parent::__construct($apiKey);
-        $this->providerName       = "Albert Heijn";
-        $this->providerConfigKey  = "LOOKUP_USE_AH";
         $this->ignoredResultCodes = array("404");
         $this->db                 = DatabaseConnection::getInstance();
     }
 
+
+    public function getName(): string {
+        return "Albert Heijn";
+    }
+
+    public function getDescription(): string {
+        return "Uses Albert Heijn API";
+    }
+
+    public function getConfigKey(): string {
+        return "LOOKUP_USE_AH";
+    }
+
+
+
+
+
     /**
      * Looks up a barcode
      * @param string $barcode The barcode to lookup
      * @return array|null Name of product, null if none found
      */
     public function lookupBarcode(string $barcode): ?array {
-        if (!$this->isProviderEnabled())
+        if (!$this->isEnabled())
             return null;
         if (strlen($barcode) >= 20)
 	        return null;

incl/lookupProviders/ProviderDebug.php

@@ -23,10 +23,25 @@ class ProviderDebug extends LookupProvider {
 
     function __construct(string $apiKey = null) {
         parent::__construct($apiKey);
-        $this->providerName      = "Debug Lookup";
-        $this->providerConfigKey = "USE_DEBUG_LOOKUP";
     }
 
+
+    public function getName(): string {
+        return "Debug Provider";
+    }
+
+    public function getDescription(): string {
+        return "Debug lookup provider";
+    }
+
+    public function getConfigKey(): string {
+        return "USE_DEBUG_LOOKUP";
+    }
+
+
+
+
+
     public function lookupBarcode(string $barcode): ?array {
         return self::createReturnArray(self::RETURN_STRING);
     }