Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .github/workflows/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ jobs:
- { name: "AuditEventTypesTest", filter: "--filter AuditEventTypesTest" }
- { name: "GuzzleTracingTest", filter: "--filter GuzzleTracingTest" }
- { name: "Repositories", filter: "--filter tests/Repositories/" }
- { name: "Services", filter: "--filter tests/Unit/Services/" }
Comment on lines 65 to +66
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

The workflow runs PHPUnit as vendor/bin/phpunit ${{ matrix.suite.filter }}; passing a directory should be done as a positional path (like tests/Unit/Entities/), not via --filter (which filters by test name regex). With --filter tests/Unit/Services/ this suite may execute zero tests. Suggest changing the entry to filter: "tests/Unit/Services/" (or use an actual name regex if you intended --filter).

Suggested change
- { name: "Repositories", filter: "--filter tests/Repositories/" }
- { name: "Services", filter: "--filter tests/Unit/Services/" }
- { name: "Repositories", filter: "tests/Repositories/" }
- { name: "Services", filter: "tests/Unit/Services/" }

Copilot uses AI. Check for mistakes.
env:
OTEL_SERVICE_ENABLED: false
APP_ENV: testing
Expand Down
6 changes: 3 additions & 3 deletions app/Services/Apis/AbstractOAuth2Api.php
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ protected function getAccessToken():?string{
Log::debug("AbstractOAuth2Api::getAccessToken - access token is empty, getting new one");
$client = $this->getIDPClient();
$appConfig = $this->getAppConfig();
$scopes = $appConfig['scopes'] ?? [];
Log::debug(sprintf( "AbstractOAuth2Api::getAccessToken - got scopes %s", implode(' ', $scopes)));
$scopes = $appConfig['scopes'] ?? '';
Log::debug(sprintf( "AbstractOAuth2Api::getAccessToken - got scopes %s", $scopes));
// Try to get an access token using the client credentials grant.
$accessToken = $client->getAccessToken('client_credentials', ['scope' => $scopes]);
$token = $accessToken->getToken();
Expand Down Expand Up @@ -163,4 +163,4 @@ protected function getAccessToken():?string{
protected function cleanAccessToken():void{
$this->cacheService->delete($this->getAccessTokenCacheKey());
}
}
}
85 changes: 85 additions & 0 deletions tests/Unit/Services/AbstractOAuth2ApiScopesTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
<?php namespace Tests\Unit\Services;
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

File header doesn't follow the existing test-file convention in this repo: other tests use <?php on its own line, a blank line, then namespace ...;. Keeping it consistent improves readability and tooling support (linters/formatters).

Suggested change
<?php namespace Tests\Unit\Services;
<?php
namespace Tests\Unit\Services;

Copilot uses AI. Check for mistakes.
/**
* Copyright 2026 OpenStack Foundation
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
**/
use App\Services\Apis\MailApi;
use Illuminate\Support\Facades\Config;
use libs\utils\ICacheService;
use Mockery;
use Tests\TestCase;

/**
* Class AbstractOAuth2ApiScopesTest
* Regression test for implode() TypeError when scopes config is a string.
* @see https://github.com/OpenStackweb/summit-api/issues/XXX
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Update placeholder issue reference.

The @see annotation contains a placeholder XXX that should be updated to reference the actual issue or PR number (e.g., PR #521 or the linked ClickUp task).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/Unit/Services/AbstractOAuth2ApiScopesTest.php` at line 23, Update the
placeholder `@see` annotation in the docblock of AbstractOAuth2ApiScopesTest
(tests/Unit/Services/AbstractOAuth2ApiScopesTest.php) so it points to the real
issue or PR instead of "XXX" (for example replace
"https://github.com/OpenStackweb/summit-api/issues/XXX" with the actual issue/PR
URL or ClickUp task link); edit the `@see` line in the class docblock to include
the correct identifier or remove the tag if no reference is needed.

Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

The @see link still contains a placeholder (issues/XXX). Please replace it with the real issue/ClickUp link or remove it to avoid leaving dead references in the codebase.

Suggested change
* @see https://github.com/OpenStackweb/summit-api/issues/XXX

Copilot uses AI. Check for mistakes.
* @package Tests\Unit\Services
*/
class AbstractOAuth2ApiScopesTest extends TestCase
{
protected function tearDown(): void
{
Mockery::close();
parent::tearDown();
}

/**
* Regression test: getAccessToken() must not throw TypeError when
* scopes is a string from env(). Previously, implode(' ', $scopes)
* crashed because $scopes was a string, not an array.
*
* @dataProvider scopesProvider
*/
public function testGetAccessTokenHandlesVariousScopesTypes($scopeValue, string $description)
{
Config::set('idp.authorization_endpoint', 'https://idp.test/authorize');
Config::set('idp.token_endpoint', 'https://idp.test/token');
Config::set('mail.service_base_url', 'https://mail.test');
Config::set('mail.service_client_id', 'test-client');
Config::set('mail.service_client_secret', 'test-secret');
Config::set('mail.service_client_scopes', $scopeValue);
Config::set('curl.timeout', 1);
Config::set('curl.allow_redirects', false);
Config::set('curl.verify_ssl_cert', true);

$cacheService = Mockery::mock(ICacheService::class);
$cacheService->shouldReceive('getSingleValue')->andReturn(null);

$api = new MailApi($cacheService);

$reflection = new \ReflectionMethod($api, 'getAccessToken');
$reflection->setAccessible(true);

try {
$reflection->invoke($api);
} catch (\TypeError $e) {
Comment on lines +56 to +63
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

This unit test currently triggers a real OAuth2 token request (it relies on a timeout/connection failure). That can make CI slow or flaky depending on DNS/network policies. Consider mocking the HTTP layer (e.g., a Guzzle MockHandler) or injecting a stubbed OAuth2 provider so the test deterministically exercises the scopes handling without any outbound network call.

Copilot uses AI. Check for mistakes.
$this->fail("TypeError thrown with {$description}: " . $e->getMessage());
} catch (\Exception $e) {
// Connection/HTTP errors are expected since IDP is not reachable.
// The critical assertion is that no TypeError was thrown from implode().
$this->assertNotInstanceOf(
\TypeError::class,
$e,
"No TypeError should occur with {$description}"
);
}
}

public static function scopesProvider(): array
{
return [
'string scopes (space-separated)' => ['scope1 scope2', 'space-separated string scopes'],
'single string scope' => ['payment-profile/read', 'single string scope'],
'null scopes' => [null, 'null scopes'],
'empty string scopes' => ['', 'empty string scopes'],
];
}
}
Loading