From 689956a266ca0ec9239d87e302a34a0fb5e84908 Mon Sep 17 00:00:00 2001
From: Shashwat Mishra <11258035+secrethash@users.noreply.github.com>
Date: Wed, 11 Dec 2024 10:29:43 +0530
Subject: [PATCH] fix: error handling & reporting, bucket naming & refactor
 code

---
 src/Bucket.php                  | 52 +++++++++++++++++++++------------
 src/Jobs/CreateTenantBucket.php | 18 ++----------
 src/Jobs/DeleteTenantBucket.php | 15 ++--------
 3 files changed, 37 insertions(+), 48 deletions(-)

diff --git a/src/Bucket.php b/src/Bucket.php
index 91dbf8f..0b54558 100644
--- a/src/Bucket.php
+++ b/src/Bucket.php
@@ -39,12 +39,12 @@ class Bucket
     /**
      * @var string|null Name of the Created Bucket
     */
-    protected string|null $createdBucketName;
+    protected string|null $bucketName;
 
     /**
      * @var \Aws\Exception\AwsException|null Exception Error Bag
     */
-    protected AwsException|null $e;
+    protected AwsException|null $e = null;
 
     public function __construct(
         protected Tenant $tenant
@@ -66,7 +66,6 @@ private function setupCredentials()
     /**
      * Create Tenant Specific Bucket
      *
-     * @access public
      * @return self $this
      */
     public function createTenantBucket(): self
@@ -79,7 +78,6 @@ public function createTenantBucket(): self
     /**
      * Delete Tenant Specific Bucket
      *
-     * @access public
      * @return self $this
      */
     public function deleteTenantBucket(): self
@@ -94,7 +92,6 @@ public function deleteTenantBucket(): self
      *
      * @param string $name Name of the S3 Bucket
      * @param \Aws\Credentials\Credentials $credentials AWS Credentials Object
-     * @access public
      * @return self $this
      */
     public function createBucket(string $name, Credentials $credentials): self
@@ -108,21 +105,22 @@ public function createBucket(string $name, Credentials $credentials): self
             "version" => $this->version,
             "use_path_style_endpoint" => $this->pathStyle,
         ];
