Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix build failures for the cake 5 upgrade #326

Merged
merged 16 commits into from
Oct 3, 2023
Merged

Fix build failures for the cake 5 upgrade #326

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e021b4e
cake 5 upgrade - upgrade to PhpUnit 10, which is needed when working …
Sep 16, 2023
753a8b4
cake 5 upgrade - The request handler component has been removed from …
Sep 18, 2023
5989f4c
cake 5 upgrade - Fix style issues reported by phpcs
Sep 18, 2023
d861f93
cake 5 upgrade - Fix Coding Standard & Static Analysis Errors / Warnings
Sep 17, 2023
ec7fb17
cake 5 upgrade - Static code analysis A warns that $fileInfo->getPath…
Sep 17, 2023
439ec18
cake 5 upgrade - readd psalm and phpstan to fix the build
Sep 18, 2023
99eb126
cake 5 upgrade - psalm 4 does not have support for intersecion types,…
Sep 18, 2023
945a72f
cake 5 upgrade - psalm 5.0 throws a "Could not get class storage for …
Sep 18, 2023
57eb38e
cake 5 upgrade - phpunit 10 remoced the --verbose option
Sep 18, 2023
5af9bfe
cake 5 upgrade - no need for dev versions anymore, as cake 5 is decla…
Sep 18, 2023
ac96c8a
cake 5 upgrade - update to mpdf 8.1.6, because it's the first minor r…
Sep 18, 2023
fe876b6
cake 5 upgrade - implement pr feedback
Sep 18, 2023
21c3938
cake 5 upgrade - upgrade to cakephp 5.0.1 and remove the temporary wo…
Oct 1, 2023
cf112d2
cake 5 upgrade - adapt the PdfView for content type negotiation
Oct 1, 2023
00e40ef
Fix CS error
ADmad Oct 3, 2023
6f83dce
Fix CS error
ADmad Oct 3, 2023
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
8 changes: 4 additions & 4 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
- name: Run PHPUnit
run: |
if [[ ${{ matrix.php-version }} == '8.1' ]]; then
vendor/bin/phpunit --verbose --coverage-clover=coverage.xml
vendor/bin/phpunit --coverage-clover=coverage.xml
else
vendor/bin/phpunit
fi
Expand All @@ -74,7 +74,7 @@ jobs:
php-version: '8.1'
extensions: mbstring, intl
coverage: none
tools: cs2pr
tools: cs2pr, vimeo/psalm:5.15, phpstan:1.8

- name: Composer Install
run: composer install
Expand All @@ -84,8 +84,8 @@ jobs:

- name: Run psalm
if: success() || failure()
run: vendor/bin/psalm --output-format=github
run: psalm --output-format=github
BGehrels marked this conversation as resolved.
Show resolved Hide resolved

