[BUGFIX] Improve PlantumlRenderer robustness and temp file cleanup

[CybotTM]

Summary

Follow-up to #1279 addressing code review findings for improved robustness.

Changes

1. Race-safe directory creation (lines 57, 112-130)

• Extracted ensureDirectoryExists() helper method with clear documentation
• Handles race conditions where concurrent processes may create the directory
• Step-by-step logic with comments explaining each check

2. Check tempnam() return value (lines 66-74)

• tempnam() can return false on failure
• Now properly checks and logs error before continuing

3. Clean up temporary files (lines 106-107)

• Deletes .pu source file and .svg output after reading
• Prevents unbounded disk usage growth during documentation builds

4. Safe test cleanup (test lines 47-53)

• Multiple guards to prevent accidental deletion of wrong directories
• Validates temp directory path before cleanup

Files Changed

• packages/guides-graphs/src/Graphs/Renderer/PlantumlRenderer.php
• packages/guides-graphs/tests/unit/Renderer/PlantumlRendererTest.php

Test Plan

• PHPUnit test passes
• PHPStan passes
• PHPCS passes

[CybotTM]

sorry for the noise, should be fine now.

[jaapio]

I think it's better to split the conditions here. And not to perform operations in an condition like mkdir those are very hard to detect, people do not expect them in there.
Also the !is_dir($this->tempDirectory) is performed twice, that seems to be invalid.

[CybotTM]

That's a simple trinary operation, not so uncommon/unknown, and the duplicated is_dir() is intentionally, that's the whole point of it.

if (!is_dir($this->tempDirectory)) {

    // 2. Attempt Creation: Try to create it with 0755 permissions.
    // The '@' suppresses a PHP warning if it fails.
    // 'true' allows recursive creation of parent folders.
    $creationSuccessful = @mkdir($this->tempDirectory, 0755, true);

    if (!$creationSuccessful) {
        
        // 3. Final Verification (Race Condition Check):
        // If mkdir failed, it might be because another process 
        // created it between step 1 and step 2.
        if (!is_dir($this->tempDirectory)) {
            
            // IF IT STILL DOESN'T EXIST: Handle the real failure.
            // This is where you would place your error logic or throw an exception.
            throw new \Exception("Failed to create directory: " . $this->tempDirectory);
        }
    }
}

But this is already explained in the PR description.

[jaapio]

I get it's common to do this, but I do see it as a bad practice to combine things like this. It makes the code hard to understand.

[jaapio]

I get it's common to do this, but I do see it as a bad practice to combine things like this. It makes the code hard to understand.

[CybotTM]

I already changed it. You may notice.