From 5ce9a9d7cb070b06cc032c8424eeba8da0c5a75c Mon Sep 17 00:00:00 2001 From: Ashim Shrestha Date: Mon, 15 Jun 2026 14:22:51 +0545 Subject: [PATCH 1/2] fix: openproject avatar remains stale due to long browser cache Signed-off-by: Ashim Shrestha --- CHANGELOG.md | 4 + lib/Controller/OpenProjectAPIController.php | 21 ++- lib/Service/OpenProjectAPIService.php | 12 +- .../OpenProjectAPIControllerTest.php | 125 ++++++++++++------ 4 files changed, 112 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd8def738..6fa1993cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added +### Fixed + +- Fix: OpenProject avatar remains stale [#1058](https://github.com/nextcloud/integration_openproject/pull/1058) + ### Changed ### Removed diff --git a/lib/Controller/OpenProjectAPIController.php b/lib/Controller/OpenProjectAPIController.php index 6b8bd0726..28662c16f 100644 --- a/lib/Controller/OpenProjectAPIController.php +++ b/lib/Controller/OpenProjectAPIController.php @@ -16,9 +16,9 @@ use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; -use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\DataDownloadResponse; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Http\Response; use OCP\AppFramework\OCSController; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; @@ -70,7 +70,7 @@ public function getOpenProjectUrl(): DataResponse { * * @param string $userId * @param string $userName - * @return DataDisplayResponse|DataDownloadResponse + * @return DataDownloadResponse|Response * @throws \OCP\Files\NotFoundException * @throws \OCP\Files\NotPermittedException * @throws \OCP\Lock\LockedException @@ -81,11 +81,20 @@ public function getOpenProjectAvatar(string $userId = '', string $userName = '') $result = $this->openprojectAPIService->getOpenProjectAvatar( $userId, $userName, $this->userId ); - $response = new DataDownloadResponse( - $result['avatar'], 'avatar', $result['type'] ?? '' + + $etag = '"' . $result['etag'] . '"'; + $headers = [ + 'Cache-Control' => 'no-cache', + 'ETag' => $etag, + ]; + $ifNoneMatch = $this->request->getHeader('If-None-Match'); + if ($ifNoneMatch && $ifNoneMatch === $etag) { + return new Response(Http::STATUS_NOT_MODIFIED, $headers); + } + + return new DataDownloadResponse( + $result['avatar'], 'avatar', $result['type'], Http::STATUS_OK, $headers ); - $response->cacheFor(60 * 60 * 24); - return $response; } /** diff --git a/lib/Service/OpenProjectAPIService.php b/lib/Service/OpenProjectAPIService.php index e76e73cf1..4f41fb59b 100644 --- a/lib/Service/OpenProjectAPIService.php +++ b/lib/Service/OpenProjectAPIService.php @@ -338,7 +338,7 @@ private function searchRequest(string $userId, array $filters, bool $onlyLinkabl * @param string $openprojectUserId * @param string $openprojectUserName * @param string $nextcloudUserId - * @return array{avatar: string, type?: string} + * @return array{avatar: string, type: string, etag: string} * @throws \OCP\Files\NotFoundException * @throws \OCP\Files\NotPermittedException * @throws \OCP\Lock\LockedException @@ -387,12 +387,18 @@ public function getOpenProjectAvatar( return [ 'avatar' => $imageData, 'type' => $imageMimeType , + 'etag' => md5($imageData), ]; } catch (ServerException | ClientException | ConnectException | OpenprojectAvatarErrorException | Exception $e) { $this->logger->debug('Error while getting OpenProject avatar for user ' . $openprojectUserId . ': ' . $e->getMessage(), ['app' => $this->appName]); $avatar = $this->avatarManager->getGuestAvatar($openprojectUserName); - $avatarContent = $avatar->getFile(64)->getContent(); - return ['avatar' => $avatarContent]; + $avatarFile = $avatar->getFile(64); + $avatarContent = $avatarFile->getContent(); + return [ + 'avatar' => $avatarContent, + 'type' => $avatarFile->getMimeType(), + 'etag' => md5($avatarContent), + ]; } } diff --git a/tests/lib/Controller/OpenProjectAPIControllerTest.php b/tests/lib/Controller/OpenProjectAPIControllerTest.php index 3fcd72179..1c94770b2 100644 --- a/tests/lib/Controller/OpenProjectAPIControllerTest.php +++ b/tests/lib/Controller/OpenProjectAPIControllerTest.php @@ -205,50 +205,67 @@ public function testGetNotificationsErrorResponse(string $authorizationMethod) { } /** - * - * @param string $authorizationMethod - * @return void - * - * @dataProvider getAuthorizationMethodDataProvider + * Data provider for testGetOpenProjectAvatar */ - public function testGetOpenProjectAvatar(string $authorizationMethod) { - $service = $this->getMockBuilder(OpenProjectAPIService::class) - ->disableOriginalConstructor() - ->onlyMethods(['getOpenProjectAvatar', 'getAccessToken']) - ->getMock(); - $service - ->method('getAccessToken') - ->willReturn('123'); - $service->expects($this->once()) - ->method('getOpenProjectAvatar') - ->with( - 'id', 'name' - ) - ->willReturn(['avatar' => 'some image data', 'type' => 'image/png']); - $controller = $this->getOpenProjectAPIControllerMock([ - 'openProjectAPIService' => $service, - 'config' => $this->getConfigMock($authorizationMethod), - ]); - $response = $controller->getOpenProjectAvatar('id', 'name'); - $this->assertSame('some image data', $response->render()); - $this->assertMatchesRegularExpression( - '/attachment; filename="?avatar"?/', - $response->getHeaders()["Content-Disposition"] - ); - $this->assertSame( - "image/png", - $response->getHeaders()["Content-Type"] - ); + public function getOpenProjectAvatarDataProvider(): array { + return [ + 'OAuth: returns 200 OK when If-None-Match differs from ETag' => [ + 'authorizationMethod' => OpenProjectAPIService::AUTH_METHOD_OAUTH, + 'etag' => 'some etag', + 'ifNoneMatch' => '"different etag"', + 'contentType' => 'image/jpeg', + 'expectedStatusCode' => Http::STATUS_OK, + ], + 'OAuth: returns 200 OK if If-None-Match is empty' => [ + 'authorizationMethod' => OpenProjectAPIService::AUTH_METHOD_OAUTH, + 'etag' => 'some etag', + 'ifNoneMatch' => '', + 'contentType' => 'image/jpeg', + 'expectedStatusCode' => Http::STATUS_OK + ], + 'OAuth: returns 304 Not Modified when If-None-Match matches ETag' => [ + 'authorizationMethod' => OpenProjectAPIService::AUTH_METHOD_OAUTH, + 'etag' => 'some etag', + 'ifNoneMatch' => '"some etag"', + 'contentType' => 'image/jpeg', + 'expectedStatusCode' => Http::STATUS_NOT_MODIFIED, + ], + 'OIDC: returns 200 OK when If-None-Match differs from ETag' => [ + 'authorizationMethod' => OpenProjectAPIService::AUTH_METHOD_OIDC, + 'etag' => 'some etag', + 'ifNoneMatch' => '"different etag"', + 'contentType' => 'image/jpeg', + 'expectedStatusCode' => Http::STATUS_OK, + ], + 'OIDC: returns 200 OK if If-None-Match is empty' => [ + 'authorizationMethod' => OpenProjectAPIService::AUTH_METHOD_OIDC, + 'etag' => 'some etag', + 'ifNoneMatch' => '', + 'contentType' => 'image/jpeg', + 'expectedStatusCode' => Http::STATUS_OK + ], + 'OIDC: returns 304 Not Modified when If-None-Match matches ETag' => [ + 'authorizationMethod' => OpenProjectAPIService::AUTH_METHOD_OIDC, + 'etag' => 'some etag', + 'ifNoneMatch' => '"some etag"', + 'contentType' => 'image/jpeg', + 'expectedStatusCode' => Http::STATUS_NOT_MODIFIED, + ] + ]; } /** * + * @dataProvider getOpenProjectAvatarDataProvider * @param string $authorizationMethod + * @param string $etag + * @param string $ifNoneMatch + * @param string $contentType + * @param int $expectedStatusCode * @return void * - * @dataProvider getAuthorizationMethodDataProvider */ - public function testGetOpenProjectAvatarNoType(string $authorizationMethod) { + public function testGetOpenProjectAvatar(string $authorizationMethod, string $etag, string $ifNoneMatch, string $contentType, int $expectedStatusCode): void { $service = $this->getMockBuilder(OpenProjectAPIService::class) ->disableOriginalConstructor() ->onlyMethods(['getOpenProjectAvatar', 'getAccessToken']) @@ -261,18 +278,44 @@ public function testGetOpenProjectAvatarNoType(string $authorizationMethod) { ->with( 'id', 'name' ) - ->willReturn(['avatar' => 'some image data']); + ->willReturn(['avatar' => 'some image data', 'type' => $contentType, 'etag' => $etag]); + $request = $this->createMock(IRequest::class); + $request->expects($this->once())->method('getHeader')->with('If-None-Match')->willReturn($ifNoneMatch); $controller = $this->getOpenProjectAPIControllerMock([ 'openProjectAPIService' => $service, 'config' => $this->getConfigMock($authorizationMethod), + 'request' => $request, ]); $response = $controller->getOpenProjectAvatar('id', 'name'); - $this->assertSame('some image data', $response->render()); - $this->assertMatchesRegularExpression( - '/attachment; filename="?avatar"?/', - $response->getHeaders()["Content-Disposition"] + $etag = '"' . $etag . '"'; + $headers = $response->getHeaders(); + $this->assertSame( + "no-cache", + $headers["Cache-Control"] + ); + $this->assertSame( + $etag, + $headers["ETag"] + ); + $this->assertSame( + $expectedStatusCode, + $response->getStatus() ); - $this->assertEmpty($response->getHeaders()["Content-Type"]); + if ($etag === $ifNoneMatch) { + $this->assertEmpty($response->render()); + $this->assertArrayNotHasKey("Content-Disposition", $headers); + $this->assertArrayNotHasKey("Content-Type", $headers); + } else { + $this->assertSame('some image data', $response->render()); + $this->assertMatchesRegularExpression( + '/attachment; filename="?avatar"?/', + $headers["Content-Disposition"] + ); + $this->assertSame( + $contentType, + $headers["Content-Type"] + ); + } } /** From 84223a9ed1e32ca9318c519a6b9959fc6e6be427 Mon Sep 17 00:00:00 2001 From: Ashim Shrestha Date: Tue, 23 Jun 2026 10:54:44 +0545 Subject: [PATCH 2/2] chore: fix psalm error Signed-off-by: Ashim Shrestha --- .../lib/Controller/OpenProjectAPIControllerTest.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/lib/Controller/OpenProjectAPIControllerTest.php b/tests/lib/Controller/OpenProjectAPIControllerTest.php index 1c94770b2..40a7bbde7 100644 --- a/tests/lib/Controller/OpenProjectAPIControllerTest.php +++ b/tests/lib/Controller/OpenProjectAPIControllerTest.php @@ -210,42 +210,42 @@ public function testGetNotificationsErrorResponse(string $authorizationMethod) { public function getOpenProjectAvatarDataProvider(): array { return [ 'OAuth: returns 200 OK when If-None-Match differs from ETag' => [ - 'authorizationMethod' => OpenProjectAPIService::AUTH_METHOD_OAUTH, + 'authorizationMethod' => Application::AUTH_METHOD_OAUTH, 'etag' => 'some etag', 'ifNoneMatch' => '"different etag"', 'contentType' => 'image/jpeg', 'expectedStatusCode' => Http::STATUS_OK, ], 'OAuth: returns 200 OK if If-None-Match is empty' => [ - 'authorizationMethod' => OpenProjectAPIService::AUTH_METHOD_OAUTH, + 'authorizationMethod' => Application::AUTH_METHOD_OAUTH, 'etag' => 'some etag', 'ifNoneMatch' => '', 'contentType' => 'image/jpeg', 'expectedStatusCode' => Http::STATUS_OK ], 'OAuth: returns 304 Not Modified when If-None-Match matches ETag' => [ - 'authorizationMethod' => OpenProjectAPIService::AUTH_METHOD_OAUTH, + 'authorizationMethod' => Application::AUTH_METHOD_OAUTH, 'etag' => 'some etag', 'ifNoneMatch' => '"some etag"', 'contentType' => 'image/jpeg', 'expectedStatusCode' => Http::STATUS_NOT_MODIFIED, ], 'OIDC: returns 200 OK when If-None-Match differs from ETag' => [ - 'authorizationMethod' => OpenProjectAPIService::AUTH_METHOD_OIDC, + 'authorizationMethod' => Application::AUTH_METHOD_OIDC, 'etag' => 'some etag', 'ifNoneMatch' => '"different etag"', 'contentType' => 'image/jpeg', 'expectedStatusCode' => Http::STATUS_OK, ], 'OIDC: returns 200 OK if If-None-Match is empty' => [ - 'authorizationMethod' => OpenProjectAPIService::AUTH_METHOD_OIDC, + 'authorizationMethod' => Application::AUTH_METHOD_OIDC, 'etag' => 'some etag', 'ifNoneMatch' => '', 'contentType' => 'image/jpeg', 'expectedStatusCode' => Http::STATUS_OK ], 'OIDC: returns 304 Not Modified when If-None-Match matches ETag' => [ - 'authorizationMethod' => OpenProjectAPIService::AUTH_METHOD_OIDC, + 'authorizationMethod' => Application::AUTH_METHOD_OIDC, 'etag' => 'some etag', 'ifNoneMatch' => '"some etag"', 'contentType' => 'image/jpeg',