Skip to content
Merged
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
6 changes: 4 additions & 2 deletions lib/Controller/AangebodenGebruikController.php
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,9 @@ public function getGebruiksWhereAfnemer(): JSONResponse
$result = $this->gebruikSvc->getGebruiksWhereAfnemer($options);

// Determine HTTP status code based on whether there's an error.
$statusCode = 200;
$statusCode = 200;
if (isset($result['error']) === true) {
$statusCode = 500;
}

$this->logger->info(
Expand Down Expand Up @@ -231,8 +232,9 @@ public function getKoppelingenGebruikByUuid(string $uuid): JSONResponse
);

// Determine HTTP status code based on whether there's an error.
$statusCode = 200;
$statusCode = 200;
if (isset($result['error']) === true) {
$statusCode = 500;
}

$this->logger->info(
Expand Down
3 changes: 2 additions & 1 deletion lib/Service/AanbodService.php
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,9 @@ public function getAanbod(array $options=[]): array
foreach ($searchResult['results'] ?? [] as $result) {
// Use jsonSerialize() instead of getObject() to include @self metadata.
// GetObject() only returns raw object data without @self.organisation.
$resultData = $result;
if (is_array($result) === false) {
$resultData = $result->jsonSerialize();
if (is_array($result) === true) {
}

$selfOrg = $resultData['@self']['organisation'] ?? null;
Expand Down
15 changes: 10 additions & 5 deletions lib/Service/AangebodenGebruikService.php
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,9 @@ public function getGebruiksWhereAfnemer(array $options=[]): array
$filteredResults = [];
foreach ($searchResult['results'] ?? [] as $result) {
// Convert ObjectEntity to array if needed.
$resultData = $result;
if (is_array(value: $result) === false) {
$resultData = $result->getObject();
if (is_array(value: $result) === true) {
}

$selfOrg = $resultData['@self']['organisation'] ?? null;
Expand Down Expand Up @@ -405,8 +406,9 @@ public function getKoppelingenGebruikByUuid(string $uuid, array $options=[], boo
}

// Get organization filter if provided (for ambtenaar).
$organisationFilter = null;
$organisationFilter = null;
if ($isAmbtenaar === true && isset($options['organisation']) === true) {
$organisationFilter = $options['organisation'];
}

// Build search query using ObjectService's buildSearchQuery.
Expand Down Expand Up @@ -823,8 +825,9 @@ public function getGebruiksWhereDeelnemers(array $options=[]): array

// Process and add to results.
foreach ($gebruikItems as $gebruik) {
$gebruikData = $gebruik;
if (is_array(value: $gebruik) === false) {
$gebruikData = $gebruik->jsonSerialize();
if (is_array(value: $gebruik) === true) {
}

$gebruikData['_filter_type'] = 'deelnemers';
Expand Down Expand Up @@ -1345,8 +1348,9 @@ private function getApplicationsOwnedByOrganisation(
);

foreach ($suites as $suite) {
$suiteData = $suite;
if (is_array(value: $suite) === false) {
$suiteData = $suite->getObject();
if (is_array(value: $suite) === true) {
}

$appUuids[] = $suiteData['uuid'] ?? $suiteData['id'] ?? null;
Expand All @@ -1372,8 +1376,9 @@ private function getApplicationsOwnedByOrganisation(
);

foreach ($modules as $module) {
$moduleData = $module;
if (is_array(value: $module) === false) {
$moduleData = $module->getObject();
if (is_array(value: $module) === true) {
}

$appUuids[] = $moduleData['uuid'] ?? $moduleData['id'] ?? null;
Expand Down
3 changes: 2 additions & 1 deletion lib/Service/ModuleComplianceService.php
Original file line number Diff line number Diff line change
Expand Up @@ -406,8 +406,9 @@ private function extractStandaardversieUuids(array $complianceObjects): array
&& (is_object($standaardversie) === false || isset($standaardversie->uuid) === false)
) {
$tracking['invalidType']++;
$standaardversieValue = (string) $standaardversie;
$standaardversieValue = (string) $standaardversie;
if (is_array($standaardversie) === true) {
$standaardversieValue = json_encode($standaardversie);
}

$this->logger->warning(
Expand Down
15 changes: 10 additions & 5 deletions lib/Service/SettingsService.php
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,9 @@ public function updateSettings(array $data): array
$stringValue = json_encode($value);
} else {
// Ensure value is converted to string as required by setValueString.
$stringValue = (string) $value;
$stringValue = (string) $value;
if (is_string($value) === true) {
$stringValue = $value;
}
}

Expand Down Expand Up @@ -4179,12 +4180,14 @@ private function configureVoorzieningen(): array
$originalSlug = $schema['slug'] ?? '';
$lowercaseSlug = strtolower($originalSlug);

$hasMappingOriginalValue = 'NO';
$hasMappingOriginalValue = 'NO';
if (isset($slugToKey[$originalSlug]) === true) {
$hasMappingOriginalValue = 'YES';
}

$hasMappingLowercaseValue = 'NO';
$hasMappingLowercaseValue = 'NO';
if (isset($slugToKey[$lowercaseSlug]) === true) {
$hasMappingLowercaseValue = 'YES';
}

$this->logger->info(
Expand Down Expand Up @@ -4802,12 +4805,14 @@ public function getArchiMateStatus(): array
// Get AMEF object counts.
$amefObjectCounts = $this->getAmefObjectCounts();

$importValue = [];
$importValue = [];
if (is_array($importDecoded) === true) {
$importValue = $importDecoded;
}

$exportValue = [];
$exportValue = [];
if (is_array($exportDecoded) === true) {
$exportValue = $exportDecoded;
}

return [
Expand Down
6 changes: 4 additions & 2 deletions lib/Service/SymfonyEmailService.php
Original file line number Diff line number Diff line change
Expand Up @@ -688,8 +688,9 @@ public function sendUserCreationEmail(array $user, array $organization=[]): bool
);

// Prepare template data.
$displayName = 'Gebruiker';
$displayName = 'Gebruiker';
if (empty($userName) === false) {
$displayName = $userName;
}

$templateData = [
Expand Down Expand Up @@ -784,8 +785,9 @@ public function sendUserUpdateEmail(array $user, array $organization=[]): bool
);

// Prepare template data.
$displayName = 'Gebruiker';
$displayName = 'Gebruiker';
if (empty($userName) === false) {
$displayName = $userName;
}

$templateData = [
Expand Down
225 changes: 225 additions & 0 deletions tests/Unit/Controller/AangebodenGebruikControllerStatusCodeTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
<?php

/**
* Regression tests for the HTTP status code AangebodenGebruikController
* returns when the service layer reports an error.
*
* @category Test
* @package OCA\SoftwareCatalog\Tests\Unit\Controller
* @author Conduction b.v. <info@conduction.nl>
* @copyright 2026 Conduction B.V.
* @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12
* @link https://codeberg.org/Conduction/SoftwareCatalog
*
* @spec openspec/specs/vendor-visibility-rbac/spec.md#requirement-the-offered-usage-afnemer-endpoint-must-require-authentication-explicitly-not-implicitly-req-004
*
* SPDX-FileCopyrightText: 2026 Conduction B.V. <info@conduction.nl>
* SPDX-License-Identifier: EUPL-1.2
*/

declare(strict_types=1);

namespace OCA\SoftwareCatalog\Tests\Unit\Controller;

use OCA\SoftwareCatalog\Controller\AangebodenGebruikController;
use OCA\SoftwareCatalog\Service\AangebodenGebruikService;
use OCP\IGroupManager;
use OCP\IRequest;
use OCP\IUser;
use OCP\IUserSession;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use Psr\Log\LoggerInterface;

/**
* THE DEFECT UNDER TEST.
*
* Both endpoints below documented their intent in a comment — "Determine
* HTTP status code based on whether there's an error" — and then shipped
*
* $statusCode = 200;
* if (isset($result['error']) === true) {
* }
*
* an EMPTY if body. Commit 651a055f ("refactor: Replace else clauses with
* early returns") rewrote `if (err) { $s = 500; } else { $s = 200; }` by
* hoisting the else-body out and deleting the if-body along with the
* `else` keyword. The condition survived; the only statement it guarded
* did not.
*
* The consequence is not cosmetic: a service-level failure was returned to
* the caller as **HTTP 200** with an `error` key in the body. Every client
* that branches on `response.ok` — which is what this app's own Pinia
* stores do — read a failed request as a successful one with zero results.
* A "no results" screen and a "the backend blew up" screen became
* indistinguishable over the wire.
*
* These tests assert the STATUS CODE, not the envelope, because the
* envelope was always right and is exactly what made the defect invisible.
*/
final class AangebodenGebruikControllerStatusCodeTest extends TestCase
{

/**
* The service double the controller under test delegates to.
*
* @var AangebodenGebruikService|MockObject
*/
private AangebodenGebruikService|MockObject $gebruikSvc;

/**
* The session double, always populated with an authenticated user so
* the controller's own auth guard is not what these tests measure.
*
* @var IUserSession|MockObject
*/
private IUserSession|MockObject $userSession;


/**
* Build the controller with an authenticated caller in session.
*
* @return AangebodenGebruikController The controller under test.
*/
private function makeController(): AangebodenGebruikController
{
$request = $this->createMock(IRequest::class);
$request->method('getParams')->willReturn([]);
$request->method('getParam')->willReturn(null);

$this->gebruikSvc = $this->createMock(AangebodenGebruikService::class);
$this->userSession = $this->createMock(IUserSession::class);

$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('caller-uid');
$this->userSession->method('getUser')->willReturn($user);

return new AangebodenGebruikController(
'softwarecatalog',
$request,
$this->userSession,
$this->gebruikSvc,
$this->createMock(LoggerInterface::class),
$this->createMock(IGroupManager::class)
);

}//end makeController()


/**
* The error envelope produced by the service layer on failure.
*
* @param string|null $error The error message, or null for the success shape.
*
* @return array The service return value.
*/
private function envelope(?string $error): array
{
$envelope = [
'results' => [],
'total' => 0,
'page' => 1,
'pages' => 0,
'limit' => 20,
'offset' => 0,
];

if ($error !== null) {
$envelope['error'] = $error;
}

return $envelope;

}//end envelope()


/**
* A service-reported error on the afnemer listing MUST surface as 500,
* not as a 200 carrying an `error` key.
*
* @return void
*/
public function testAfnemerListingReturns500WhenTheServiceReportsAnError(): void
{
$controller = $this->makeController();

$this->gebruikSvc->method('getGebruiksWhereAfnemer')
->willReturn($this->envelope('Voorzieningen configuration not found'));

$response = $controller->getGebruiksWhereAfnemer();

$this->assertSame(
500,
$response->getStatus(),
'A service error must be reported as HTTP 500. Returning 200 makes a '
.'backend failure indistinguishable from an empty result set for every '
.'client that branches on response.ok.'
);

}//end testAfnemerListingReturns500WhenTheServiceReportsAnError()


/**
* The success path must stay 200 — the fix must not turn every
* response into a 500. Without this arm the test above would also pass
* against a hardcoded `$statusCode = 500`.
*
* @return void
*/
public function testAfnemerListingReturns200OnSuccess(): void
{
$controller = $this->makeController();

$this->gebruikSvc->method('getGebruiksWhereAfnemer')
->willReturn($this->envelope(null));

$response = $controller->getGebruiksWhereAfnemer();

$this->assertSame(200, $response->getStatus());

}//end testAfnemerListingReturns200OnSuccess()


/**
* Same defect, second site: the koppelingen-by-UUID endpoint.
*
* @return void
*/
public function testKoppelingenByUuidReturns500WhenTheServiceReportsAnError(): void
{
$controller = $this->makeController();

$this->gebruikSvc->method('getKoppelingenGebruikByUuid')
->willReturn($this->envelope('Voorzieningen configuration not found'));

$response = $controller->getKoppelingenGebruikByUuid('some-uuid');

$this->assertSame(
500,
$response->getStatus(),
'A service error must be reported as HTTP 500 on the koppelingen-by-UUID endpoint too.'
);

}//end testKoppelingenByUuidReturns500WhenTheServiceReportsAnError()


/**
* And its success arm.
*
* @return void
*/
public function testKoppelingenByUuidReturns200OnSuccess(): void
{
$controller = $this->makeController();

$this->gebruikSvc->method('getKoppelingenGebruikByUuid')
->willReturn($this->envelope(null));

$response = $controller->getKoppelingenGebruikByUuid('some-uuid');

$this->assertSame(200, $response->getStatus());

}//end testKoppelingenByUuidReturns200OnSuccess()


}//end class
Loading
Loading