Merge pull request #8 from swisnl/feature/feedback

Refactoring, based on feedback

CHANGELOG.md

@@ -1,6 +1,6 @@
 # Changelog
 
-All notable changes to `laravel-elastic` will be documented in this file.
+All notable changes to `laravel-lti-provider` will be documented in this file.
 
 Updates should follow the [Keep a CHANGELOG](https://keepachangelog.com/) principles.
 

CONTRIBUTING.md

@@ -2,7 +2,7 @@
 
 Contributions are **welcome** and will be fully **credited**.
 
-We accept contributions via Pull Requests on [Github](https://github.com/swisnl/laravel-elastic).
+We accept contributions via Pull Requests on [Github](https://github.com/swisnl/laravel-lti-provider).
 
 
 ## Pull Requests

README.md

@@ -7,50 +7,89 @@
 [![Total Downloads][ico-downloads]][link-downloads]
 [![Made by SWIS][ico-swis]][link-swis]
 
-This packages provides a bridge between the Celtic LTI Provider and Laravel models.
+This packages provides a bridge between the [Celtic LTI Package](https://github.com/celtic-project/LTI-PHP) and Laravel
+models, by implementing a DataConnector for the Celtic LTI Package.
 
-## Install & Setup
+## Install
 
 Via Composer
 
 ``` bash
 $ composer require swisnl/laravel-lti-provider
 ```
 
-Then run command to copy the required files into your project.
+Then run command to copy the required files (including the migrations) into your project.
 
 ```bash
-php artisan lti-service-provider:install
+php artisan lti-provider:install
 ```
 
 If you have Laravel package auto discovery disabled, add the service provider to your `config/app.php` file:
 
 ```php
 'providers' => [
     // ...
-    Swis\Laravel\Lti\Providers\LtiServiceProvider::class,
+    Swis\Laravel\Lti\Providers\LtiProviderServiceProvider::class,
 ];
 ```
 
-The package comes with a very basic client implementation, but allows for overriding this implementation. To do so,
-create a new class that implements the `Swis\Laravel\Lti\Contracts\LtiClient` interface and change the following to your
-`config/lti-provider.php` file:
+Finally run the migrations.
+
+```bash
+php artisan migrate
+```
+
+## Cron jobs
+
+The package comes with a command to clean up expired LTI nonces. To run this command, add the following to your
+`app/Console/Kernel.php` file:
 
 ```php
-        'lti-client' => 'REFERENCE TO YOUR NEW CLASS',
+    protected function schedule(Schedule $schedule)
+    {
+        // ...
+        $schedule->command('lti-provider:delete-expired-nonces')->daily();
+    }
 ```
 
-For inspiration on how to implement your own client, take a look at the `Swis\Laravel\Lti\Models\SimpleClient` class
-(the very basic implementation) or the `\Workbench\App\OverrideModels\Client` class (this is a more complex example used
-in the tests to check if it is possible to override the default implementation and if the package can handle clients 
-with UUIDs instead of numeric ids).
+## Usage
 
-After you have set up your client, you can run the migrations to create the required tables:
+Define a model that you use as an LTI environment. This model should implement the 
+`Swis\Laravel\Lti\Contracts\LtiEnvironment`. The packages scopes all other models to the current LTI environment. 
 
-```bash
-php artisan migrate
+Using this this environment, you can create a new `ModelDataConnector` instance. This instance can be used like the
+`DataConnector` from the Celtic LTI Package.
+
+```php
+    $environment = MyLtiEnvironment::find($id);
+    $dataConnector = new ModelDataConnector($environment);
+```
+
+## Customization
+
+This package allows overriding most of the models. We use this to override some models to use UUIDs instead of numeric
+ids, and to add some extra functionality (examples of this extra functionality: adding logging to `UserResult`; and
+using the same `Client` model for both LTI and [Laravel Passport](https://laravel.com/docs/10.x/passport)). 
+
+To override a model (except the client), create a new class that extends the original model and register the new model
+in the `config/lti-provider.php` file. For example, to override the `UserResult` model, create a new class that extends
+the `Swis\Laravel\Lti\Models\UserResult` class and change the following to your `config/lti-provider.php` file:
+
+```php
+        'models' => [
+            'user-result' => 'REFERENCE TO YOUR NEW CLASS',
+        ],
 ```
 
+Clients are a bit different, because you don't need to extend from an existing class. To override the client, create a
+new class that implements the `Swis\Laravel\Lti\Contracts\Client` interface and register the new model in the
+`config/lti-provider.php` file.
+
+For inspiration on how to implement your own client, take a look at the `Swis\Laravel\Lti\Models\SimpleClient` class
+(a very basic implementation) or the `\Workbench\App\OverrideModels\Client` class (this is a more complex example used
+in the tests to check if it is possible to override the default implementation and if the package can handle clients 
+with UUIDs instead of numeric ids).
+
 ## Change log
 
 Please see [CHANGELOG](CHANGELOG.md) for more information on what has changed recently.

composer.json

@@ -20,7 +20,6 @@
         "php": "^8.1",
         "laravel/framework": "^10.0",
         "spatie/laravel-package-tools": "^1.15",
-        "illuminate/contracts": "^10.0",
         "celtic/lti": "^5.0"
     },
     "require-dev": {
@@ -38,28 +37,24 @@
             "Swis\\Laravel\\LtiProvider\\": "src"
         }
     },
+    "autoload-dev": {
+        "psr-4": {
+            "Tests\\": "tests/",
+            "Workbench\\App\\": "workbench/app/",
+            "Workbench\\Database\\Factories\\": "workbench/database/factories/",
+            "Workbench\\Database\\Seeders\\": "workbench/database/seeders/"
+        }
+    },
     "scripts": {
         "post-autoload-dump": [
             "@clear",
-            "@prepare",
-            "@composer run prepare"
+            "@prepare"
         ],
         "clear": "@php vendor/bin/testbench package:purge-skeleton --ansi",
         "prepare": "@php vendor/bin/testbench package:discover --ansi",
-        "build": "@php vendor/bin/testbench workbench:build --ansi",
-        "start": [
-            "Composer\\Config::disableProcessTimeout",
-            "@composer run build",
-            "@php vendor/bin/testbench serve"
-        ],
         "analyse": "vendor/bin/phpstan analyse",
         "test": "vendor/bin/phpunit",
         "format": "vendor/bin/pint",
-        "serve": [
-            "Composer\\Config::disableProcessTimeout",
-            "@build",
-            "@php vendor/bin/testbench serve"
-        ],
         "lint": [
             "@php vendor/bin/pint",
             "@php vendor/bin/phpstan analyse"
@@ -68,7 +63,7 @@
     "extra": {
         "laravel": {
             "providers": [
-                "Swis\\Laravel\\LtiProvider\\LtiServiceProvider"
+                "Swis\\Laravel\\LtiProvider\\LtiProviderServiceProvider"
             ]
         }
     },
@@ -81,13 +76,5 @@
         }
     },
     "minimum-stability": "dev",
-    "prefer-stable": true,
-    "autoload-dev": {
-        "psr-4": {
-            "Tests\\": "tests/",
-            "Workbench\\App\\": "workbench/app/",
-            "Workbench\\Database\\Factories\\": "workbench/database/factories/",
-            "Workbench\\Database\\Seeders\\": "workbench/database/seeders/"
-        }
-    }
+    "prefer-stable": true
 }

config/lti-provider.php

@@ -2,11 +2,11 @@
 
 return [
     'class-names' => [
-        'lti-client' => \Swis\Laravel\LtiProvider\Models\SimpleClient::class,
-        'lti-context' => \Swis\Laravel\LtiProvider\Models\LtiContext::class,
-        'lti-resource-link' => \Swis\Laravel\LtiProvider\Models\LtiResourceLink::class,
-        'lti-nonce' => \Swis\Laravel\LtiProvider\Models\LtiNonce::class,
-        'lti-user-result' => \Swis\Laravel\LtiProvider\Models\LtiUserResult::class,
-        'lti-access-token' => \Swis\Laravel\LtiProvider\Models\LtiAccessToken::class,
+        'client' => \Swis\Laravel\LtiProvider\Models\SimpleClient::class,
+        'context' => \Swis\Laravel\LtiProvider\Models\Context::class,
+        'resource-link' => \Swis\Laravel\LtiProvider\Models\ResourceLink::class,
+        'nonce' => \Swis\Laravel\LtiProvider\Models\Nonce::class,
+        'user-result' => \Swis\Laravel\LtiProvider\Models\UserResult::class,
+        'access-token' => \Swis\Laravel\LtiProvider\Models\AccessToken::class,
     ],
 ];

database/migrations/2023_10_26_100000_add_client_table.php → database/migrations/2023_10_26_100000_add_lti_clients_table.php

@@ -11,7 +11,7 @@
      */
     public function up(): void
     {
-        Schema::create('clients', function (Blueprint $table) {
+        Schema::create('lti_clients', function (Blueprint $table) {
             $table->id();
 
             // Admin title
@@ -42,6 +42,6 @@ public function up(): void
      */
     public function down(): void
     {
-        Schema::drop('clients');
+        Schema::drop('lti_clients');
     }
 };

database/migrations/2023_10_26_200000_add_lti_tables.php

@@ -16,7 +16,7 @@ public function up(): void
 
             $table->morphs('lti_environment');
 
-            $table->foreignId('client_id')->constrained('clients')->cascadeOnDelete();
+            $table->foreignId('client_id')->constrained('lti_clients')->cascadeOnDelete();
 
             $table->string('nonce', 50);
             $table->dateTime('expires_at');
@@ -31,7 +31,7 @@ public function up(): void
 
             $table->morphs('lti_environment');
 
-            $table->foreignId('client_id')->constrained('clients')->cascadeOnDelete();
+            $table->foreignId('client_id')->constrained('lti_clients')->cascadeOnDelete();
 
             $table->string('access_token', 2000);
             $table->text('scopes');
@@ -45,7 +45,7 @@ public function up(): void
 
             $table->morphs('lti_environment');
 
-            $table->foreignId('client_id')->constrained('clients')->cascadeOnDelete();
+            $table->foreignId('client_id')->constrained('lti_clients')->cascadeOnDelete();
 
             $table->string('title')->nullable();
             $table->string('external_context_id', 255);
@@ -59,7 +59,7 @@ public function up(): void
 
             $table->morphs('lti_environment');
 
-            $table->foreignId('client_id')->constrained('clients')->cascadeOnDelete();
+            $table->foreignId('client_id')->constrained('lti_clients')->cascadeOnDelete();
 
             $table->foreignId('lti_context_id')->nullable()->constrained('lti_contexts')->cascadeOnDelete();
 

src/Commands/DeleteExpiredLtiNonces.php

@@ -3,17 +3,16 @@
 namespace Swis\Laravel\LtiProvider\Commands;
 
 use Illuminate\Console\Command;
-use Swis\Laravel\LtiProvider\Models\LtiNonce;
 
 class DeleteExpiredLtiNonces extends Command
 {
-    protected $signature = 'lti:delete-expired-nonces';
+    protected $signature = 'lti-provider:delete-expired-nonces';
 
     protected $description = 'Cleanup the expired LTI nonces from the database';
 
     public function handle(): int
     {
-        LtiNonce::deleteExpired();
+        config('lti-provider.class-names.nonce')::deleteExpired();
 
         return static::SUCCESS;
     }

src/LtiServiceProvider.php → src/LtiProviderServiceProvider.php

@@ -6,17 +6,21 @@
 
 use Spatie\LaravelPackageTools\Commands\InstallCommand;
 use Spatie\LaravelPackageTools\Package;
+use Spatie\LaravelPackageTools\PackageServiceProvider;
 use Swis\Laravel\LtiProvider\Commands\DeleteExpiredLtiNonces;
 
-class LtiServiceProvider extends \Spatie\LaravelPackageTools\PackageServiceProvider
+class LtiProviderServiceProvider extends PackageServiceProvider
 {
     public function configurePackage(Package $package): void
     {
         $package
-            ->name('lti-service-provider')
-            ->hasMigration('2023_10_26_100000_add_lti_tables')
-            ->publishesServiceProvider('LtiServiceProvider')
-            ->hasConfigFile('lti-provider')
+            ->name('laravel-lti-provider')
+            ->hasMigrations(
+                '2023_10_26_100000_add_client_table',
+                '2023_10_26_200000_add_lti_tables'
+            )
+            ->publishesServiceProvider('LtiProviderServiceProvider')
+            ->hasConfigFile()
             ->hasCommand(DeleteExpiredLtiNonces::class)
             ->hasInstallCommand(function (InstallCommand $command) {
                 $command