refactor: simplify deployment skip patterns

This commit is contained in:
harsh mahajan
2026-05-11 18:56:31 +05:30
parent cc0207cfe4
commit dda38442ad
4 changed files with 81 additions and 121 deletions
@@ -8,7 +8,7 @@ use Appwrite\Event\Publisher\Build as BuildPublisher;
use Appwrite\Extend\Exception;
use Appwrite\Filter\BranchDomain as BranchDomainFilter;
use Appwrite\Vcs\Comment;
use Appwrite\Vcs\Validator\CommitSkipPatterns;
use Appwrite\Vcs\Validator\DeploymentSkipPatterns;
use Utopia\Config\Config;
use Utopia\Console;
use Utopia\Database\Database;
@@ -96,7 +96,9 @@ trait Deployment
$resource = $authorization->skip(fn () => $dbForProject->getDocument($resourceCollection, $resourceId));
$resourceInternalId = $resource->getSequence();
if (!$this->isResourceBuildable($logBase, $providerCommitMessage)) {
$commitSkip = new DeploymentSkipPatterns();
if (!$commitSkip->isValid($providerCommitMessage)) {
Span::add("{$logBase}.build.skipped.reason", 'commitMessage');
Span::add("{$logBase}.build.skipped", 'true');
continue;
}
@@ -568,14 +570,4 @@ trait Deployment
return System::getEnv('_APP_BUILDS_QUEUE_NAME', Event::BUILDS_QUEUE_NAME);
}
private function isResourceBuildable(string $logBase, string $providerCommitMessage = ''): bool
{
$commitSkip = new CommitSkipPatterns();
if (!$commitSkip->isValid($providerCommitMessage)) {
Span::add("{$logBase}.build.skipped.reason", 'commitMessage');
return false;
}
return true;
}
}
@@ -1,90 +0,0 @@
<?php
namespace Appwrite\Vcs\Validator;
use Utopia\Validator;
class CommitSkipPatterns extends Validator
{
private const PATTERNS = [
'[skip ci]',
'[ci skip]',
'[no ci]',
'[skip action]',
'[action skip]',
'[no action]',
'[skip actions]',
'[actions skip]',
'[no actions]',
'[skip deploy]',
'[deploy skip]',
'[no deploy]',
'[skip appwrite]',
'[appwrite skip]',
'[no appwrite]',
];
/**
* Returns false (skip deployment) when the commit message contains any of the
* known skip directives as a standalone directive (case-insensitive).
* Returns true (proceed) when none match.
*
* Matching rules:
* - Case-insensitive
* - The directive must be surrounded by whitespace or string boundaries, so
* "prefix[skip deploy]suffix" does NOT accidentally skip
* - Internal whitespace in the pattern is normalised: tokens are split on \s+
* and rejoined with \s+ in the regex, so "[skip deploy]" matches
* "[skip deploy]".
*/
public function isValid($value): bool
{
if (!is_string($value)) {
return true;
}
foreach (self::PATTERNS as $pattern) {
$pattern = trim($pattern);
// Split on whitespace; each token is regex-quoted. Tokens are rejoined
// with \s+ (required space) so that "skipappwrite" does NOT match the
// pattern "skip appwrite".
$tokens = preg_split('/\s+/', $pattern);
$regexParts = [];
$count = count($tokens);
for ($i = 0; $i < $count; $i++) {
$regexParts[] = preg_quote($tokens[$i], '~');
if ($i < $count - 1) {
$regexParts[] = '\s+';
}
}
$regexBody = implode('', $regexParts);
// (?<!\S) / (?!\S) assert whitespace (or string edge) on both sides,
// ensuring the directive is a standalone group, not buried inside a
// longer token like "prefix[skip deploy]suffix".
$regex = '~(?<!\S)' . $regexBody . '(?!\S)~i';
if (preg_match($regex, $value)) {
return false;
}
}
return true;
}
public function getDescription(): string
{
return 'Commit message must not contain any of the configured skip patterns.';
}
public function isArray(): bool
{
return false;
}
public function getType(): string
{
return self::TYPE_STRING;
}
}
@@ -0,0 +1,66 @@
<?php
namespace Appwrite\Vcs\Validator;
use Utopia\Validator;
class DeploymentSkipPatterns extends Validator
{
private const PATTERNS = [
'[skip ci]',
'[ci skip]',
'[no ci]',
'[skip action]',
'[action skip]',
'[no action]',
'[skip actions]',
'[actions skip]',
'[no actions]',
'[skip deploy]',
'[deploy skip]',
'[no deploy]',
'[skip appwrite]',
'[appwrite skip]',
'[no appwrite]',
];
/**
* Returns false (skip deployment) when the commit message contains any of the
* known skip directives as a standalone directive (case-insensitive).
* Returns true (proceed) when none match.
*
* Matching rules:
* - Case-insensitive
*/
public function isValid($value): bool
{
if (!is_string($value)) {
return true;
}
$value = strtolower($value);
foreach (self::PATTERNS as $pattern) {
if (str_contains($value, $pattern)) {
return false;
}
}
return true;
}
public function getDescription(): string
{
return 'Commit message must not contain any of the configured skip patterns.';
}
public function isArray(): bool
{
return false;
}
public function getType(): string
{
return self::TYPE_STRING;
}
}
@@ -2,14 +2,14 @@
namespace Tests\Unit\Vcs\Validator;
use Appwrite\Vcs\Validator\CommitSkipPatterns;
use Appwrite\Vcs\Validator\DeploymentSkipPatterns;
use PHPUnit\Framework\TestCase;
class CommitSkipPatternsTest extends TestCase
class DeploymentSkipPatternsTest extends TestCase
{
public function testKnownSkipDirectivesSkip(): void
{
$validator = new CommitSkipPatterns();
$validator = new DeploymentSkipPatterns();
$this->assertFalse($validator->isValid('[skip ci] update changelog'));
$this->assertFalse($validator->isValid('[ci skip] update changelog'));
@@ -30,7 +30,7 @@ class CommitSkipPatternsTest extends TestCase
public function testKnownSkipDirectivesAreCaseInsensitive(): void
{
$validator = new CommitSkipPatterns();
$validator = new DeploymentSkipPatterns();
$this->assertFalse($validator->isValid('[SKIP CI] update changelog'));
$this->assertFalse($validator->isValid('[Skip Deploy] update changelog'));
@@ -41,7 +41,7 @@ class CommitSkipPatternsTest extends TestCase
public function testMessageWithoutKnownDirectiveProceeds(): void
{
$validator = new CommitSkipPatterns();
$validator = new DeploymentSkipPatterns();
$this->assertTrue($validator->isValid('fix: real bug fix'));
$this->assertTrue($validator->isValid('feat: add new feature'));
@@ -50,36 +50,28 @@ class CommitSkipPatternsTest extends TestCase
$this->assertTrue($validator->isValid('skip-checks:true'));
}
public function testDirectiveMustBeStandalone(): void
public function testDirectiveCanAppearAnywhere(): void
{
$validator = new CommitSkipPatterns();
$validator = new DeploymentSkipPatterns();
$this->assertFalse($validator->isValid('docs: update readme [skip deploy]'));
$this->assertTrue($validator->isValid('docs: update readme[skip deploy]'));
$this->assertTrue($validator->isValid('prefix[skip deploy]suffix'));
$this->assertFalse($validator->isValid('docs: update readme[skip deploy]'));
$this->assertFalse($validator->isValid('prefix[skip deploy]suffix'));
$this->assertTrue($validator->isValid('refactor: skip appwrite cache seeding'));
$this->assertTrue($validator->isValid('fix: appwrite skip quota check in tests'));
}
public function testMultilineCommitMessageSkips(): void
{
$validator = new CommitSkipPatterns();
$validator = new DeploymentSkipPatterns();
$message = "feat: add new stuff\n\nMore detail here.\n\n[skip deploy]";
$this->assertFalse($validator->isValid($message));
}
public function testWhitespaceInsideDirectiveIsNormalized(): void
{
$validator = new CommitSkipPatterns();
$this->assertFalse($validator->isValid('[skip deploy] docs only'));
$this->assertFalse($validator->isValid('[no actions] docs only'));
}
public function testNonStringCommitMessageProceeds(): void
{
$validator = new CommitSkipPatterns();
$validator = new DeploymentSkipPatterns();
$this->assertTrue($validator->isValid(null));
$this->assertTrue($validator->isValid([]));