Skip to content

Commit

Permalink
Fixed #1622 - Use doesObjectExistV2 instead of doesObjectExist for AW…
Browse files Browse the repository at this point in the history
…S adapter
  • Loading branch information
frankdejonge committed Jan 6, 2023
1 parent f593bf9 commit ea10034
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion AwsS3V3Adapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public function __construct(
public function fileExists(string $path): bool
{
try {
return $this->client->doesObjectExist($this->bucket, $this->prefixer->prefixPath($path), $this->options);
return $this->client->doesObjectExistV2($this->bucket, $this->prefixer->prefixPath($path), $this->options);

This comment has been minimized.

Copy link
@SergeyPodgornyy

SergeyPodgornyy Jan 17, 2023

doesObjectExistV2 accepts 4 arguments, but doesObjectExist only 3. It is not enough to just change the method name. DeleteMarker should be in use as a third argument. Maybe it makes sense to have it as false for the hotfix, as it was the behaviour before

This comment has been minimized.

Copy link
@SergeyPodgornyy

This comment has been minimized.

Copy link
@SergeyPodgornyy
} catch (Throwable $exception) {
throw UnableToCheckFileExistence::forLocation($path, $exception);
}
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"php": "^8.0.2",
"league/flysystem": "^3.10.0",
"league/mime-type-detection": "^1.0.0",
"aws/aws-sdk-php": "^3.132.4"
"aws/aws-sdk-php": "^3.220.0"
},
"conflict": {
"guzzlehttp/ringphp": "<1.1.1",
Expand Down

4 comments on commit ea10034

@Tklaversma1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frankdejonge are you aware this breaks the functionality in of ->exists()? Currently a lot of users using Laravel and using this package can no longer user the exists() function, since it returns an Exception, which then ends up in UnableToCheckFileExistence... See laravel/framework#45639

@tcgunel
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frankdejonge are you aware this breaks the functionality in of ->exists()? Currently a lot of users using Laravel and using this package can no longer user the exists() function, since it returns an Exception, which then ends up in UnableToCheckFileExistence... See laravel/framework#45639

i reverted from 3.15.0 to 3.10 because of ->exists() errors. doesn't seem to be fixed yet.

@Tklaversma
Copy link

@Tklaversma Tklaversma commented on ea10034 Jul 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frankdejonge are you aware this breaks the functionality in of ->exists()? Currently a lot of users using Laravel and using this package can no longer user the exists() function, since it returns an Exception, which then ends up in UnableToCheckFileExistence... See laravel/framework#45639

i reverted from 3.15.0 to 3.10 because of ->exists() errors. doesn't seem to be fixed yet.

Although less sufficient, but instead of decreasing the version, we use the get function with a try-catch as a workaround.

@tcgunel
Copy link

@tcgunel tcgunel commented on ea10034 Jul 14, 2023 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.