wp-media / backwpup

BackWPup - WordPress Backup Plugin
https://backwpup.com
GNU General Public License v2.0
88 stars 35 forks source link

Filter to extend list of S3 destinations #82

Closed noplanman closed 5 years ago

noplanman commented 5 years ago

This PR adds a new filter backwpup_s3_destinations which allows for the addition of new destinations.

e.g.

add_filter( 'backwpup_s3_destinations', function ( $destinations ) {
    return array_merge( array(
        new BackWPup_S3_Destination( __( 'DigitalOcean: AMS3', 'backwpup' ), 'https://ams3.digitaloceanspaces.com', 'ams3' ),
        new BackWPup_S3_Destination( __( 'Scaleway: AMS', 'backwpup' ), 'https://s3.nl-ams.scw.cloud', 'nl-ams' ),
    ), $destinations );
} );

/*
!!!OUTDATED!!!
add_filter( 'backwpup_s3_destinations', function ( $destinations ) {
    return array_merge( array(
        array(
            'label'    => __( 'DigitalOcean: AMS3', 'backwpup' ),
            'region'   => 'ams3',
            'base_url' => 'https://ams3.digitaloceanspaces.com',
        ),
        array(
            'label'    => __( 'Scaleway: AMS', 'backwpup' ),
            'region'   => 'nl-ams',
            'base_url' => 'https://s3.nl-ams.scw.cloud',
        ),
    ), $destinations );
} );
*/

NOTE: Due to the fact that the base URL is selected via region, it makes sense to add the custom entries before the included ones, just in case the region name is the same. Or, the built-in ones could be replaced completely by just returning an array of custom entries (without array_merge( ..., $destinations ))

79 #81

widoz commented 5 years ago

Hi @noplanman and thanks for the pull request. Could you make the code php 5.3 compliant? Actually I see short hand version of array declarations.

The idea is good, have you considered to have a data class for the list instead of an array?

noplanman commented 5 years ago

Done @ PHP 5.3 compliance.

Regarding the data class, hadn't thought of that, as I didn't want to over-complicate. But maybe it would make sense :thinking:

noplanman commented 5 years ago

@widoz I've introduced a BackWPup_S3_Destination data class.

So instead of an array of arrays, now an array of objects can be passed to the filter:

add_filter( 'backwpup_s3_destinations', function ( $destinations ) {
    return array_merge( array(
        new BackWPup_S3_Destination( __( 'DigitalOcean: AMS3', 'backwpup' ), 'https://ams3.digitaloceanspaces.com', 'ams3' ),
        new BackWPup_S3_Destination( __( 'Scaleway: AMS', 'backwpup' ), 'https://s3.nl-ams.scw.cloud', 'nl-ams' ),
    ), $destinations );
} );

I've also tried to make the multipart upload bit more dynamic, not limiting it just to Google and Dreamhost, but being able to set it in the destination itself. This is still WIP as the way destinations are saved at the moment, the region is key. In my opinion, the base URL and the region together should be key.

noplanman commented 5 years ago

Ping @widoz

danielhuesken commented 5 years ago

Hi, i will implemet it in a similar way but add some changes for the pro version. Thanks for you work.

noplanman commented 5 years ago

@danielhuesken Great! Of course feel free to use any bits of my code.

If I understand correctly, the filtering will be available for the free version too, right? Just with some extras for the Pro version?

danielhuesken commented 5 years ago

@noplanman Yes, but i extend it a bit.

noplanman commented 5 years ago

Awesome, really looking forward to having it in the core plugin :pray: