Skip to content
Draft
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
35 changes: 18 additions & 17 deletions docs/coding-guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@ See `src/Doctrine/DBALDatabaseConnectionChecker.php` for an example.

> **Note**: Do NOT use PHP 8.2 `readonly class` syntax - we're on PHP 8.1.

## Preferences

When working on this codebase, prefer:

- Extracting logic into testable classes with interfaces over inline implementation
- Constructor injection over service locator patterns
- Explicit code over clever abstractions
- Integration with existing patterns over introducing new ones
- Inline expressions over unnecessary local variables (if a variable is only used once and doesn't improve readability, inline it)

## Comments & Self-Documenting Code

Avoid comments in favor of self-documenting code:
Expand All @@ -38,6 +48,14 @@ Comments are acceptable when explaining **why** something is done (not **what**)
- Descriptive name without `Exception` suffix
- Name should describe what went wrong (e.g., `DocumentDoesNotExist`, `NewsArticleNotFound`)

## Dependency Injection

- Service providers in `app/` wire all dependencies
- Use **constructor injection**, not service locators
- Extract testable logic into separate classes with **interfaces**

See `src/Doctrine/DatabaseConnectionChecker.php` and `src/Doctrine/DBALDatabaseConnectionChecker.php` for an example of interface + implementation pattern.

## Testing

- Use `@test` annotation style for test methods
Expand Down Expand Up @@ -67,29 +85,12 @@ Comments are acceptable when explaining **why** something is done (not **what**)
- **Never** create commits - only humans commit code
- **Plan** larger changes but implement step by step, waiting for human review between steps

## Dependency Injection

- Service providers in `app/` wire all dependencies
- Use **constructor injection**, not service locators
- Extract testable logic into separate classes with **interfaces**

See `src/Doctrine/DatabaseConnectionChecker.php` and `src/Doctrine/DBALDatabaseConnectionChecker.php` for an example of interface + implementation pattern.

## Workflow

- **Small commits**: One logical change per commit
- **Small pull requests**: Easier to review, faster to merge
- **Feature flags**: Deploy often, enable features when ready

## Preferences

When working on this codebase, prefer:

- Extracting logic into testable classes with interfaces over inline implementation
- Constructor injection over service locator patterns
- Explicit code over clever abstractions
- Integration with existing patterns over introducing new ones

## Security

### Never Commit
Expand Down
2 changes: 1 addition & 1 deletion docs/features/search.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ SAPI3 supports two types of query parameters:

Search tests can use scenario-based label isolation to prevent interference from other tests:

- Tag scenarios with `@labelIsolation` to enable isolation
- Tag scenarios with `@testIsolation` to enable isolation
- A unique label (`scenario-{uuid}`) is automatically generated per scenario
- The label is added to all fixtures created during the scenario
- Search queries automatically filter by this label
Expand Down
17 changes: 17 additions & 0 deletions features/State/VariableState.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ final class VariableState
private const START_DELIMITER = '%{';
private const END_DELIMITER = '}';

private static ?string $scenarioLabel = null;

private array $variables = [];

public function setVariable(string $key, string $value): void
Expand Down Expand Up @@ -38,6 +40,21 @@ public function getVariable(string $key): string
return $this->variables[$key];
}

public static function setScenarioLabel(string $label): void
{
self::$scenarioLabel = $label;
}

public static function getScenarioLabel(): ?string
{
return self::$scenarioLabel;
}

public static function clearScenarioLabel(): void
{
self::$scenarioLabel = null;
}

public function replaceVariables(string $key): string
{
if ($this->isVariable($key)) {
Expand Down
2 changes: 2 additions & 0 deletions features/Steps/EventSteps.php
Original file line number Diff line number Diff line change
Expand Up @@ -204,5 +204,7 @@ private function createEvent(string $endpoint, string $json, string $jsonPath, s
$this->theResponseStatusShouldBe(201);
$this->theResponseBodyShouldBeValidJson();
$this->iKeepTheValueOfTheJsonResponseAtAs($jsonPath, $variableName);

$this->addScenarioLabelToResource('event');
}
}
2 changes: 2 additions & 0 deletions features/Steps/OrganizerSteps.php
Original file line number Diff line number Diff line change
Expand Up @@ -129,5 +129,7 @@ private function createOrganizer(string $endpoint, string $json, string $jsonPat
$this->theResponseStatusShouldBe(str_contains($endpoint, 'imports') ? 200 : 201);
$this->theResponseBodyShouldBeValidJson();
$this->iKeepTheValueOfTheJsonResponseAtAs($jsonPath, $variableName);

$this->addScenarioLabelToResource('organizer');
}
}
2 changes: 2 additions & 0 deletions features/Steps/PlaceSteps.php
Original file line number Diff line number Diff line change
Expand Up @@ -286,5 +286,7 @@ private function createPlace(string $endpoint, string $json, string $jsonPath, s
$this->theResponseStatusShouldBe($responseStatus);
$this->theResponseBodyShouldBeValidJson();
$this->iKeepTheValueOfTheJsonResponseAtAs($jsonPath, $variableName);

$this->addScenarioLabelToResource('place');
}
}
43 changes: 42 additions & 1 deletion features/Steps/RequestSteps.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Behat\Gherkin\Node\PyStringNode;
use Behat\Gherkin\Node\TableNode;
use CultuurNet\UDB3\State\VariableState;

