sausin / laravel-ovh

Wrapper for OVH Object Storage integration with laravel
MIT License
37 stars 12 forks source link

[BUG] Optional large object config produces unexpected results when not set #55

Closed wfeller closed 4 years ago

wfeller commented 4 years ago

Hi, I believe there's an issue with the large objects configuration.

As it's optional, I've removed it from my config file, and now I've got a container named "XXX_segments" in my OVH project, and I can't access the backups created by spatie/laravel-backup created these pasts 2 weeks through the API anymore 😅. Actually, I can, but they're empty in my XXX container, and they exist in the XXX_segments container (normal not-empty files in the XXX_segments container). My backups aren't large. It's weird that the backups are not moved from the XXX_segments to the XXX container somehow - but that's not the problem right now.

The backups created before I upgraded the package are OK. Some other files created since I upgraded the package (such as images) are also normally accessible. (only the new zip files are bugged)

Unless someone takes care of it before, I'll submit a PR this weekend to fix this, the problem comes from OVHServiceProvider. The large object config values (null) are passed even if no large object configuration is defined. It prevents the Flysystem config from returning the default value (see https://github.com/chrisnharvey/flysystem-openstack-swift/blob/58b8dd825c53f620c1658b42e1361763a1da3781/src/SwiftAdapter.php#L55).

See:

We should do something along these lines:

    return new Filesystem(
            $adapter,
            // $this->config->hasLargeObjectConfig() ? [/* Large object config data */] : []
            [
                'swiftLargeObjectThreshold' => $this->config->getSwiftLargeObjectThreshold(),
                'swiftSegmentSize' => $this->config->getSwiftSegmentSize(),
                'swiftSegmentContainer' => $this->config->getSwiftSegmentContainer(),
            ]
        );

Or simply (I believe simpler is better):

    return new Filesystem(
            $adapter,
            array_filter([
                'swiftLargeObjectThreshold' => $this->config->getSwiftLargeObjectThreshold(),
                'swiftSegmentSize' => $this->config->getSwiftSegmentSize(),
                'swiftSegmentContainer' => $this->config->getSwiftSegmentContainer(),
            ])
        );
sausin commented 4 years ago

Thanks for the comprehensive report @wfeller !! I've started the PR, will just take another look and merge it with a release afterwards.