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

Update metafiles and code style #36

Merged
merged 4 commits into from
Jan 9, 2025
Merged

Update metafiles and code style #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4e4981c
Update metafiles
roxblnfk Jan 9, 2025
c825782
Apply code style
roxblnfk Jan 9, 2025
a367a4c
Fix risky test
roxblnfk Jan 9, 2025
3e4512d
PHPUnit: add `coverage` and `source` blocks
roxblnfk Jan 9, 2025
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
10 changes: 9 additions & 1 deletion .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,10 @@
/tests export-ignore
* text=auto

/.* export-ignore
/example export-ignore
/tests export-ignore
/*.xml export-ignore
/*.yml export-ignore
/*.lock export-ignore
/*.dist export-ignore
/*.php export-ignore
12 changes: 12 additions & 0 deletions .github/workflows/cs-fix.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
on:
push:
branches:
- '*'

name: Fix Code Style

jobs:
cs-fix:
permissions:
contents: write
uses: spiral/gh-actions/.github/workflows/cs-fix.yml@master
Comment on lines +1 to +12
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Using a floating reference (@master) is indeed risky

The verification confirms that:

  • The repository has no tagged releases
  • The workflow file receives active updates (last modified Dec 11, 2024)
  • Breaking changes have occurred (e.g., removal of dry-run option in September 2024)

To ensure stability, pin the workflow to a specific commit SHA:

-    uses: spiral/gh-actions/.github/workflows/cs-fix.yml@master
+    uses: spiral/gh-actions/.github/workflows/cs-fix.yml@69d50ba35b48703584c28dc0600cc283d96f2303
🔗 Analysis chain

Consider pinning the workflow version.

The workflow is well-configured, but it uses a floating reference (@master) which could lead to unexpected behavior if the upstream workflow changes.

Consider using a specific commit SHA or tag for spiral/gh-actions to ensure reproducible builds:

-    uses: spiral/gh-actions/.github/workflows/cs-fix.yml@master
+    uses: spiral/gh-actions/.github/workflows/[email protected]  # or specific SHA
🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found

Script:

#!/bin/bash
# Check repository and its releases
gh repo view spiral/gh-actions --json url,description,isArchived,defaultBranch
gh release list --repo spiral/gh-actions

# Check commit history of the specific workflow file
gh api -X GET /repos/spiral/gh-actions/commits \
  --jq '.[] | {sha: .sha, date: .commit.committer.date, message: .commit.message}' \
  -F path=.github/workflows/cs-fix.yml \
  -F per_page=5 \
  --repo spiral/gh-actions

Length of output: 2838

Script:

#!/bin/bash
# Check repository info
gh repo view spiral/gh-actions --json url,description,isArchived

# List releases
gh release list --repo spiral/gh-actions

# Check recent commits
gh api \
  repos/spiral/gh-actions/commits \
  --jq '.[] | {sha: .sha, date: .commit.committer.date, message: .commit.message}' \
  -X GET \
  -f path=.github/workflows/cs-fix.yml \
  -f per_page=5

Length of output: 897

13 changes: 4 additions & 9 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
.idea/
.env
/runtime
/vendor
/.idea
/.env
composer.lock
vendor/
*.db
clover.xml
example/vendor/
go.sum
builds/
.phpunit.result.cache
.phpunit.cache/
11 changes: 11 additions & 0 deletions .php-cs-fixer.dist.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

declare(strict_types=1);

require_once 'vendor/autoload.php';

return \Spiral\CodeStyle\Builder::create()
->include(__DIR__ . '/src')
->include(__FILE__)
->allowRisky(true)
->build();
9 changes: 7 additions & 2 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
"jetbrains/phpstorm-attributes": "^1.0",
"mockery/mockery": "^1.4",
"phpunit/phpunit": "^10.0",
"spiral/code-style": "^2.2",
"spiral/dumper": "^3.3",
"vimeo/psalm": ">=5.8"
},
"autoload": {
Expand All @@ -66,8 +68,11 @@
}
},
"scripts": {
"tests": "phpunit",
"psalm": "psalm --no-cache"
"cs:diff": "php-cs-fixer fix --dry-run -v --diff --show-progress dots",
"cs:fix": "php-cs-fixer fix -v",
"psalm": "psalm",
"psalm:baseline": "psalm --set-baseline=psalm-baseline.xml",
"tests": "phpunit"
},
"config": {
"sort-packages": true
Expand Down
36 changes: 27 additions & 9 deletions phpunit.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,37 @@
<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" bootstrap="vendor/autoload.php" backupGlobals="false"
colors="true" processIsolation="false" stopOnFailure="false" stopOnError="false" stderr="true"
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.0/phpunit.xsd" cacheDirectory=".phpunit.cache"
backupStaticProperties="false">
<coverage>
<include>
<directory>src</directory>
</include>
</coverage>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd"
bootstrap="vendor/autoload.php"
cacheResultFile="runtime/phpunit/result.cache"
backupGlobals="false"
colors="true"
processIsolation="false"
stopOnFailure="false"
stopOnError="false"
stderr="true"
backupStaticProperties="false"
>
Comment on lines +2 to +13
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add coverage configuration to fix pipeline warning.

The pipeline is warning about missing coverage filter. This could impact code coverage reporting.

Add the following configuration:

 <phpunit ...>
+    <coverage>
+        <include>
+            <directory>src</directory>
+        </include>
+    </coverage>
     <testsuites>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd"
bootstrap="vendor/autoload.php"
cacheResultFile="runtime/phpunit/result.cache"
backupGlobals="false"
colors="true"
processIsolation="false"
stopOnFailure="false"
stopOnError="false"
stderr="true"
backupStaticProperties="false"
>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd"
bootstrap="vendor/autoload.php"
cacheResultFile="runtime/phpunit/result.cache"
backupGlobals="false"
colors="true"
processIsolation="false"
stopOnFailure="false"
stopOnError="false"
stderr="true"
backupStaticProperties="false"
>
<coverage>
<include>
<directory>src</directory>
</include>
</coverage>
🧰 Tools
🪛 GitHub Actions: phpunit

[warning] No filter is configured, code coverage will not be processed

<testsuites>
<testsuite name="Test Suite">
<directory suffix="Test.php">tests</directory>
<exclude>tests/generated</exclude>
</testsuite>
</testsuites>
<coverage>
<report>
<html outputDirectory="runtime/phpunit/coverage"/>
<text outputFile="runtime/phpunit/coverage.txt"/>
<clover outputFile="runtime/phpunit/logs/clover.xml"/>
</report>
</coverage>
<source>
<include>
<directory>src</directory>
</include>
<exclude>
<directory>tests</directory>
</exclude>
</source>
<php>
<!-- error_reporting = E_ALL ^ E_DEPRECATED -->
<ini name="error_reporting" value="24575"/>
Expand Down
2 changes: 2 additions & 0 deletions psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<?xml version="1.0" encoding="UTF-8"?>
<files psalm-version="dev-master@765dcbfe43002e52e4808b65561842784fe7bcc7"/>
1 change: 1 addition & 0 deletions psalm.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://getpsalm.org/schema/config"
xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
errorBaseline="psalm-baseline.xml"
errorLevel="1"
hoistConstants="true"
resolveFromConfigFile="true"
Expand Down
3 changes: 1 addition & 2 deletions src/Context.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ final class Context implements ContextInterface, \IteratorAggregate, \Countable,
*/
public function __construct(
private array $values,
) {
}
) {}

