fmbiete / Z-Push-contrib

Z-Push fork with changes that I will try to contrib
GNU Affero General Public License v3.0
134 stars 61 forks source link

per-user backend configurations #249

Open Takika opened 8 years ago

Takika commented 8 years ago

I have a very strange Z-Push and webmail setup with Z-Push-contrib, Roundcube, LDAP addressbook, Dovecot IMAP and SabreDAV as CalDAV. I made a Roundcube plugin where my users can control what they want to sync (globally and per device). To use this config from RC I modified the BackendCombinedConfig class, which sends not just the name of the backend but a config variable to the backend class too. Changed this:

        $this->config = BackendCombinedConfig::GetBackendCombinedConfig();
        $backend_values = array_unique(array_values($this->config['folderbackend']));
        foreach ($backend_values as $i) {
            ZPush::IncludeBackend($this->config['backends'][$i]['name']);
            $this->backends[$i] = new $this->config['backends'][$i]['name']();
        }

to this:

        $this->config = BackendCombinedConfig::GetBackendCombinedConfig();

        if (is_array($this->config) && array_key_exists('folderbackend', $this->config)) {
            $backend_values = array_unique(array_values($this->config['folderbackend']));
            foreach ($backend_values as $i) {
                // load and instatiate backend
                $backend_info = $this->config['backends'][$i];
                ZPush::IncludeBackend($backend_info['name']);
                $this->backends[$i] = new $backend_info['name']($backend_info['config']);
            }
        }

And modified my backend classes to use this new config parameter (if it isn't empty) instead of hardcoded define-s. What do you think about these changes? :) If you want I can show some examples from any backend class :)

fmbiete commented 8 years ago

This could be problematic if an user doesn't need BackendCombined, but another of the backends.

For instance, if I only use BackendIMAP its constructor would require an argument (the configuration variable), and Z-Push core won't pass it, it just call the constructor (normal operations won't even call the constructor and directly hit the Logon method).

Takika commented 8 years ago

My solution from the caldav.php:

    public function BackendCalDAV($config = array()) {
        if (sizeof($config) > 0) {
            $this->config = $config;
        }
        else {
            $this->config['protocol'] = CALDAV_PROTOCOL;
            $this->config['host']     = CALDAV_SERVER;
            $this->config['port']     = CALDAV_PORT;
            $this->config['path']     = CALDAV_PATH;
            $this->config['personal'] = CALDAV_PERSONAL;
            $this->config['sync']     = CALDAV_SUPPORTS_SYNC;
            $this->config['debug']    = true;
        }
    }

And after this everywhere I use $this->config array.

fmbiete commented 8 years ago

But, as I said you could be creating instances of your backend without passing config array. You would be using defaults values. I don't see the benefit, since you also have to define the constants CALDAV...

Takika commented 8 years ago

Yes, if a user doesn't want to use the BackendCombined backend he/she have to set the correct config variables in the backend specific config.php .