symphonycms / remote_datasource

An improved datasource to fetch data from TXT, CSV, XML and JSON sources.
Other
14 stars 9 forks source link

Mixing datasource formats on one page fails #37

Closed kmeinke closed 5 years ago

kmeinke commented 6 years ago

Hello,

first of all thank you for this extension! the new version has improved a lot!

I spend 2 days debugging to find this bug... I use 2 different Remote_Datasource - one in XML and one in JSON Format - on the same page.

Line 9 of remote_datasource/data-sources/datasource.remote.php defines the $transformer class as static member. Line 1092 of remote_datasource/data-sources/datasource.remote.php only loads a transformer for given format if no transformer was choosen yet.

The first datasource (XML) is executed - and the XML tranformer is loaded. The 2nd datasource (JSON) is executed, and resuses the XML tranformer - wich fails cause... doh! :)

Because the transformer is a static member, choice is made once per page based on the first format encountered.

i suggest making $transformer an array for formats

    public static function getTransformer($format)
    {
        if (is_array(self::$transformer) && array_key_exists($format, self::$transformer)) {
            return self::$transformer[$format];
        }

        if (!is_array(self::$transformer)) {
            self::$transformer = Array();
        }

        $transformerFile = EXTENSIONS . '/remote_datasource/lib/class.' . strtolower($format) . '.php';
        if (file_exists($transformerFile)) {
            $classname = require_once $transformerFile;
            self::$transformer[$format] = new $classname;
            return self::$transformer[$format];
        }

        throw new Exception("could not load transformer [$transformerFile] for format [$format]");
    }
michael-e commented 6 years ago

This sounds like an important issue. @nitriques what do you think?

nitriques commented 6 years ago

@michael-e it is. Fixing it is easy. Fixing it while not crashing existing data source and ensure they still work will be harder.

@kmeinke Wanna send a PR with your modifications in order to discuss it better ?

Thanks.