From 7890f006c1ca9c6854bfac275a2de46e53983a67 Mon Sep 17 00:00:00 2001 From: David Jardin Date: Sun, 17 Nov 2024 12:41:30 +0100 Subject: [PATCH 1/5] wip commit --- .../Controllers/Api/V1/SiteController.php | 25 ++++++------------- app/Http/Requests/SiteRequest.php | 16 ++++++++++++ 2 files changed, 23 insertions(+), 18 deletions(-) create mode 100644 app/Http/Requests/SiteRequest.php diff --git a/app/Http/Controllers/Api/V1/SiteController.php b/app/Http/Controllers/Api/V1/SiteController.php index b63c749..54319d9 100644 --- a/app/Http/Controllers/Api/V1/SiteController.php +++ b/app/Http/Controllers/Api/V1/SiteController.php @@ -3,6 +3,7 @@ namespace App\Http\Controllers\Api\V1; use App\Http\Controllers\Controller; +use App\Http\Requests\SiteRequest; use App\Jobs\CheckSiteHealth; use App\Models\Site; use App\RemoteSite\Connection; @@ -23,20 +24,16 @@ class SiteController extends Controller use ApiResponse; /** - * @param Request $request + * @param SiteRequest $request * * @return JsonResponse * @throws \Exception */ - public function register(Request $request): JsonResponse + public function register(SiteRequest $request): JsonResponse { $url = $request->string('url'); $key = $request->string('key'); - if ($url->isEmpty() || $key->isEmpty()) { - return $this->error('BadRequest'); - } - $connectionService = new Connection($url, $key); // Do a health check @@ -66,19 +63,15 @@ public function register(Request $request): JsonResponse } /** - * @param Request $request + * @param SiteRequest $request * * @return JsonResponse */ - public function check(Request $request): JsonResponse + public function check(SiteRequest $request): JsonResponse { $url = $request->string('url'); $key = $request->string('key'); - if ($url->isEmpty() || $key->isEmpty()) { - return $this->error('BadRequest'); - } - $connectionService = new Connection($url, $key); // Do a health check @@ -94,19 +87,15 @@ public function check(Request $request): JsonResponse } /** - * @param Request $request + * @param SiteRequest $request * * @return JsonResponse */ - public function delete(Request $request): JsonResponse + public function delete(SiteRequest $request): JsonResponse { $url = $request->string('url'); $key = $request->string('key'); - if ($url->isEmpty() || $key->isEmpty()) { - return $this->error('BadRequest'); - } - try { Site::where('url', $url)->where('key', $key)->delete(); } catch (\Exception $e) { diff --git a/app/Http/Requests/SiteRequest.php b/app/Http/Requests/SiteRequest.php new file mode 100644 index 0000000..76c2b9a --- /dev/null +++ b/app/Http/Requests/SiteRequest.php @@ -0,0 +1,16 @@ + 'required|url', + 'key' => 'required|string|min:32|max:64', + ]; + } +} From 18354397ed8ba3683b0d43fd71954c75214cde55 Mon Sep 17 00:00:00 2001 From: David Jardin Date: Sun, 17 Nov 2024 13:07:22 +0100 Subject: [PATCH 2/5] added controller tests --- .../Controllers/Api/V1/SiteController.php | 3 +- ..._remove_patch_minor_columns_from_sites.php | 32 ++++++++++ phpunit.xml | 4 +- tests/Feature/Api/SiteControllerTest.php | 59 +++++++++++++++++++ tests/Feature/ExampleTest.php | 19 ------ 5 files changed, 95 insertions(+), 22 deletions(-) create mode 100644 database/migrations/2024_11_17_115646_remove_patch_minor_columns_from_sites.php create mode 100644 tests/Feature/Api/SiteControllerTest.php delete mode 100644 tests/Feature/ExampleTest.php diff --git a/app/Http/Controllers/Api/V1/SiteController.php b/app/Http/Controllers/Api/V1/SiteController.php index 54319d9..8058a8d 100644 --- a/app/Http/Controllers/Api/V1/SiteController.php +++ b/app/Http/Controllers/Api/V1/SiteController.php @@ -13,6 +13,7 @@ use GuzzleHttp\Exception\ServerException; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; +use Illuminate\Support\Facades\App; /** * Class SiteController @@ -34,7 +35,7 @@ public function register(SiteRequest $request): JsonResponse $url = $request->string('url'); $key = $request->string('key'); - $connectionService = new Connection($url, $key); + $connectionService = App::makeWith(Connection::class, [$url, $key]); // Do a health check try { diff --git a/database/migrations/2024_11_17_115646_remove_patch_minor_columns_from_sites.php b/database/migrations/2024_11_17_115646_remove_patch_minor_columns_from_sites.php new file mode 100644 index 0000000..7529f8b --- /dev/null +++ b/database/migrations/2024_11_17_115646_remove_patch_minor_columns_from_sites.php @@ -0,0 +1,32 @@ +dropColumn('update_patch'); + $table->dropColumn('update_minor'); + $table->dropColumn('update_major'); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('sites', function (Blueprint $table) { + $table->boolean('update_patch'); + $table->boolean('update_minor'); + $table->boolean('update_major'); + }); + } +}; diff --git a/phpunit.xml b/phpunit.xml index 506b9a3..61c031c 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -22,8 +22,8 @@ - - + + diff --git a/tests/Feature/Api/SiteControllerTest.php b/tests/Feature/Api/SiteControllerTest.php new file mode 100644 index 0000000..9e6e3ed --- /dev/null +++ b/tests/Feature/Api/SiteControllerTest.php @@ -0,0 +1,59 @@ +postJson( + '/api/v1/register', + ); + + $response->assertStatus(422); + } + + public function testRegisteringASiteWithUrlAndKeyCreatesRow(): void + { + App::bind(Connection::class, fn() => $this->getConnectionMock(HealthCheck::from([ + "php_version" => "1.0.0", + "db_type" => "mysqli", + "db_version" => "1.0.0", + "cms_version" => "1.0.0", + "server_os" => "Joomla OS 1.0.0" + ]))); + + $response = $this->postJson( + '/api/v1/register', + ["url" => "https://www.joomla.org", "key" => "foobar123foobar123foobar123foobar123"] + ); + + $response->assertStatus(200); + $result = Site::where('url', 'https://www.joomla.org')->first(); + + $this->assertEquals([ + + ], $result->toArray()); + } + + protected function getConnectionMock(HealthCheck $response) + { + $mock = $this->getMockBuilder(Connection::class) + ->disableOriginalConstructor() + ->onlyMethods(['__call']) + ->getMock(); + + $mock->method('__call')->willReturn($response); + + return $mock; + } +} diff --git a/tests/Feature/ExampleTest.php b/tests/Feature/ExampleTest.php deleted file mode 100644 index 8364a84..0000000 --- a/tests/Feature/ExampleTest.php +++ /dev/null @@ -1,19 +0,0 @@ -get('/'); - - $response->assertStatus(200); - } -} From b4bb2246dc1cf3af190ffb415690e7d495829886 Mon Sep 17 00:00:00 2001 From: David Jardin Date: Sun, 17 Nov 2024 13:34:12 +0100 Subject: [PATCH 3/5] fix webservice request headers --- app/Http/Controllers/Api/V1/SiteController.php | 4 ++-- app/RemoteSite/Connection.php | 12 +++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/app/Http/Controllers/Api/V1/SiteController.php b/app/Http/Controllers/Api/V1/SiteController.php index 089d127..85d9bd8 100644 --- a/app/Http/Controllers/Api/V1/SiteController.php +++ b/app/Http/Controllers/Api/V1/SiteController.php @@ -34,7 +34,7 @@ public function register(SiteRequest $request): JsonResponse $url = $request->string('url'); $key = $request->string('key'); - $connectionService = App::makeWith(Connection::class, [$url, $key]); + $connectionService = App::makeWith(Connection::class, ["baseUrl" => $url, "key" => $key]); // Do a health check try { @@ -49,7 +49,7 @@ public function register(SiteRequest $request): JsonResponse $site = new Site(); $site->key = $key; - $site->url = $url; + $site->url = rtrim($url, "/"); $site->last_seen = Carbon::now(); // Fill with site info diff --git a/app/RemoteSite/Connection.php b/app/RemoteSite/Connection.php index 7b2cab7..e4fa96f 100644 --- a/app/RemoteSite/Connection.php +++ b/app/RemoteSite/Connection.php @@ -67,15 +67,13 @@ public function performExtractionRequest(array $requestData): array ] ); - $responseData = $this->decodeResponse($response, $request); - - return $responseData; + return $this->decodeResponse($response, $request); } protected function performWebserviceRequest( HttpMethod $method, string $endpoint, - array $requestData = [] + ?array $requestData = null ): array { $request = new Request( $method->name, @@ -89,7 +87,11 @@ protected function performWebserviceRequest( $response = $this->performHttpRequest( $request, [ - "json" => $requestData + "json" => $requestData, + "headers" => [ + "Content-Type" => "application/json", + "Accept" => "application/vnd.api+json" + ] ] ); From dda172d99a973fa53b89f7c7c0b4441a8e1db8af Mon Sep 17 00:00:00 2001 From: David Jardin Date: Sun, 17 Nov 2024 13:56:49 +0100 Subject: [PATCH 4/5] Added controller tests for register endpoint --- .../Controllers/Api/V1/SiteController.php | 11 +++-- routes/api.php | 2 +- tests/Feature/Api/SiteControllerTest.php | 42 +++++++++++++++---- 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/app/Http/Controllers/Api/V1/SiteController.php b/app/Http/Controllers/Api/V1/SiteController.php index 85d9bd8..bae991d 100644 --- a/app/Http/Controllers/Api/V1/SiteController.php +++ b/app/Http/Controllers/Api/V1/SiteController.php @@ -34,15 +34,18 @@ public function register(SiteRequest $request): JsonResponse $url = $request->string('url'); $key = $request->string('key'); - $connectionService = App::makeWith(Connection::class, ["baseUrl" => $url, "key" => $key]); + $connectionService = App::makeWith( + Connection::class, + ["baseUrl" => $url, "key" => $key] + ); // Do a health check try { $healthResponse = $connectionService->checkHealth(); - } catch (ServerException $e) { + } catch (ServerException|ClientException $e) { + return $this->error($e->getMessage(), 400); + } catch (\Exception $e) { return $this->error($e->getMessage(), 500); - } catch (ClientException|\Exception $e) { - return $this->error($e->getMessage()); } // If successful save site diff --git a/routes/api.php b/routes/api.php index decce7f..305c168 100644 --- a/routes/api.php +++ b/routes/api.php @@ -7,6 +7,6 @@ Route::controller(SiteController::class)->group(function () { Route::post('register', 'register'); Route::post('check', 'check'); - Route::delete('delete', 'delete'); + Route::post('delete', 'delete'); }); }); diff --git a/tests/Feature/Api/SiteControllerTest.php b/tests/Feature/Api/SiteControllerTest.php index 9e6e3ed..c404b52 100644 --- a/tests/Feature/Api/SiteControllerTest.php +++ b/tests/Feature/Api/SiteControllerTest.php @@ -2,11 +2,12 @@ namespace Tests\Api\Feature; +use App\Jobs\CheckSiteHealth; use App\Models\Site; use App\RemoteSite\Connection; use App\RemoteSite\Responses\HealthCheck; use Illuminate\Foundation\Testing\RefreshDatabase; -use Illuminate\Support\Facades\App; +use Illuminate\Support\Facades\Queue; use Tests\TestCase; class SiteControllerTest extends TestCase @@ -24,13 +25,21 @@ public function testRegisteringASiteWithoutUrlOrKeyFails(): void public function testRegisteringASiteWithUrlAndKeyCreatesRow(): void { - App::bind(Connection::class, fn() => $this->getConnectionMock(HealthCheck::from([ + Queue::fake(); + + $mock = $this->getMockBuilder(Connection::class) + ->disableOriginalConstructor() + ->getMock(); + + $mock->method('__call')->willReturn(HealthCheck::from([ "php_version" => "1.0.0", "db_type" => "mysqli", "db_version" => "1.0.0", "cms_version" => "1.0.0", "server_os" => "Joomla OS 1.0.0" - ]))); + ])); + + $this->app->bind(Connection::class, fn() => $mock); $response = $this->postJson( '/api/v1/register', @@ -38,18 +47,37 @@ public function testRegisteringASiteWithUrlAndKeyCreatesRow(): void ); $response->assertStatus(200); - $result = Site::where('url', 'https://www.joomla.org')->first(); + $row = Site::where('url', 'https://www.joomla.org')->first(); + + $this->assertEquals("https://www.joomla.org", $row->url); + $this->assertEquals("foobar123foobar123foobar123foobar123", $row->key); + $this->assertEquals("1.0.0", $row->php_version); + + Queue::assertPushed(CheckSiteHealth::class); + } + + public function testRegisteringASiteFailsWhenHealthCheckFails(): void + { + $mock = $this->getMockBuilder(Connection::class) + ->disableOriginalConstructor() + ->getMock(); + + $mock->method('__call')->willThrowException(new \Exception()); - $this->assertEquals([ + $this->app->bind(Connection::class, fn() => $mock); + + $response = $this->postJson( + '/api/v1/register', + ["url" => "https://www.joomla.org", "key" => "foobar123foobar123foobar123foobar123"] + ); - ], $result->toArray()); + $response->assertStatus(500); } protected function getConnectionMock(HealthCheck $response) { $mock = $this->getMockBuilder(Connection::class) ->disableOriginalConstructor() - ->onlyMethods(['__call']) ->getMock(); $mock->method('__call')->willReturn($response); From 12f65717d9a82cbfe61632c11e35c007a7ae4f1b Mon Sep 17 00:00:00 2001 From: David Jardin Date: Sun, 17 Nov 2024 14:00:02 +0100 Subject: [PATCH 5/5] cs fix --- app/RemoteSite/Connection.php | 2 +- ...024_11_17_115646_remove_patch_minor_columns_from_sites.php | 3 +-- tests/Feature/Api/SiteControllerTest.php | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/app/RemoteSite/Connection.php b/app/RemoteSite/Connection.php index e4fa96f..467c4da 100644 --- a/app/RemoteSite/Connection.php +++ b/app/RemoteSite/Connection.php @@ -70,7 +70,7 @@ public function performExtractionRequest(array $requestData): array return $this->decodeResponse($response, $request); } - protected function performWebserviceRequest( + public function performWebserviceRequest( HttpMethod $method, string $endpoint, ?array $requestData = null diff --git a/database/migrations/2024_11_17_115646_remove_patch_minor_columns_from_sites.php b/database/migrations/2024_11_17_115646_remove_patch_minor_columns_from_sites.php index 7529f8b..8ba27de 100644 --- a/database/migrations/2024_11_17_115646_remove_patch_minor_columns_from_sites.php +++ b/database/migrations/2024_11_17_115646_remove_patch_minor_columns_from_sites.php @@ -4,8 +4,7 @@ use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\Schema; -return new class extends Migration -{ +return new class () extends Migration { /** * Run the migrations. */ diff --git a/tests/Feature/Api/SiteControllerTest.php b/tests/Feature/Api/SiteControllerTest.php index c404b52..6470805 100644 --- a/tests/Feature/Api/SiteControllerTest.php +++ b/tests/Feature/Api/SiteControllerTest.php @@ -39,7 +39,7 @@ public function testRegisteringASiteWithUrlAndKeyCreatesRow(): void "server_os" => "Joomla OS 1.0.0" ])); - $this->app->bind(Connection::class, fn() => $mock); + $this->app->bind(Connection::class, fn () => $mock); $response = $this->postJson( '/api/v1/register', @@ -64,7 +64,7 @@ public function testRegisteringASiteFailsWhenHealthCheckFails(): void $mock->method('__call')->willThrowException(new \Exception()); - $this->app->bind(Connection::class, fn() => $mock); + $this->app->bind(Connection::class, fn () => $mock); $response = $this->postJson( '/api/v1/register',