-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add a clear task #40
Add a clear task #40
Changes from all commits
0505e7f
ec9312f
a396385
4487251
176f50e
9aa6b7f
5f2bf2b
5a63b45
b946f56
1c4a0b6
ee340ce
9f73902
5b11725
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
version: ~> 1.0 | ||
|
||
import: | ||
- silverstripe/silverstripe-travis-shared:config/provision/standard-jobs-range.yml |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,18 +2,23 @@ | |
"name": "silverstripe/graphql-devtools", | ||
"description": "Tools to help developers building new applications on SilverStripe’s GraphQL API", | ||
"type": "silverstripe-vendormodule", | ||
"license": "BSD-3-Clause", | ||
"require": { | ||
"silverstripe/graphql": "*" | ||
"silverstripe/graphql": "^3 || ^4", | ||
"symfony/finder": "^4 || ^5", | ||
"symfony/filesystem": "^4 || ^5" | ||
}, | ||
"require-dev": { | ||
"phpunit/phpunit": "^9.5", | ||
"squizlabs/php_codesniffer": "^3.0" | ||
}, | ||
"autoload": { | ||
"psr-4": { | ||
"SilverStripe\\GraphQLDevTools\\": "src/" | ||
"SilverStripe\\GraphQLDevTools\\": "src/", | ||
"SilverStripe\\GraphQLDevTools\\Tests\\": "tests/" | ||
} | ||
}, | ||
"extra": { | ||
"branch-alias": { | ||
"dev-master": "1.x-dev" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this being removed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There a history of these things being in Silverstripe modules, they're generally out of date so more annoying than helpful. These days we remove these things any time we see them. I don't think this needs a separate PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh there's no There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I agree it's okay in this PR so long as we can say why it's happening, and if it should happen. Which you've now provided! Thanks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My instinct would be to start tagging release numbers for this. |
||
}, | ||
"expose": [ | ||
"client/" | ||
] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<ruleset name="SilverStripe"> | ||
<description>CodeSniffer ruleset for SilverStripe coding conventions.</description> | ||
|
||
<file>src</file> | ||
<file>tests</file> | ||
|
||
<exclude-pattern>*/\.graphql-generated/*</exclude-pattern> | ||
|
||
<!-- base rules are PSR-2 --> | ||
<rule ref="PSR2" > | ||
<!-- Current exclusions --> | ||
<exclude name="PSR1.Methods.CamelCapsMethodName" /> | ||
<exclude name="PSR1.Files.SideEffects.FoundWithSymbols" /> | ||
<exclude name="PSR2.Classes.PropertyDeclaration" /> | ||
<exclude name="PSR2.ControlStructures.SwitchDeclaration" /> <!-- causes php notice while linting --> | ||
<exclude name="PSR2.ControlStructures.SwitchDeclaration.WrongOpenercase" /> | ||
<exclude name="PSR2.ControlStructures.SwitchDeclaration.WrongOpenerdefault" /> | ||
<exclude name="PSR2.ControlStructures.SwitchDeclaration.TerminatingComment" /> | ||
<exclude name="PSR2.Methods.MethodDeclaration.Underscore" /> | ||
<exclude name="Squiz.Scope.MethodScope" /> | ||
<exclude name="Squiz.Classes.ValidClassName.NotCamelCaps" /> | ||
<exclude name="Generic.Files.LineLength.TooLong" /> | ||
<exclude name="PEAR.Functions.ValidDefaultValue.NotAtEnd" /> | ||
</rule> | ||
|
||
</ruleset> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
<phpunit bootstrap="vendor/silverstripe/framework/tests/bootstrap.php" colors="true"> | ||
|
||
<testsuites> | ||
<testsuite name="Default"> | ||
<directory>tests</directory> | ||
</testsuite> | ||
</testsuites> | ||
|
||
<filter> | ||
<whitelist addUncoveredFilesFromWhitelist="true"> | ||
<directory suffix=".php">src/</directory> | ||
<exclude> | ||
<directory suffix=".php">tests/</directory> | ||
</exclude> | ||
</whitelist> | ||
</filter> | ||
|
||
</phpunit> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
<?php | ||
namespace SilverStripe\GraphQLDevTools; | ||
|
||
use Psr\Log\LoggerInterface; | ||
use SilverStripe\Control\Controller; | ||
use SilverStripe\Control\HTTPRequest; | ||
use SilverStripe\Core\Injector\Injector; | ||
use SilverStripe\GraphQL\Schema\Storage\CodeGenerationStore; | ||
use Symfony\Component\Filesystem\Filesystem; | ||
use Symfony\Component\Finder\Finder; | ||
|
||
class Clear extends Controller | ||
{ | ||
private static $url_handlers = [ | ||
'' => 'clear', | ||
]; | ||
|
||
private static $allowed_actions = [ | ||
'clear', | ||
]; | ||
|
||
public function clear(HTTPRequest $request): void | ||
{ | ||
$logger = Injector::inst()->get(LoggerInterface::class . '.graphql-build'); | ||
$dirName = CodeGenerationStore::config()->get('dirName'); | ||
$expectedPath = BASE_PATH . DIRECTORY_SEPARATOR . $dirName; | ||
$fs = new Filesystem(); | ||
|
||
$finder = new Finder(); | ||
// Make finder not recursive | ||
$finder->depth('== 0'); | ||
|
||
$logger->info('Clearing GraphQL code generation directory'); | ||
if ($fs->exists($expectedPath)) { | ||
$logger->info('Directory has been found'); | ||
if ($finder->in($expectedPath)->hasResults()) { | ||
foreach ($finder as $file) { | ||
$logger->info('Removing ' . $file->getFilename()); | ||
$fs->remove($file->getRealPath()); | ||
} | ||
$logger->info('Directory now empty'); | ||
} else { | ||
$logger->info('Directory is already empty.'); | ||
} | ||
} else { | ||
$logger->info('Directory was not found. There is nothing to clear'); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
<?php | ||
|
||
namespace SilverStripe\GraphQLDevTools\Tests; | ||
|
||
use SilverStripe\Dev\FunctionalTest; | ||
use SilverStripe\GraphQL\Schema\Storage\CodeGenerationStore; | ||
use Symfony\Component\Filesystem\Filesystem; | ||
use Symfony\Component\Finder\Finder; | ||
use SilverStripe\GraphQL\Schema\Logger; | ||
use SilverStripe\GraphQL\Schema\Schema; | ||
|
||
class ClearTest extends FunctionalTest | ||
{ | ||
|
||
private $originalDirName; | ||
private $dirName = 'test-graphql-generated'; | ||
private $absDirName = BASE_PATH . DIRECTORY_SEPARATOR . 'test-graphql-generated'; | ||
|
||
protected $usesDatabase = true; | ||
|
||
protected function setUp(): void | ||
{ | ||
parent::setUp(); | ||
if (!class_exists(Schema::class)) { | ||
$this->markTestSkipped('GraphQL 4 test ' . __CLASS__ . ' skipped'); | ||
return; | ||
} | ||
|
||
$this->originalDirName = CodeGenerationStore::config()->get('dirName'); | ||
Logger::singleton()->setVerbosity(Logger::EMERGENCY); | ||
CodeGenerationStore::config()->set('dirName', $this->dirName); | ||
|
||
$fs = new Filesystem(); | ||
$fs->mkdir($this->absDirName); | ||
} | ||
|
||
protected function tearDown(): void | ||
{ | ||
parent::tearDown(); | ||
CodeGenerationStore::config()->set('dirName', $this->originalDirName); | ||
$fs = new Filesystem(); | ||
if ($fs->exists($this->absDirName)) { | ||
$fs->remove($this->absDirName); | ||
} | ||
} | ||
|
||
public function testClear() | ||
{ | ||
$fs = new Filesystem(); | ||
$finder = new Finder(); | ||
$finder->in($this->absDirName); | ||
|
||
$this->assertTrue($fs->exists($this->absDirName), 'Test should begin with a fake code gen folder'); | ||
|
||
$this->get('dev/graphql/clear'); | ||
$this->assertFalse($finder->hasResults(), 'GraphQL clear should not break on an empty folder'); | ||
|
||
$fs->mkdir($this->absDirName . DIRECTORY_SEPARATOR . 'default'); | ||
$this->assertTrue($finder->hasResults(), 'A fake schema folder should have been created'); | ||
|
||
$this->get('dev/graphql/clear'); | ||
$this->assertFalse($finder->hasResults(), 'GraphQL clear should have removed the fake schema folder'); | ||
|
||
$fs->remove($this->absDirName); | ||
$this->get('dev/graphql/clear'); | ||
$this->assertFalse($fs->exists($this->absDirName), 'GraphQL clear should not break on a non-existent folder'); | ||
} | ||
} |
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.
Does this actually work with graphql 3? I'm seeing imports in other files such as
SilverStripe\GraphQL\Schema\Logger
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.
The clear functionality doesn't work with v3 - but it also isn't available with v3 (see yml config)
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.
Oh right but the rest of this module does work with graphql3? Just not clear?
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.
I believe it does - it at least used to and is supposed to, and I don't think anything in PR would stop it from doing so.
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.
It's meant to work with both of them. However, I'm thinking we should tag a v3 version and a v4 version to avoid confusion. I thought of doing it in this PR, but the travis build wanted to test both of them any way, so it was failing the build.
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 it's meant to have dual support, then why not just let it have dual support? Especially since at this point it will be more work to unpick it.
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.
Yeah, the dual support is a bit awful, though pragmatically we should just leave as is. Perhaps do a new major at CMS 5 time that is graphql 4 only