trait RequestSteps
{
Expand Down Expand Up @@ -101,10 +102,50 @@ public function iSendAGetRequestTo(string $url): void
*/
public function iSendAGetRequestToWithParameters(string $url, TableNode $parameters): void
{
$response = $this->getHttpClient()->getWithParameters($url, $parameters->getRows(), $this->variableState);
$params = $this->addScenarioLabelToSearchParameters($url, $parameters->getRows());
$response = $this->getHttpClient()->getWithParameters($url, $params, $this->variableState);
$this->responseState->setResponse($response);
}

private function addScenarioLabelToSearchParameters(string $url, array $parameters): array
{
$scenarioLabel = VariableState::getScenarioLabel();
if ($scenarioLabel === null) {
return $parameters;
}

$searchEndpoints = ['/events', '/places', '/organizers', '/offers'];
$isSearchEndpoint = false;
foreach ($searchEndpoints as $endpoint) {
if (str_starts_with($url, $endpoint)) {
$isSearchEndpoint = true;
break;
}
}

if (!$isSearchEndpoint) {
return $parameters;
}

$labelFilter = 'labels:' . $scenarioLabel;

$qIndex = null;
foreach ($parameters as $index => $param) {
if ($param[0] === 'q') {
$qIndex = $index;
break;
}
}

if ($qIndex !== null) {
$parameters[$qIndex][1] = '(' . $parameters[$qIndex][1] . ') AND ' . $labelFilter;
} else {
$parameters[] = ['q', $labelFilter];
}

return $parameters;
}

/**
* @When I send a PATCH request to :url
*/
Expand Down
13 changes: 13 additions & 0 deletions features/Steps/UtilitySteps.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

namespace CultuurNet\UDB3\Steps;

use CultuurNet\UDB3\State\VariableState;

use function PHPUnit\Framework\assertEquals;

trait UtilitySteps
Expand Down Expand Up @@ -96,4 +98,15 @@ private function countFilesByType(string $type, string $folder): int
$downloadsFolder = $this->config['folders'][$folder];
return count(glob($downloadsFolder . '/*.' . $type));
}

private function addScenarioLabelToResource(string $resourceType): void
{
$scenarioLabel = VariableState::getScenarioLabel();
if ($scenarioLabel === null) {
return;
}

$resourceId = $this->responseState->getValueOnPath($resourceType . 'Id');
$this->getHttpClient()->putJSON('/' . $resourceType . 's/' . $resourceId . '/labels/' . $scenarioLabel, '');
}
}
18 changes: 18 additions & 0 deletions features/bootstrap/FeatureContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
declare(strict_types=1);

use Behat\Behat\Context\Context;
use Behat\Behat\Hook\Scope\AfterScenarioScope;
use Behat\Behat\Hook\Scope\BeforeScenarioScope;
use Behat\Testwork\Hook\Scope\BeforeSuiteScope;
use CultuurNet\UDB3\Model\ValueObject\Identity\Uuid;
use CultuurNet\UDB3\State\RequestState;
use CultuurNet\UDB3\State\ResponseState;
use CultuurNet\UDB3\State\VariableState;
Expand Down Expand Up @@ -94,6 +96,22 @@ public function beforeScenarioMails(BeforeScenarioScope $scope): void
$this->getMailClient()->deleteAllMails();
}

/**
* @BeforeScenario @testIsolation
*/
public function beforeScenarioTestIsolation(BeforeScenarioScope $scope): void
{
VariableState::setScenarioLabel('scenario-' . Uuid::uuid4()->toString());
}

/**
* @AfterScenario @testIsolation
*/
public function afterScenarioTestIsolation(AfterScenarioScope $scope): void
{
VariableState::clearScenarioLabel();
}

/**
* @Transform :url
*/
Expand Down