public function withValue(string $key, mixed $value): ContextInterface
{
Expand Down
2 changes: 1 addition & 1 deletion src/Internal/Json.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ public static function encode(mixed $payload): string
*/
public static function decode(string $payload): array
{
return (array)\json_decode($payload, true, self::DEFAULT_JSON_DEPTH, self::DEFAULT_JSON_FLAGS);
return (array) \json_decode($payload, true, self::DEFAULT_JSON_DEPTH, self::DEFAULT_JSON_FLAGS);
}
}
1 change: 0 additions & 1 deletion src/Invoker.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ final class Invoker implements InvokerInterface
private const ERROR_METHOD_RETURN =
'Method %s must return an object that instance of %s, ' .
'but the result provides type of %s';

private const ERROR_METHOD_IN_TYPE =
'Method %s input type must be an instance of %s, ' .
'but the input is type of %s';
Expand Down
83 changes: 38 additions & 45 deletions src/Method.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,18 @@ final class Method
private const ERROR_PARAMS_COUNT =
'The GRPC method %s can only contain 2 parameters (input and output), but ' .
'signature contains an %d parameters';

private const ERROR_PARAM_UNION_TYPE =
'Parameter $%s of the GRPC method %s cannot be declared using union type';

private const ERROR_PARAM_CONTEXT_TYPE =
'The first parameter $%s of the GRPC method %s can only take an instance of %s';

