Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(federation): Allow cloud federation providers to handle unsuccess… #43884

Merged
merged 1 commit into from
Feb 28, 2024
Merged

fix(federation): Allow cloud federation providers to handle unsuccess… #43884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions lib/private/Federation/CloudFederationProviderManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
use OCP\Federation\ICloudFederationShare;
use OCP\Federation\ICloudIdManager;
use OCP\Http\Client\IClientService;
use OCP\Http\Client\IResponse;
use OCP\IConfig;
use OCP\OCM\Exceptions\OCMProviderException;
use OCP\OCM\IOCMDiscoveryService;
Expand Down Expand Up @@ -111,6 +112,9 @@ public function getCloudFederationProvider($resourceType) {
}
}

/**
* @deprecated 29.0.0 - Use {@see sendCloudShare()} instead and handle errors manually
*/
public function sendShare(ICloudFederationShare $share) {
$cloudID = $this->cloudIdManager->resolveCloudId($share->getShareWith());
try {
Expand Down Expand Up @@ -147,10 +151,39 @@ public function sendShare(ICloudFederationShare $share) {
return false;
}

/**
* @param ICloudFederationShare $share
* @return IResponse
* @throws OCMProviderException
*/
public function sendCloudShare(ICloudFederationShare $share): IResponse {
$cloudID = $this->cloudIdManager->resolveCloudId($share->getShareWith());
$ocmProvider = $this->discoveryService->discover($cloudID->getRemote());

$client = $this->httpClientService->newClient();
try {
return $client->post($ocmProvider->getEndPoint() . '/shares', [
'body' => json_encode($share->getShare()),
'headers' => ['content-type' => 'application/json'],
'verify' => !$this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates', false),
'timeout' => 10,
'connect_timeout' => 10,
]);
} catch (\Throwable $e) {
$this->logger->error('Error while sending share to federation server: ' . $e->getMessage(), ['exception' => $e]);
try {
return $client->getResponseFromThrowable($e);
} catch (\Throwable $e) {
throw new OCMProviderException($e->getMessage(), $e->getCode(), $e);
}
}
}

/**
* @param string $url
* @param ICloudFederationNotification $notification
* @return array|false
* @deprecated 29.0.0 - Use {@see sendCloudNotification()} instead and handle errors manually
*/
public function sendNotification($url, ICloudFederationNotification $notification) {
try {
Expand Down Expand Up @@ -180,6 +213,34 @@ public function sendNotification($url, ICloudFederationNotification $notificatio
return false;
}

/**
* @param string $url
* @param ICloudFederationNotification $notification
* @return IResponse
* @throws OCMProviderException
*/
public function sendCloudNotification(string $url, ICloudFederationNotification $notification): IResponse {
$ocmProvider = $this->discoveryService->discover($url);

$client = $this->httpClientService->newClient();
try {
return $client->post($ocmProvider->getEndPoint() . '/notifications', [
'body' => json_encode($notification->getMessage()),
'headers' => ['content-type' => 'application/json'],
'verify' => !$this->config->getSystemValueBool('sharing.federation.allowSelfSignedCertificates', false),
'timeout' => 10,
'connect_timeout' => 10,
]);
} catch (\Throwable $e) {
$this->logger->error('Error while sending notification to federation server: ' . $e->getMessage(), ['exception' => $e]);
try {
return $client->getResponseFromThrowable($e);
} catch (\Throwable $e) {
throw new OCMProviderException($e->getMessage(), $e->getCode(), $e);
}
}
}

/**
* check if the new cloud federation API is ready to be used
*
Expand Down
16 changes: 16 additions & 0 deletions lib/private/Http/Client/Client.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,22 @@ public function options(string $uri, array $options = []): IResponse {
return new Response($response);
}

/**
* Get the response of a Throwable thrown by the request methods when possible
*
* @param \Throwable $e
* @return IResponse
* @throws \Throwable When $e did not have a response
* @since 29.0.0
*/
public function getResponseFromThrowable(\Throwable $e): IResponse {
if (method_exists($e, 'hasResponse') && method_exists($e, 'getResponse') && $e->hasResponse()) {
return new Response($e->getResponse());
}

throw $e;
}

protected function wrapGuzzlePromise(PromiseInterface $promise): IPromise {
return new GuzzlePromiseAdapter(
$promise,
Expand Down
22 changes: 22 additions & 0 deletions lib/public/Federation/ICloudFederationProviderManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
*/
namespace OCP\Federation;

use OCP\Http\Client\IResponse;
use OCP\OCM\Exceptions\OCMProviderException;

/**
* Class ICloudFederationProviderManager
*
Expand Down Expand Up @@ -80,9 +83,18 @@ public function getCloudFederationProvider($resourceType);
* @return mixed
*
* @since 14.0.0
* @deprecated 29.0.0 - Use {@see sendCloudShare()} instead and handle errors manually
*/
public function sendShare(ICloudFederationShare $share);

/**
* @param ICloudFederationShare $share
* @return IResponse
* @throws OCMProviderException
* @since 29.0.0
*/
public function sendCloudShare(ICloudFederationShare $share): IResponse;
Copy link
Contributor

Choose a reason for hiding this comment

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

why add two new methods when you could modify the exisitng one?

Copy link
Member Author

Choose a reason for hiding this comment

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

because it would now throw instead of returning false, which would break the current consumers?


/**
* send notification about existing share
*
Expand All @@ -91,9 +103,19 @@ public function sendShare(ICloudFederationShare $share);
* @return array|false
*
* @since 14.0.0
* @deprecated 29.0.0 - Use {@see sendCloudNotification()} instead and handle errors manually
*/
public function sendNotification($url, ICloudFederationNotification $notification);

/**
* @param string $url
* @param ICloudFederationNotification $notification
* @return IResponse
* @throws OCMProviderException
* @since 29.0.0
*/
public function sendCloudNotification(string $url, ICloudFederationNotification $notification): IResponse;

/**
* check if the new cloud federation API is ready to be used
*
Expand Down
10 changes: 10 additions & 0 deletions lib/public/Http/Client/IClient.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,16 @@ public function delete(string $uri, array $options = []): IResponse;
*/
public function options(string $uri, array $options = []): IResponse;

/**
* Get the response of a Throwable thrown by the request methods when possible
*
* @param \Throwable $e
* @return IResponse
* @throws \Throwable When $e did not have a response
* @since 29.0.0
*/
public function getResponseFromThrowable(\Throwable $e): IResponse;
Copy link
Member Author

Choose a reason for hiding this comment

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

I added and used this method. I also put it in OCP, because it's not the first time where I find myself wanting to get the response of a "Guzzle ClientException" as an IResponse, without new \OC\…\Response() violating private class access.


/**
* Sends an asynchronous GET request
* @param string $uri
Expand Down
Loading