-
Notifications
You must be signed in to change notification settings - Fork 2
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
[SPN-1440] Web Api Documentation Check and Deprecated API Attribute Metric #125
Merged
bknutson123
merged 5 commits into
BK/integators/SPN-1438/guardrailDeprecatedMetric
from
BK/integators/SPN-1440/deprecatedApiAttributeMetric
Jan 23, 2025
Merged
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
f4798fc
[SPN-1440] New Guardrail Check to ensure documentation is present and…
bknutson123 05d9571
fix test by ignoring documentation requirement
bknutson123 e7a07a2
add search phrases check
bknutson123 64eaa72
Merge branch 'refs/heads/BK/integators/SPN-1438/guardrailDeprecatedMe…
bknutson123 e88a91c
rename check to open api attribute documentation instead of web api d…
bknutson123 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
<?php | ||
|
||
namespace BambooHR\Guardrail\Checks; | ||
|
||
use BambooHR\Guardrail\Metrics\Metric; | ||
use BambooHR\Guardrail\Metrics\MetricOutputInterface; | ||
use BambooHR\Guardrail\Scope; | ||
use PhpParser\Node; | ||
use PhpParser\Node\Stmt\ClassLike; | ||
|
||
class WebApiDocumentationCheck extends BaseCheck { | ||
private const string ATTRIBUTE_NAMESPACE = 'OpenApi\Attributes'; | ||
private const string SEARCH_PHRASES_KEY = 'vector-search-phrases'; | ||
private const string DEPRECATED_KEY = 'deprecated'; | ||
private const string X_KEY = 'x'; | ||
|
||
function __construct($index, $output, private readonly MetricOutputInterface $metricOutput) { | ||
parent::__construct($index, $output); | ||
} | ||
|
||
/** | ||
* getCheckNodeTypes | ||
* | ||
* @return string[] | ||
*/ | ||
function getCheckNodeTypes() { | ||
return [Node\Stmt\ClassMethod::class]; | ||
} | ||
|
||
/** | ||
* @param string $fileName The name of the file we are parsing | ||
* @param Node $node Instance of the Node | ||
* @param ClassLike|null $inside Instance of the ClassLike (the class we are parsing) [optional] | ||
* @param Scope|null $scope Instance of the Scope (all variables in the current state) [optional] | ||
* | ||
* @return void | ||
*/ | ||
public function run($fileName, Node $node, ClassLike $inside = null, Scope $scope = null) { | ||
if ($node instanceof Node\Stmt\ClassMethod && $node->isPublic()) { | ||
foreach ($node->attrGroups as $attrGroup) { | ||
foreach ($attrGroup->attrs as $attribute) { | ||
$attributeName = $attribute->name->toString(); | ||
if (str_starts_with($attributeName, self::ATTRIBUTE_NAMESPACE)) { | ||
$hasDefinedSearchPhrases = false; | ||
foreach ($attribute->args as $arg) { | ||
$this->checkDeprecatedAttribute($arg, $fileName, $node); | ||
$hasDefinedSearchPhrases = $hasDefinedSearchPhrases ?: $this->hasVectorSearchPhrase($arg); | ||
} | ||
if (!$hasDefinedSearchPhrases) { | ||
$this->emitErrorOnLine( | ||
$fileName, | ||
$node->getLine(), | ||
ErrorConstants::TYPE_WEB_API_DOCUMENTATION_SEARCH_PHRASES_CHECK, | ||
"OpenAPI Attribute must have a vector-search-phrases key defined. Method: {$node->name->name}" | ||
); | ||
} | ||
return; | ||
} | ||
} | ||
} | ||
$className = $inside->namespacedName->toString(); | ||
$this->emitErrorOnLine( | ||
$fileName, | ||
$node->getLine(), | ||
ErrorConstants::TYPE_WEB_API_DOCUMENTATION_CHECK, | ||
"All public controller methods should be associated with a route and must have | ||
documentation through an OpenAPI Attribute. Method: {$node->name->name}, Class: $className" | ||
); | ||
} | ||
} | ||
|
||
private function checkDeprecatedAttribute($arg, $fileName, $node) { | ||
if ($arg->name->name === self::DEPRECATED_KEY && $arg->value->name->toString() == 'true') { | ||
$this->metricOutput->emitMetric(new Metric( | ||
$fileName, | ||
$node->getLine(), | ||
ErrorConstants::TYPE_METRICS_DEPRECATED_FUNCTIONS, | ||
jlesueur marked this conversation as resolved.
Show resolved
Hide resolved
|
||
[] | ||
)); | ||
} | ||
} | ||
|
||
private function hasVectorSearchPhrase($arg): bool { | ||
if ($arg->name->name === self::X_KEY && $arg->value instanceof Node\Expr\Array_) { | ||
foreach ($arg->value->items as $item) { | ||
if ($item->key->value === self::SEARCH_PHRASES_KEY) { | ||
return true; | ||
} | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
29 changes: 29 additions & 0 deletions
29
tests/units/Checks/TestData/TestWebApiDocumentationCheck.1.inc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
<?php | ||
use OpenApi\Attributes as OA; | ||
|
||
|
||
class MyController { | ||
/** | ||
* @return bool | ||
*/ | ||
#[\Onsen\SecurityAudit\Sensitivity\Low] | ||
#[OA\Get(path: "/test")] | ||
public function hasAttribute() { | ||
return false; | ||
} | ||
|
||
/** | ||
* @return int | ||
*/ | ||
public function doesNotHaveAttribute() { | ||
return 123; | ||
} | ||
|
||
/** | ||
* @return int | ||
*/ | ||
#[\Onsen\SecurityAudit\Sensitivity\Low] | ||
public function hasFakeAttribute() { | ||
return 456; | ||
} | ||
} |
17 changes: 17 additions & 0 deletions
17
tests/units/Checks/TestData/TestWebApiDocumentationCheck.2.inc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
<?php | ||
use OpenApi\Attributes as OA; | ||
|
||
class MyController { | ||
function undefinedVisibilityMethod() { | ||
return false; | ||
} | ||
public function publicMethod() { | ||
return false; | ||
} | ||
protected function protectedMethod() { | ||
return false; | ||
} | ||
private function privateMethod() { | ||
return false; | ||
} | ||
} |
18 changes: 18 additions & 0 deletions
18
tests/units/Checks/TestData/TestWebApiDocumentationCheck.3.inc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
<?php | ||
use OpenApi\Attributes as OA; | ||
|
||
|
||
class MyController { | ||
/** | ||
* @return bool | ||
*/ | ||
#[OA\Get(path: "/test", deprecated: true)] | ||
public function hasDeprecatedAttribute() { | ||
return false; | ||
} | ||
|
||
#[OA\Get(path: "/test")] | ||
public function doesNotHaveDeprecatedAttribute() { | ||
return false; | ||
} | ||
} |
20 changes: 20 additions & 0 deletions
20
tests/units/Checks/TestData/TestWebApiDocumentationCheck.4.inc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
<?php | ||
use OpenApi\Attributes as OA; | ||
|
||
|
||
class MyController { | ||
#[OA\Get(path: "/test")] | ||
public function withoutSearchPhrases() { | ||
return false; | ||
} | ||
|
||
#[OA\Get(path: "/test", x: ['vector-search-phrases' => []])] | ||
public function withEmptySearchPhrases() { | ||
return false; | ||
} | ||
|
||
#[OA\Get(path: "/test", x: ['vector-search-phrases' => ['my test search phrase']])] | ||
public function withSearchPhrases() { | ||
return false; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
<?php | ||
|
||
namespace BambooHR\Guardrail\Tests\units\Checks; | ||
|
||
use BambooHR\Guardrail\Checks\ErrorConstants; | ||
use BambooHR\Guardrail\Tests\TestSuiteSetup; | ||
|
||
class TestWebApiDocumentationCheck extends TestSuiteSetup { | ||
/** | ||
* testApiAttributeIsPresent | ||
* | ||
* @return void | ||
*/ | ||
public function testApiAttributeIsPresent() { | ||
$this->assertEquals(2, $this->runAnalyzerOnFile('.1.inc', ErrorConstants::TYPE_WEB_API_DOCUMENTATION_CHECK), ""); | ||
} | ||
|
||
/** | ||
* @return void | ||
*/ | ||
public function testOnlyErrorsOnPublicMethods() { | ||
$this->assertEquals(2, $this->runAnalyzerOnFile('.2.inc', ErrorConstants::TYPE_WEB_API_DOCUMENTATION_CHECK), ""); | ||
} | ||
|
||
/** | ||
* @return void | ||
*/ | ||
public function testMethodWithDeprecatedAttribute() { | ||
$output = $this->getOutputFromAnalyzer('.3.inc', ErrorConstants::TYPE_METRICS_DEPRECATED_FUNCTIONS); | ||
$this->assertEquals(1, $this->getMetricCountByName($output, ErrorConstants::TYPE_METRICS_DEPRECATED_FUNCTIONS)); | ||
} | ||
|
||
/** | ||
* @return void | ||
*/ | ||
public function testWithAndWithoutVectorSearchPhrases() { | ||
$this->assertEquals(1, $this->runAnalyzerOnFile('.4.inc', ErrorConstants::TYPE_WEB_API_DOCUMENTATION_SEARCH_PHRASES_CHECK), ""); | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to talk about names, we might name this something a bit more specifically aimed at OA attributes?