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.

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

Update phpcr-shell to Symfony 7 #224

Closed
19 changes: 19 additions & 0 deletions .github/workflows/test-application.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,21 @@ jobs:
dependencies: highest
behat-suite: cli

- php-version: '8.3'
dependencies: highest
behat-suite: standalone
composer-stability: 'dev'

- php-version: '8.3'
dependencies: highest
behat-suite: embedded
composer-stability: 'dev'

- php-version: '8.3'
dependencies: highest
behat-suite: cli
composer-stability: 'dev'

steps:
- name: Checkout project
uses: actions/checkout@v2
Expand All @@ -74,6 +89,10 @@ jobs:
php-version: ${{ matrix.php-version }}
tools: 'composer:v2'

- name: Set composer stability
if: ${{ matrix.composer-stability }}
run: composer config minimum-stability ${{ matrix.composer-stability }}

- name: Install dependencies with Composer
uses: ramsey/composer-install@v2
with:
Expand Down
14 changes: 7 additions & 7 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
"description": "Shell for PHPCR",
"require": {
"php": "^8.0",
"symfony/console": "^5.0 || ^6.0",
"symfony/console": "^5.0 || ^6.0 || ^7.0",
"jackalope/jackalope": "^1.3.4",
"phpcr/phpcr": "^2.1",
"phpcr/phpcr-utils": "^1.2",
"symfony/finder": "^5.0 || ^6.0",
"symfony/serializer": "^5.0 || ^6.0",
"symfony/yaml": "^5.0 || ^6.0",
"symfony/dependency-injection": "^5.0 || ^6.0",
"symfony/expression-language": "^5.0 || ^6.0",
"phpcr/phpcr-utils": "^1.2 || ^2.0",
Copy link
Member

@dbu dbu Nov 30, 2023

Choose a reason for hiding this comment

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

can we try the dev version for now, to avoid a broken release of phpcr-utils?

Suggested change
"phpcr/phpcr-utils": "^1.2 || ^2.0",
"phpcr/phpcr-utils": "^1.2 || ^2.0@dev",

Copy link
Contributor Author

@alexander-schranz alexander-schranz Nov 30, 2023

Choose a reason for hiding this comment

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

Did apply this in the CI to test against @dev dependencies. We still are blocked by phpspec and behat here to test fully against Symfony 7:

"symfony/finder": "^5.0 || ^6.0 || ^7.0",
"symfony/serializer": "^5.0 || ^6.0 || ^7.0",
"symfony/yaml": "^5.0 || ^6.0 || ^7.0",
"symfony/dependency-injection": "^5.0 || ^6.0 || ^7.0",
"symfony/expression-language": "^5.0 || ^6.0 || ^7.0",
"dantleech/glob-finder": "^1.0"
},
"require-dev": {
Expand Down
17 changes: 12 additions & 5 deletions src/PHPCR/Shell/Serializer/NodeNormalizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,21 @@ public function getNotes()
return $this->notes;
}

public function getSupportedTypes(?string $format): array
{
return [
NodeInterface::class => true,
];
}

/**
* {@inheritdoc}
*/
public function normalize($node, $format = null, array $context = [])
public function normalize($object, $format = null, array $context = []): array
{
$res = [];

foreach ($node->getProperties() as $property) {
foreach ($object->getProperties() as $property) {
if (false === $this->isPropertyEditable($property)) {
continue;
}
Expand Down Expand Up @@ -85,15 +92,15 @@ public function normalize($node, $format = null, array $context = [])
/**
* {@inheritdoc}
*/
public function supportsNormalization($data, $format = null)
public function supportsNormalization($data, $format = null, array $context = []): bool
Copy link
Contributor Author

Choose a reason for hiding this comment

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

technical a bc break but I think we could ignore it?

Copy link
Member

Choose a reason for hiding this comment

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

this is to adjust to symfony 7 BC breaks, right?

it would only be an issue if somebody extended the NodeNormalizer class. for this component imo that is not expected, agree that we don't do a new major version for that change.

{
return is_object($data) && $data instanceof NodeInterface;
}

/**
* {@inheritdoc}
*/
public function denormalize($data, $class, $format = null, array $context = [])
public function denormalize($data, $class, $format = null, array $context = []): 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.

technical a bc break but I think we could ignore it?

{
if (!$data) {
throw new \InvalidArgumentException(
Expand Down Expand Up @@ -166,7 +173,7 @@ public function denormalize($data, $class, $format = null, array $context = [])
/**
* {@inheritdoc}
*/
public function supportsDenormalization($data, $type, $format = null)
public function supportsDenormalization($data, $type, $format = null, array $context = []): bool
Copy link
Contributor Author

Choose a reason for hiding this comment

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

technical a bc break but I think we could ignore it?

{
return $type === 'PHPCR\NodeInterface';
}
Expand Down
2 changes: 1 addition & 1 deletion src/PHPCR/Shell/Serializer/YamlEncoder.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function encode($data, $format, array $context = []): string
return Yaml::dump($data);
}

public function decode($data, $format, array $context = [])
public function decode($data, $format, array $context = []): mixed
{
$arr = Yaml::parse($data);

Expand Down
Loading