Skip to content
This repository has been archived by the owner on Dec 3, 2024. It is now read-only.

Added SerializationHelper that handles PostgreSql serialization correctly & replaced all serialization function calls #63

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/Casts/CallbackCollectionCast.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use InvalidArgumentException;
use Sammyjo20\LaravelHaystack\Data\CallbackCollection;
use Sammyjo20\LaravelHaystack\Helpers\SerializationHelper;
use Illuminate\Contracts\Database\Eloquent\CastsAttributes;

class CallbackCollectionCast implements CastsAttributes
Expand All @@ -21,7 +22,7 @@ class CallbackCollectionCast implements CastsAttributes
*/
public function get($model, string $key, $value, array $attributes)
{
return isset($value) ? unserialize($value, ['allowed_classes' => true]) : null;
return isset($value) ? SerializationHelper::unserialize($value, ['allowed_classes' => true]) : null;
}

/**
Expand All @@ -43,6 +44,6 @@ public function set($model, string $key, $value, array $attributes)
throw new InvalidArgumentException(sprintf('Value provided must be an instance of %s.', CallbackCollection::class));
}

return serialize($value);
return SerializationHelper::serialize($value);
}
}
5 changes: 3 additions & 2 deletions src/Casts/MiddlewareCollectionCast.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use InvalidArgumentException;
use Sammyjo20\LaravelHaystack\Data\MiddlewareCollection;
use Sammyjo20\LaravelHaystack\Helpers\SerializationHelper;
use Illuminate\Contracts\Database\Eloquent\CastsAttributes;

class MiddlewareCollectionCast implements CastsAttributes
Expand All @@ -21,7 +22,7 @@ class MiddlewareCollectionCast implements CastsAttributes
*/
public function get($model, string $key, $value, array $attributes)
{
return isset($value) ? unserialize($value, ['allowed_classes' => true]) : null;
return isset($value) ? SerializationHelper::unserialize($value, ['allowed_classes' => true]) : null;
}

/**
Expand All @@ -43,6 +44,6 @@ public function set($model, string $key, $value, array $attributes)
throw new InvalidArgumentException(sprintf('Value provided must be an instance of %s.', MiddlewareCollection::class));
}

return serialize($value);
return SerializationHelper::serialize($value);
}
}
5 changes: 3 additions & 2 deletions src/Casts/SerializeClosure.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use InvalidArgumentException;
use Laravel\SerializableClosure\SerializableClosure;
use Sammyjo20\LaravelHaystack\Helpers\ClosureHelper;
use Sammyjo20\LaravelHaystack\Helpers\SerializationHelper;
use Illuminate\Contracts\Database\Eloquent\CastsAttributes;

class SerializeClosure implements CastsAttributes
Expand All @@ -23,7 +24,7 @@ class SerializeClosure implements CastsAttributes
*/
public function get($model, string $key, $value, array $attributes): ?Closure
{
return isset($value) ? unserialize($value, ['allowed_classes' => true])->getClosure() : null;
return isset($value) ? SerializationHelper::unserialize($value, ['allowed_classes' => true])->getClosure() : null;
}

/**
Expand All @@ -49,6 +50,6 @@ public function set($model, string $key, $value, array $attributes): ?string

$closure = ClosureHelper::fromCallable($value);

return serialize(new SerializableClosure($closure));
return SerializationHelper::serialize(new SerializableClosure($closure));
}
}
5 changes: 3 additions & 2 deletions src/Casts/Serialized.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Sammyjo20\LaravelHaystack\Casts;

use Illuminate\Contracts\Database\Eloquent\CastsAttributes;
use Sammyjo20\LaravelHaystack\Helpers\SerializationHelper;

class Serialized implements CastsAttributes
{
Expand All @@ -19,7 +20,7 @@ class Serialized implements CastsAttributes
*/
public function get($model, string $key, $value, array $attributes)
{
return isset($value) ? unserialize($value, ['allowed_classes' => true]) : null;
return isset($value) ? SerializationHelper::unserialize($value, ['allowed_classes' => true]) : null;
}

/**
Expand All @@ -37,6 +38,6 @@ public function set($model, string $key, $value, array $attributes)
return null;
}

return serialize($value);
return SerializationHelper::serialize($value);
}
}
5 changes: 3 additions & 2 deletions src/Casts/SerializedModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Illuminate\Database\Eloquent\Model;
use Illuminate\Contracts\Database\Eloquent\CastsAttributes;
use Sammyjo20\LaravelHaystack\Data\SerializedModel as SerializedModelData;
use Sammyjo20\LaravelHaystack\Helpers\SerializationHelper;

class SerializedModel implements CastsAttributes
{
Expand All @@ -22,7 +23,7 @@ class SerializedModel implements CastsAttributes
*/
public function get($model, string $key, $value, array $attributes)
{
return isset($value) ? unserialize($value, ['allowed_classes' => true])->model : null;
return isset($value) ? SerializationHelper::unserialize($value, ['allowed_classes' => true])->model : null;
}

/**
Expand All @@ -44,6 +45,6 @@ public function set($model, string $key, $value, array $attributes)
throw new InvalidArgumentException('The provided value must be a model.');
}

return serialize(new SerializedModelData($value));
return SerializationHelper::serialize(new SerializedModelData($value));
}
}
43 changes: 43 additions & 0 deletions src/Helpers/SerializationHelper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php

declare(strict_types=1);

namespace Sammyjo20\LaravelHaystack\Helpers;

use Illuminate\Database\PostgresConnection;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Str;

class SerializationHelper
{
/**
* Serialize the given value.
*
* @param mixed $value
* @return string
*/
public static function serialize(mixed $value) : string
{
$serialized = serialize($value);

return DB::connection() instanceof PostgresConnection
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be using the connection that has been specified on the model instead of the global DB connection? From Laravel's PR:

https://github.com/laravel/framework/pull/36081/files#diff-c46d3e14c232058da1ea59d469b8abc9866c1ced2104eece667c4077b54a5e22R304-R306

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think you are right. This solution only covers the default scenario. I will look into it and make fixes accordingly.

? base64_encode($serialized)
: $serialized;
}

/**
* Unserialize the given value.
*
* @param string $serialized
* @param array $options
* @return mixed
*/
public static function unserialize(string $serialized, array $options = []) : mixed
{
if (DB::connection() instanceof PostgresConnection && ! Str::contains($serialized, [':', ';'])) {
$serialized = base64_decode($serialized);
}

return unserialize($serialized, $options);
}
}
3 changes: 2 additions & 1 deletion src/JobEventListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Illuminate\Queue\Events\JobProcessed;
use Sammyjo20\LaravelHaystack\Models\Haystack;
use Sammyjo20\LaravelHaystack\Contracts\StackableJob;
use Sammyjo20\LaravelHaystack\Helpers\SerializationHelper;

class JobEventListener
{
Expand Down Expand Up @@ -163,7 +164,7 @@ private function unserializeJobFromPayload(array $payload): ?object
return null;
}

return unserialize($payload['data']['command'], ['allowed_classes' => true]);
return SerializationHelper::unserialize($payload['data']['command'], ['allowed_classes' => true]);
}

/**
Expand Down