Skip to content
This repository was archived by the owner on Aug 22, 2023. It is now read-only.

Remove calls to Str::contains and Arr::get when not necessary #27

Closed
wants to merge 2 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
3 changes: 1 addition & 2 deletions src/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use function class_exists;
use Composer\InstalledVersions;
use Illuminate\Database\Connection as BaseConnection;
use Illuminate\Support\Arr;
use InvalidArgumentException;
use Jenssegers\Mongodb\Concerns\ManagesTransactions;
use MongoDB\Client;
Expand Down Expand Up @@ -48,7 +47,7 @@ public function __construct(array $config)
$dsn = $this->getDsn($config);

// You can pass options directly to the MongoDB constructor
$options = Arr::get($config, 'options', []);
$options = $config['options'] ?? [];
Copy link
Owner Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Noted that we're not using dot notation here (per Arr::get() docs), so these changes seem sensible.


// Create the connection
$this->connection = $this->createConnection($dsn, $config, $options);
Expand Down
6 changes: 3 additions & 3 deletions src/Eloquent/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public function getAttribute($key)
}

// Dot notation support.
if (Str::contains($key, '.') && Arr::has($this->attributes, $key)) {
if (str_contains($key, '.') && Arr::has($this->attributes, $key)) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

$key is a string.

Copy link
Collaborator

@jmikola jmikola Aug 21, 2023

Choose a reason for hiding this comment

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

Based on https://laravel.com/docs/10.x/helpers#method-str-contains, am I correct to assume that Str::contains() only differs in its handling of array parameters?

Is it feasible to use scalar type hints for the getAttribute() signature, or would that conflict with inheritance? Likewise for other str_contains() changes below.

return $this->getAttributeValue($key);
}

Expand All @@ -177,7 +177,7 @@ public function getAttribute($key)
protected function getAttributeFromArray($key)
{
// Support keys in dot notation.
if (Str::contains($key, '.')) {
if (str_contains($key, '.')) {
return Arr::get($this->attributes, $key);
}

Expand All @@ -195,7 +195,7 @@ public function setAttribute($key, $value)

$value = $builder->convertKey($value);
} // Support keys in dot notation.
elseif (Str::contains($key, '.')) {
elseif (str_contains($key, '.')) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where does this package require PHP 8+? I don't see it in composer.json so I reckon it's an indirect dependency. I think it'd make sense to explicitly require it if we're going to be utilizing PHP 8+ functionality directly.

Hhappy to defer to a subsequent PHPORM ticket.

Copy link
Owner Author

Choose a reason for hiding this comment

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

It requires laravel/framework: ^10.0 which requires PHP 8.1+.

// Store to a temporary key, then move data to the actual key
$uniqueKey = uniqid($key);
parent::setAttribute($uniqueKey, $value);
Expand Down
5 changes: 2 additions & 3 deletions src/Query/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
use Illuminate\Support\Arr;
use Illuminate\Support\Collection;
use Illuminate\Support\LazyCollection;
use Illuminate\Support\Str;
use Jenssegers\Mongodb\Connection;
use MongoDB\BSON\Binary;
use MongoDB\BSON\ObjectID;
Expand Down Expand Up @@ -617,7 +616,7 @@ public function insertGetId(array $values, $sequence = null)
public function update(array $values, array $options = [])
{
// Use $set as default operator.
if (! Str::startsWith(key($values), '$')) {
if (! str_starts_with(key($values), '$')) {
$values = ['$set' => $values];
}

Expand Down Expand Up @@ -951,7 +950,7 @@ protected function compileWheres(): array
}

// Convert id's.
if (isset($where['column']) && ($where['column'] == '_id' || Str::endsWith($where['column'], '._id'))) {
if (isset($where['column']) && ($where['column'] == '_id' || str_ends_with($where['column'], '._id'))) {
// Multiple values.
if (isset($where['values'])) {
foreach ($where['values'] as &$value) {
Expand Down
5 changes: 2 additions & 3 deletions src/Queue/MongoConnector.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

use Illuminate\Database\ConnectionResolverInterface;
use Illuminate\Queue\Connectors\ConnectorInterface;
use Illuminate\Support\Arr;

class MongoConnector implements ConnectorInterface
{
Expand Down Expand Up @@ -34,10 +33,10 @@ public function __construct(ConnectionResolverInterface $connections)
public function connect(array $config)
{
return new MongoQueue(
$this->connections->connection(Arr::get($config, 'connection')),
$this->connections->connection($config['connection']),
$config['table'],
$config['queue'],
Arr::get($config, 'expire', 60)
$config['expire'] ?? 60
);
}
}