+        $this->bucketName = $this->formatBucketName($name);
 
         $client = new S3Client($params);
 
         try {
-            $exec = $client->createBucket([
+            $client->createBucket([
                 'Bucket' => $name,
             ]);
-            $this->createdBucketName = $name;
 
             // Update Tenant
             $this->tenant->tenant_bucket = $name;
             $this->tenant->save();
         } catch (AwsException $e) {
             $this->e = $e;
-            Log::error($this->getErrorMessage());
+            throw_if(config('app.debug', false), $e);
+            Log::critical($this->getErrorMessage());
         }
 
         event(new CreatedBucket($this->tenant));
@@ -135,12 +133,11 @@ public function createBucket(string $name, Credentials $credentials): self
      *
      * @param string $name Name of the S3 Bucket
      * @param \Aws\Credentials\Credentials $credentials AWS Credentials Object
-     * @access public
      * @return self $this
      */
     public function deleteBucket(string $name, Credentials $credentials): self
     {
-        event(new DeletingBucket($this->tenant));
+        event(new DeletingBucket(tenant: $this->tenant));
 
         $params = [
             "credentials" => $credentials,
@@ -153,15 +150,13 @@ public function deleteBucket(string $name, Credentials $credentials): self
         $client = new S3Client($params);
 
         try {
-            $exec = $client->deleteBucket([
+            $client->deleteBucket([
                 'Bucket' => $name,
             ]);
         } catch (AwsException $e) {
             $this->e = $e;
-            if (config('tenant-buckets.errors.throw', true)) {
-                throw $e;
-            }
-            Log::error($this->getErrorMessage());
+            throw_if(config('app.debug'), $e);
+            Log::critical($this->getErrorMessage());
         }
 
         event(new DeletedBucket($this->tenant));
@@ -176,7 +171,17 @@ public function deleteBucket(string $name, Credentials $credentials): self
      */
     public function getBucketName(): string|null
     {
-        return $this->createdBucketName;
+        return $this->bucketName;
+    }
+
+    /**
+     * Format Bucket Name according to AWS S3 Bucket Naming Rules
+     * @see https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html
+     * @param string $name
+     */
+    protected function formatBucketName(string $name): string
+    {
+        return str(preg_replace('/[^a-zA-Z0-9]/', '', $name))->lower()->toString();
     }
 
     /**
@@ -186,9 +191,18 @@ public function getBucketName(): string|null
      */
     public function getErrorMessage(): string|null
     {
-        return ($this->e) ?
-        "Error: " . $this->e->getAwsErrorMessage() :
-        null;
+        if ($this->e) {
+            $data = [
+                'tenant' => $this->tenant->id,
+                'bucket' => $this->bucketName,
+                'error_code' => $this->e?->getAwsErrorCode(),
+                'error_type' => $this->e?->getAwsErrorType(),
+                'error_message' => $this->e?->getAwsErrorMessage(),
+                'response' => $this->e?->getResponse(),
+            ];
+            return "[tenant-buckets] Error: (Tenant ID: {$this->tenant->id}) {$this->e->getAwsErrorMessage()} ". json_encode($data);
+        }
+        return null;
     }
 
     /**
diff --git a/src/Jobs/CreateTenantBucket.php b/src/Jobs/CreateTenantBucket.php
index f6c68e1..2c1959e 100644
--- a/src/Jobs/CreateTenantBucket.php
+++ b/src/Jobs/CreateTenantBucket.php
@@ -17,12 +17,6 @@ class CreateTenantBucket implements ShouldQueue
     use Queueable;
     use SerializesModels;
 
-    /**
-     * Current Tenant
-     * @access protected
-     */
-    protected $tenant;
-
     /**
      * The number of times the job may be attempted.
      *
@@ -42,11 +36,7 @@ class CreateTenantBucket implements ShouldQueue
      *
      * @return void
      */
-    public function __construct(Tenant $tenant)
-    {
-        //
-        $this->tenant = $tenant;
-    }
+    public function __construct(protected Tenant $tenant) {}
 
     /**
      * Execute the job.
@@ -55,11 +45,7 @@ public function __construct(Tenant $tenant)
      */
     public function handle()
     {
-        $bucket = new Bucket($this->tenant);
-        $create = $bucket->createTenantBucket();
-
-        // $this->tenant->tenant_bucket = $create->getBucketName();
-        // $this->tenant->save();
+        (new Bucket($this->tenant))->createTenantBucket();
     }
 
     /**
diff --git a/src/Jobs/DeleteTenantBucket.php b/src/Jobs/DeleteTenantBucket.php
index 7b3a039..56958b2 100644
--- a/src/Jobs/DeleteTenantBucket.php
+++ b/src/Jobs/DeleteTenantBucket.php
@@ -17,12 +17,6 @@ class DeleteTenantBucket implements ShouldQueue
     use Queueable;
     use SerializesModels;
 
-    /**
-     * Current Tenant
-     * @access protected
-     */
-    protected $tenant;
-
     /**
      * The number of times the job may be attempted.
      *
@@ -42,11 +36,7 @@ class DeleteTenantBucket implements ShouldQueue
      *
      * @return void
      */
-    public function __construct(Tenant $tenant)
-    {
-        //
-        $this->tenant = $tenant;
-    }
+    public function __construct(protected Tenant $tenant) {}
 
     /**
      * Execute the job.
@@ -55,8 +45,7 @@ public function __construct(Tenant $tenant)
      */
     public function handle()
     {
-        $bucket = new Bucket($this->tenant);
-        $delete = $bucket->deleteTenantBucket();
+        (new Bucket($this->tenant))->deleteTenantBucket();
     }
 
     /**