private const ERROR_PARAM_INPUT_TYPE =
'The second (input) parameter $%s of the GRPC method %s can only take ' .
'an instance of %s, but type %s is indicated';

private const ERROR_RETURN_UNION_TYPE =
'Return type of the GRPC method %s cannot be declared using union type';

private const ERROR_RETURN_TYPE =
'Return type of the GRPC method %s must return ' .
'an instance of %s, but type %s is indicated';

private const ERROR_INVALID_GRPC_METHOD = 'Method %s is not valid GRPC method.';

/**
Expand All @@ -45,7 +39,44 @@ private function __construct(
public readonly string $name,
public readonly string $inputType,
public readonly string $outputType,
) {
) {}

/**
* Returns true if method signature matches.
*/
public static function match(\ReflectionMethod $method): bool
{
try {
self::assertMethodSignature($method);
} catch (\Throwable) {
return false;
}

return true;
}

/**
* Creates a new {@see Method} object from a {@see \ReflectionMethod} object.
*/
public static function parse(\ReflectionMethod $method): Method
{
try {
self::assertMethodSignature($method);
} catch (\Throwable $e) {
$message = \sprintf(self::ERROR_INVALID_GRPC_METHOD, $method->getName());
throw GRPCException::create($message, StatusCode::INTERNAL, $e);
}

[, $input] = $method->getParameters();

/** @var \ReflectionNamedType $inputType */
$inputType = $input->getType();

/** @var \ReflectionNamedType $returnType */
$returnType = $method->getReturnType();

/** @psalm-suppress ArgumentTypeCoercion */
return new self($method->getName(), $inputType->getName(), $returnType->getName());
}

/**
Expand Down Expand Up @@ -75,20 +106,6 @@ public function getOutputType(): string
return $this->outputType;
}

/**
* Returns true if method signature matches.
*/
public static function match(\ReflectionMethod $method): bool
{
try {
self::assertMethodSignature($method);
} catch (\Throwable) {
return false;
}

return true;
}

/**
* @throws \ReflectionException
*/
Expand Down Expand Up @@ -231,28 +248,4 @@ private static function assertMethodSignature(\ReflectionMethod $method): void
// The return type must be declared as a Google\Protobuf\Internal\Message class
self::assertOutputReturnType($method);
}

/**
* Creates a new {@see Method} object from a {@see \ReflectionMethod} object.
*/
public static function parse(\ReflectionMethod $method): Method
{
try {
self::assertMethodSignature($method);
} catch (\Throwable $e) {
$message = \sprintf(self::ERROR_INVALID_GRPC_METHOD, $method->getName());
throw GRPCException::create($message, StatusCode::INTERNAL, $e);
}

[, $input] = $method->getParameters();

/** @var \ReflectionNamedType $inputType */
$inputType = $input->getType();

/** @var \ReflectionNamedType $returnType */
$returnType = $method->getReturnType();

/** @psalm-suppress ArgumentTypeCoercion */
return new self($method->getName(), $inputType->getName(), $returnType->getName());
}
}
1 change: 0 additions & 1 deletion src/ResponseHeaders.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ public function set(string $key, string $value): void