- name: Run phpstan
if: success() || failure()
run: vendor/bin/phpstan analyse
run: phpstan analyse
6 changes: 3 additions & 3 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
"cakephp/cakephp": "^5.0"
BGehrels marked this conversation as resolved.
Show resolved Hide resolved
},
"require-dev": {
"phpunit/phpunit": "^9.5",
"phpunit/phpunit": "^10.3",
"dompdf/dompdf": "^2.0",
"mpdf/mpdf": "^8.0.4",
"mpdf/mpdf": "^8.1.6",
"tecnickcom/tcpdf": "^6.3",
"cakephp/cakephp-codesniffer": "^5.0"
},
Expand All @@ -33,7 +33,7 @@
"TestApp\\": "tests/test_app/src/"
}
},
"minimum-stability": "dev",
"minimum-stability": "stable",
"scripts": {
"check": [
"@test",
Expand Down
16 changes: 0 additions & 16 deletions config/bootstrap.php
View file
Open in desktop

This file was deleted.

38 changes: 12 additions & 26 deletions phpunit.xml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,13 @@
<phpunit
backupGlobals="false"
backupStaticAttributes="false"
bootstrap="./tests/bootstrap.php"
colors="true"
convertErrorsToExceptions="true"
convertNoticesToExceptions="true"
convertWarningsToExceptions="true"
processIsolation="false"
stopOnError="false"
stopOnFailure="false"
stopOnIncomplete="false"
stopOnSkipped="false"
>

<testsuites>
<testsuite name="CakePdf Plugin Tests">
<directory>./tests/TestCase</directory>
</testsuite>
</testsuites>

<filter>
<whitelist>
<directory suffix=".php">./src/</directory>
</whitelist>
</filter>
<?xml version="1.0"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" backupGlobals="false" bootstrap="./tests/bootstrap.php" colors="true" processIsolation="false" stopOnError="false" stopOnFailure="false" stopOnIncomplete="false" stopOnSkipped="false" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.3/phpunit.xsd" cacheDirectory=".phpunit.cache" backupStaticProperties="false">
<testsuites>
<testsuite name="CakePdf Plugin Tests">
<directory>./tests/TestCase</directory>
</testsuite>
</testsuites>
<source>
<include>
<directory suffix=".php">./src/</directory>
</include>
</source>
BGehrels marked this conversation as resolved.
Show resolved Hide resolved
</phpunit>
9 changes: 8 additions & 1 deletion src/Plugin.php → src/CakePdfPlugin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,15 @@

use Cake\Core\BasePlugin;

class Plugin extends BasePlugin
class CakePdfPlugin extends BasePlugin
{
/**
* Do bootstrap or not
*
* @var bool
*/
protected bool $bootstrapEnabled = false;

/**
* Load routes or not
*
Expand Down
12 changes: 9 additions & 3 deletions src/Pdf/CakePdf.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ public function html(?string $html = null): mixed
/**
* Writes output to file
*
* @throws \Cake\Core\Exception\CakeException
* @param string $destination Absolute file path to write to
* @param bool $create Create file if it does not exist (if true)
* @param string|null $html Html to write
Expand All @@ -347,7 +348,12 @@ public function write(string $destination, bool $create = true, ?string $html =
return (bool)file_put_contents($destination, $output);
}

if (!$fileInfo->getPathInfo()->getRealPath()) {
$splFileInfo = $fileInfo->getPathInfo();
/** @phpstan-ignore-next-line */
if ($splFileInfo === null) {
throw new CakeException('Failed to retrieve path information');
}
if (!$splFileInfo->getRealPath()) {
mkdir($fileInfo->getPath(), 0777, true);
}

Expand All @@ -357,11 +363,11 @@ public function write(string $destination, bool $create = true, ?string $html =
/**
* Load PdfEngine
*
* @param array|string $name Classname of pdf engine without `Engine` suffix. For example `CakePdf.DomPdf`
* @param array|string|null $name Classname of pdf engine without `Engine` suffix. For example `CakePdf.DomPdf`
* @throws \Cake\Core\Exception\CakeException
* @return \CakePdf\Pdf\Engine\AbstractPdfEngine|null
*/
public function engine(string|array|null $name = null): ?AbstractPdfEngine
public function engine(array|string|null $name = null): ?AbstractPdfEngine
{
if ($name === null) {
return $this->_engineClass;
Expand Down
16 changes: 12 additions & 4 deletions src/View/PdfView.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ class PdfView extends View
/**
* Constructor
*
* @param \Cake\Http\ServerRequest $request Request instance.
* @param \Cake\Http\Response $response Response instance.
* @param \Cake\Event\EventManager $eventManager Event manager instance.
* @param \Cake\Http\ServerRequest|null $request Request instance.
* @param \Cake\Http\Response|null $response Response instance.
* @param \Cake\Event\EventManager|null $eventManager Event manager instance.
* @param array $viewOptions View options. See View::$_passedVars for list of
* options which get set as class properties.
* @throws \Cake\Core\Exception\CakeException
Expand Down Expand Up @@ -104,7 +104,7 @@ public function renderer(?array $config = null): ?CakePdf
* @param string|false|null $layout The layout being rendered.
* @return string The rendered view.
*/
public function render(?string $template = null, false|string|null $layout = null): string
public function render(?string $template = null, string|false|null $layout = null): string
{
$content = parent::render($template, $layout);

Expand All @@ -130,6 +130,14 @@ public function render(?string $template = null, false|string|null $layout = nul
return $this->Blocks->get('content');
}

/**
* @inheritDoc
*/
public function getConfig(?string $key = null, mixed $default = null): mixed
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cake 5 makes the base method protected, but it's used in the unit tests. I opened it again, but i'm also open to other solutions.

Copy link
Member

Choose a reason for hiding this comment

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

The protected visibility is a mistake in the core. It will be fixed in 5.0.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess we'll wait for te release of 5.0.1 here, then.

Copy link
Member

Choose a reason for hiding this comment

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

This class also needs

/**
 * Mime-type this view class renders as.
 *
 * @return string Either the content type or '' which means no type.
 */
public static function contentType(): string {
	return '...;
}

It seems.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With this fragment, the testRenderErrorTemplate fails and i'm wondering, if that test actually makes sense. I think i do not know enough about Cakes Error handling to judge that.

Copy link
Member

Choose a reason for hiding this comment

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

I'll look into it.

{
return parent::getConfig($key, $default);
}

/**
* Get or build a filename for forced download
*
Expand Down
23 changes: 14 additions & 9 deletions tests/TestCase/Pdf/Engine/DomPdfEngineTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public function setUp(): void
*/
public function testReceiveOptions()
{
$engineClass = $this->getMockClass(DomPdfEngine::class, ['_createInstance']);
$engineClass = $this->getMockBuilder(DomPdfEngine::class)->disableOriginalConstructor()->onlyMethods(['_createInstance'])->getMock()::class;

$Pdf = new CakePdf([
'engine' => [
Expand Down Expand Up @@ -65,8 +65,7 @@ public function testReceiveOptions()
*/
public function testSetOptions()
{
$engineClass = $this->getMockClass(DomPdfEngine::class, ['_output']);

$engineClass = $this->getMockBuilder(DomPdfEngine::class)->disableOriginalConstructor()->onlyMethods(['_output'])->getMock()::class;
$Pdf = new CakePdf([
'engine' => [
'className' => '\\' . $engineClass,
Expand Down Expand Up @@ -114,11 +113,14 @@ public function testOutput()
*/
public function testControlFlow()
{
$engineClass = $this->getMockClass(DomPdfEngine::class, [
'_createInstance',
'_render',
'_output',
]);
$engineClass = $this->getMockBuilder(DomPdfEngine::class)
->disableOriginalConstructor()
->onlyMethods([
'_createInstance',
'_render',
'_output',
])
->getMock()::class;

$Pdf = new CakePdf([
'engine' => '\\' . $engineClass,
Expand Down Expand Up @@ -149,7 +151,10 @@ public function testControlFlow()
*/
public function testDompdfControlFlow()
{
$engineClass = $this->getMockClass(DomPdfEngine::class, ['_createInstance']);
$engineClass = $this->getMockBuilder(DomPdfEngine::class)
->disableOriginalConstructor()
->onlyMethods(['_createInstance'])
->getMock()::class;

$Pdf = new CakePdf([
'engine' => '\\' . $engineClass,
Expand Down
5 changes: 4 additions & 1 deletion tests/TestCase/Pdf/Engine/MpdfEngineTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ public function setUp(): void
*/
public function testSetOptions()
{
$engineClass = $this->getMockClass(MpdfEngine::class, ['_createInstance']);
$engineClass = $this->getMockBuilder(MpdfEngine::class)
->disableOriginalConstructor()
->onlyMethods(['_createInstance'])
->getMock()::class;

$Pdf = new CakePdf([
'engine' => [
Expand Down