q2a / question2answer

Question2Answer is a free and open source platform for Q&A sites, running on PHP/MySQL.
http://www.question2answer.org/
GNU General Public License v3.0
1.64k stars 629 forks source link

Improved metadata handling for plugin and themes #145

Closed pupi1985 closed 9 years ago

pupi1985 commented 9 years ago

I was thinking about going on with the splitting of plugin/theme metadata into a separate (matadata.php) file. Although it doesn't seem to be complex at all I have to say I stumbled with a silly issue I didn't thought about before.

This is what I think the matadata file for the plugins should be:

<?php
    return array(
        // Name of the plugin
        'name' => 'My plugin',
        // Minimum Question2Answer Version
        'min_q2a' => '1.6',
        // etc
    );

The matadata file should work fine and and faster for module loading. However, I missed the fact that plugins can be updated and they download a file from the web. Currently, the file is parsed the same way the local files are parsed. But if we turn to the new schema, in which I hope to get rid of the parsing, then we can not just include this new file.

These are the options I thought of:

  1. Ok, actually we can include a file from the outside world... but it is clearly too dangerous
  2. Download the metadata.php file and parse in a similar way as we used to (different regex, clearly). The downside is we still have to parse with regexes (only for the version checking)
  3. Use an additional metadata.json file to store the plugin data. Clients will download and parse the json. The downside is we duplicate plugin information
  4. Get rid of metadata.php and always parse the metadata.json file. The downside is it decreases performance when parsing on each module load

So I don't like any alternative (at least from the ones I thought). I would at first sight remove 1 and 4. Between 2 and 3, the former gets the core dirty while the latter gets plugins dirty. The core is more important so I'd go for 3.

I'd work on this but I first need to know whether it would be merged or not :) If not, what alternative would?

q2apro commented 9 years ago

Just an idea / sidenote: I would add a priority to the metadata (or "load number"). This way, the overriding issue caused by plugins loading in certain undefined order could be reduced.

pupi1985 commented 9 years ago

This has nothing to do with the order of the data but rather on where and how to store it and retrieve it. By the way, the order is not undefined: it comes from the order in which directories are retrieved.

svivian commented 9 years ago

Ack, this is a snag I should have seen coming. The way the current system works is actually quite useful since you can easily point to the code file with the metadata on Github or elsewhere. Regarding your options:

  1. We can't include PHP code from elsewhere because we'd just get a blank page (the same as loading it direct in the browser). Technically if someone links the source code (like on Github) we could eval() it but of course that's a huge security risk so won't be happening.
  2. Feasible but a little complex. Although most people link to a PHP source file on Github I don't see this as very flexible (e.g. closed-source plugins won't want to link PHP code).
  3. Don't Repeat Yourself :)
  4. Actually this seems most feasible to me.
  5. One additional option is to have a dead simple VERSION.txt file that plugin authors store somewhere, listing the current version. But we also check the minimum Q2A version and PHP version so we'd end up with something like JSON in the end anyway...

I will take a quick look into performance, but I think including a PHP file of variables will not be significantly faster than reading a JSON file. And each JSON file will be much smaller than the current qa-plugin.php or qa-styles.css files.

I think whatever we decide, the decision must be made before 1.7 final is released. I don't want to have v1.7 with a new system (metadata.php) in the language files and then scrap it in v1.8.

svivian commented 9 years ago

Did some quick benchmarking and parsing JSON is actually slightly faster than the include method, although completely negligible (0.1s faster over 10000 iterations).

The only issue now is that the JSON functions are not in PHP 5.1. There are some polyfills out there - they appear to be a bit slower but again it is negligible.

Seems like using JSON would have the best balance of flexibility, performance and ease-of-use.

pupi1985 commented 9 years ago

Great. Thanks for performing the benchmark. Lesson learnt: parsing JSON is equivalent in performance to parsing PHP. So JSON it is. Something like this should be quite backwards compatible and not require many changes in current PHP code:

{
    "name": "Basic AdSense",
    "uri": "",
    "description": "Provides a basic widget for displaying Google Adsense ads",
    "version": "1.0",
    "date": "2011-03-27",
    "author": "Question2Answer",
    "author_uri": "http://www.question2answer.org",
    "license": "GPLv2",
    "update": "",
    "min_q2a": "1.4",
    "min_php": ""
}

Regarding JSON support, I was thinking about using a similar a approach to what the facebook plugin does and add the JSON.php file to the vendor folder then update the facebook plugin and the new metadata fetcher to first try to use the native json_encode (which could not be there even in PHP 5.2 in the awkward case in which PHP had not been compiled with JSON support).

In short, the scope of these tasks will be:

I understand your concern about the timings. I guess I can finish this during the weekend. I'll provide an update then.

svivian commented 9 years ago

Haha nice, I didn't realize that was a separate standalone JSON library. I'd already done some searching for JSON libraries and by coincidence found one that is simply a wrapper (polyfill) for that same file!

I'll set up the JSON stuff in the vendor directory tomorrow (Friday) then if you're happy to do the rest that's great! The first step would be to remove the metadata.php part, then even if the rest isn't finished soon at least we don't have that part cluttering up v1.7 :)

svivian commented 9 years ago

Latest dev branch now has json_encode and json_decode available for all versions. No need to include anything, just use the functions as necessary.

pupi1985 commented 9 years ago

Implemented on #155