/**
* @param THeaderKey $key
* @param string|null $default
* @return THeaderValue|null
*/
public function get(string $key, ?string $default = null): ?string
Expand Down
71 changes: 35 additions & 36 deletions src/Server.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ final class Server
public function __construct(
private readonly InvokerInterface $invoker = new Invoker(),
private readonly array $options = [],
) {
}
) {}

/**
* Register new GRPC service.
Expand All @@ -63,39 +62,6 @@ public function registerService(string $interface, ServiceInterface $service): v
$this->services[$service->getName()] = $service;
}

/**
* @param ContextResponse $data
* @return array{0: string, 1: string}
* @throws \JsonException
* @throws \Throwable
*/
private function tick(string $body, array $data): array
{
$context = (new Context($data['context']))
->withValue(ResponseHeaders::class, new ResponseHeaders());

$response = $this->invoke($data['service'], $data['method'], $context, $body);

/** @var ResponseHeaders|null $responseHeaders */
$responseHeaders = $context->getValue(ResponseHeaders::class);
$responseHeadersString = $responseHeaders ? $responseHeaders->packHeaders() : '{}';

return [$response, $responseHeadersString];
}

/**
* @psalm-suppress InaccessibleMethod
*/
private function workerSend(WorkerInterface $worker, string $body, string $headers): void
{
$worker->respond(new Payload($body, $headers));
}

private function workerError(WorkerInterface $worker, string $message): void
{
$worker->error($message);
}

/**
* Serve GRPC over given RoadRunner worker.
*/
Expand All @@ -121,7 +87,7 @@ public function serve(?WorkerInterface $worker = null, ?callable $finalize = nul
} catch (GRPCExceptionInterface $e) {
$this->workerGrpcError($worker, $e);
} catch (\Throwable $e) {
$this->workerError($worker, $this->isDebugMode() ? (string)$e : $e->getMessage());
$this->workerError($worker, $this->isDebugMode() ? (string) $e : $e->getMessage());
} finally {
if ($finalize !== null) {
isset($e) ? $finalize($e) : $finalize();
Expand All @@ -146,6 +112,39 @@ protected function invoke(string $service, string $method, ContextInterface $con
return $this->services[$service]->invoke($method, $context, $body);
}

/**
* @param ContextResponse $data
* @return array{0: string, 1: string}
* @throws \JsonException
* @throws \Throwable
*/
private function tick(string $body, array $data): array
{
$context = (new Context($data['context']))
->withValue(ResponseHeaders::class, new ResponseHeaders());

$response = $this->invoke($data['service'], $data['method'], $context, $body);

/** @var ResponseHeaders|null $responseHeaders */
$responseHeaders = $context->getValue(ResponseHeaders::class);
$responseHeadersString = $responseHeaders ? $responseHeaders->packHeaders() : '{}';

return [$response, $responseHeadersString];
}

/**
* @psalm-suppress InaccessibleMethod
*/
private function workerSend(WorkerInterface $worker, string $body, string $headers): void
{
$worker->respond(new Payload($body, $headers));
}

private function workerError(WorkerInterface $worker, string $message): void
{
$worker->error($message);
}

private function workerGrpcError(WorkerInterface $worker, GRPCExceptionInterface $e): void
{
$status = new Status([
Expand Down
4 changes: 1 addition & 3 deletions src/ServiceInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,4 @@
/**
* Indicates that given class expected to be GRPC service.
*/
interface ServiceInterface
{
}
interface ServiceInterface {}
Loading
Loading