diff --git a/README.md b/README.md index 6a675257..49481a95 100644 --- a/README.md +++ b/README.md @@ -1179,6 +1179,7 @@ class Message extends Model ### Authentication +// Could be simplified if we get rid of the custom PasswordResetServiceProvider If you want to use Laravel's native Auth functionality, register this included service provider: ```php diff --git a/composer.json b/composer.json index fbc082a8..e02cc273 100644 --- a/composer.json +++ b/composer.json @@ -19,6 +19,7 @@ ], "license": "MIT", "require": { + "ext-mongodb": "*", "illuminate/support": "^10.0", "illuminate/container": "^10.0", "illuminate/database": "^10.0", diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 120898c0..a7cf8aca 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -38,7 +38,7 @@ - + diff --git a/src/Auth/DatabaseTokenRepository.php b/src/Auth/DatabaseTokenRepository.php index cf0f89ea..0d782998 100644 --- a/src/Auth/DatabaseTokenRepository.php +++ b/src/Auth/DatabaseTokenRepository.php @@ -18,6 +18,7 @@ protected function getPayload($email, $token) return [ 'email' => $email, 'token' => $this->hasher->make($token), + // why is it necessary ? vs "new Carbon" in parent class 'created_at' => new UTCDateTime(Date::now()->format('Uv')), ]; } @@ -27,6 +28,7 @@ protected function getPayload($email, $token) */ protected function tokenExpired($createdAt) { + // Could be removed if Carbon was natively suported? $createdAt = $this->convertDateTime($createdAt); return parent::tokenExpired($createdAt); diff --git a/src/Connection.php b/src/Connection.php index 3a3d235e..9989d9d9 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -112,7 +112,8 @@ public function getSchemaBuilder() * * @return Database */ - public function getMongoDB() + // Should be named getDatabase ? + public function getDatabase() { return $this->db; } @@ -132,7 +133,7 @@ public function getMongoClient() */ public function getDatabaseName() { - return $this->getMongoDB()->getDatabaseName(); + return $this->getDatabase()->getDatabaseName(); } /** @@ -147,6 +148,7 @@ public function getDatabaseName() protected function getDefaultDatabaseName(string $dsn, array $config): string { if (empty($config['database'])) { + // Use parse_url() to get the database name from dsn if (preg_match('/^mongodb(?:[+]srv)?:\\/\\/.+\\/([^?&]+)/s', $dsn, $matches)) { $config['database'] = $matches[1]; } else { @@ -195,6 +197,8 @@ protected function createConnection($dsn, array $config, array $options): Client */ public function disconnect() { + // No need to explicitly disconnect? + // What appens if a reference to the connection is kept somewhere? unset($this->connection); } @@ -260,6 +264,9 @@ protected function getDsn(array $config): string /** * @inheritdoc */ + // Should be @internal because the parent method is protected + // and this method is use in Collection::__call + // Otherwise reimplement the method in Collection public function getElapsedTime($start) { return parent::getElapsedTime($start); @@ -302,6 +309,8 @@ protected function getDefaultSchemaGrammar() * * @param \MongoDB\Database $db */ + // 1 more reason to rename getMongoDB to getDatabase, for consistency with the setter + // this is not used in the codebase, use-case may be challenged as it allows to by-pass the connection public function setDatabase(\MongoDB\Database $db) { $this->db = $db; diff --git a/src/Eloquent/EmbedsRelations.php b/src/Eloquent/EmbedsRelations.php index 9e5f77d9..207415ed 100644 --- a/src/Eloquent/EmbedsRelations.php +++ b/src/Eloquent/EmbedsRelations.php @@ -6,6 +6,7 @@ use Jenssegers\Mongodb\Relations\EmbedsMany; use Jenssegers\Mongodb\Relations\EmbedsOne; +// Documentation ? trait EmbedsRelations { /** @@ -23,7 +24,8 @@ protected function embedsMany($related, $localKey = null, $foreignKey = null, $r // the calling method's name and use that as the relationship name as most // of the time this will be what we desire to use for the relationships. if ($relation === null) { - [, $caller] = debug_backtrace(false); + // Black magic. The call may be optimized to get only the method name. + [, $caller] = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2); $relation = $caller['function']; } @@ -58,7 +60,8 @@ protected function embedsOne($related, $localKey = null, $foreignKey = null, $re // the calling method's name and use that as the relationship name as most // of the time this will be what we desire to use for the relationships. if ($relation === null) { - [, $caller] = debug_backtrace(false); + // Black magic. The call may be optimized to get only the method name. + [, $caller] = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2); $relation = $caller['function']; } diff --git a/src/Eloquent/HybridRelations.php b/src/Eloquent/HybridRelations.php index 0818ca3e..4a5be896 100644 --- a/src/Eloquent/HybridRelations.php +++ b/src/Eloquent/HybridRelations.php @@ -12,6 +12,7 @@ use Jenssegers\Mongodb\Relations\MorphMany; use Jenssegers\Mongodb\Relations\MorphTo; +// Documentation ? trait HybridRelations { /** @@ -134,7 +135,7 @@ public function belongsTo($related, $foreignKey = null, $otherKey = null, $relat // the calling method's name and use that as the relationship name as most // of the time this will be what we desire to use for the relationships. if ($relation === null) { - [$current, $caller] = debug_backtrace(false, 2); + [, $caller] = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2); $relation = $caller['function']; } @@ -178,7 +179,7 @@ public function morphTo($name = null, $type = null, $id = null, $ownerKey = null // since that is most likely the name of the polymorphic interface. We can // use that to get both the class and foreign key that will be utilized. if ($name === null) { - [$current, $caller] = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2); + [, $caller] = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2); $name = $caller['function']; } diff --git a/src/Eloquent/Model.php b/src/Eloquent/Model.php index 2d938b74..89591a46 100644 --- a/src/Eloquent/Model.php +++ b/src/Eloquent/Model.php @@ -2,6 +2,7 @@ namespace Jenssegers\Mongodb\Eloquent; +use Carbon\CarbonInterface; use function array_key_exists; use DateTimeInterface; use function explode; @@ -86,6 +87,8 @@ public function getQualifiedKeyName() /** * @inheritdoc + * + * @return UTCDateTime */ public function fromDateTime($value) { @@ -104,6 +107,8 @@ public function fromDateTime($value) /** * @inheritdoc + * + * @param UTCDateTime|CarbonInterface|DateTimeInterface|int|string $value */ protected function asDateTime($value) { @@ -151,7 +156,7 @@ public function getTable() public function getAttribute($key) { if (! $key) { - return; + return null; } // Dot notation support. diff --git a/src/Query/Builder.php b/src/Query/Builder.php index 06641273..fdf024dc 100644 --- a/src/Query/Builder.php +++ b/src/Query/Builder.php @@ -69,6 +69,9 @@ class Builder extends BaseBuilder * * @var array */ + // can we get this list automatically from mongodb/mongodb? + // at least "in" is missing but works fine + // @link https://www.mongodb.com/docs/manual/reference/operator/query/ public $operators = [ '=', '<', @@ -432,6 +435,10 @@ public function getFresh($columns = [], $returnLazy = false) * * @return string */ + // never used. What's the purpose? + // Should be deprecated? + // Maybe used in conjunction with cache() method? + // @link https://github.com/jenssegers/laravel-mongodb/commit/3f1be668ee2e60ac630cd25d1230c3a449b0f9d6 public function generateCacheKey() { $key = [ @@ -454,7 +461,9 @@ public function generateCacheKey() */ public function aggregate($function, $columns = []) { - $this->aggregate = compact('function', 'columns'); + // less magic https://stackoverflow.com/a/35589135 + // "compact" is common in laravel https://github.com/search?q=repo%3Alaravel%2Fframework%20compact&type=code + $this->aggregate = ['function' => $function, 'columns' => $columns]; $previousColumns = $this->columns; @@ -513,6 +522,8 @@ public function orderBy($column, $direction = 'asc') } if ($column == 'natural') { + // A hint should be added automatically? + // https://www.mongodb.com/docs/manual/reference/operator/meta/natural/ $this->orders['$natural'] = $direction; } else { $this->orders[(string) $column] = $direction; @@ -544,9 +555,7 @@ public function whereAll($column, array $values, $boolean = 'and', $not = false) */ public function whereBetween($column, iterable $values, $boolean = 'and', $not = false) { - $type = 'between'; - - $this->wheres[] = compact('column', 'type', 'boolean', 'values', 'not'); + $this->wheres[] = ['column' => $column, 'type' => 'between', 'boolean' => $boolean, 'values' => $values, 'not' => $not]; return $this; } @@ -615,7 +624,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]; } @@ -679,9 +688,10 @@ public function pluck($column, $key = null) $results = $this->get($key === null ? [$column] : [$column, $key]); // Convert ObjectID's to strings - if ($key == '_id') { - $results = $results->map(function ($item) { - $item['_id'] = (string) $item['_id']; + // We can have ObjectId in other fields? for relations? + if ($key !== null) { + $results = $results->map(function ($item) use ($key) { + $item[$key] = (string)$item[$key]; return $item; }); @@ -721,6 +731,10 @@ public function delete($id = null) */ public function from($collection, $as = null) { + if ($as !== null) { + throw new \InvalidArgumentException('Aliasing collection name is not supported by MongoDB.'); + } + if ($collection) { $this->collection = $this->connection->getCollection($collection); } @@ -744,9 +758,10 @@ public function truncate(): bool * * @param string $column * @param string $key + * Incorrect return type pluck returns a Collection * @return array * - * @deprecated + * @deprecated to remove */ public function lists($column, $key = null) { @@ -764,18 +779,16 @@ public function raw($expression = null) } // Create an expression for the given value - if ($expression !== null) { - return new Expression($expression); - } + return new Expression($expression); // Quick access to the mongodb collection - return $this->collection; + // strange feature, the return type should be Expression. } /** * Append one or more values to an array. * - * @param mixed $column + * @param array|string $column * @param mixed $value * @param bool $unique * @return int @@ -786,9 +799,15 @@ public function push($column, $value = null, $unique = false) $operator = $unique ? '$addToSet' : '$push'; // Check if we are pushing multiple values. - $batch = (is_array($value) && array_keys($value) === range(0, count($value) - 1)); + $batch = (is_array($value) && array_is_list($value)); if (is_array($column)) { + // What is it for? $value is ignored + // https://www.mongodb.com/docs/manual/reference/operator/update/push/ + if ($value !== null) { + throw new \InvalidArgumentException('You can only pass $value argument to push() when using an array for the column.'); + } + $query = [$operator => $column]; } elseif ($batch) { $query = [$operator => [$column => ['$each' => $value]]]; @@ -802,7 +821,7 @@ public function push($column, $value = null, $unique = false) /** * Remove one or more values from an array. * - * @param mixed $column + * @param array|string $column * @param mixed $value * @return int */ @@ -815,6 +834,10 @@ public function pull($column, $value = null) $operator = $batch ? '$pullAll' : '$pull'; if (is_array($column)) { + if ($value !== null) { + throw new \InvalidArgumentException('You can only pass $value argument to push() when using an string for the column.'); + } + $query = [$operator => $column]; } else { $query = [$operator => [$column => $value]]; @@ -826,15 +849,12 @@ public function pull($column, $value = null) /** * Remove one or more fields. * - * @param mixed $columns + * @param array|string $columns * @return int */ public function drop($columns) { - if (! is_array($columns)) { - $columns = [$columns]; - } - + $columns = (array) $columns; $fields = []; foreach ($columns as $column) { @@ -871,7 +891,7 @@ protected function performUpdate($query, array $options = []) $options = $this->inheritConnectionOptions($options); $wheres = $this->compileWheres(); - $result = $this->collection->UpdateMany($wheres, $query, $options); + $result = $this->collection->updateMany($wheres, $query, $options); if (1 == (int) $result->isAcknowledged()) { return $result->getModifiedCount() ? $result->getModifiedCount() : $result->getUpsertedCount(); } @@ -906,12 +926,9 @@ public function where($column, $operator = null, $value = null, $boolean = 'and' $params = func_get_args(); // Remove the leading $ from operators. - if (func_num_args() == 3) { - $operator = &$params[1]; - - if (Str::startsWith($operator, '$')) { - $operator = substr($operator, 1); - } + // I need to understand this hack, why only when $boolean is not provided? + if (func_num_args() === 3) { + $params[1] = ltrim($params[1], '$'); } return parent::where(...$params); @@ -933,9 +950,10 @@ protected function compileWheres(): array foreach ($wheres as $i => &$where) { // Make sure the operator is in lowercase. if (isset($where['operator'])) { - $where['operator'] = strtolower($where['operator']); + $where[' operator'] = strtolower($where['operator']); // Operator conversions + // There is $operatorConvertion that contains other conversions $convert = [ 'regexp' => 'regex', 'elemmatch' => 'elemMatch', @@ -1086,6 +1104,7 @@ protected function compileWhereBasic(array $where): array * @param array $where * @return mixed */ + // what calls this methods? protected function compileWhereNested(array $where): mixed { extract($where); diff --git a/src/Query/Grammar.php b/src/Query/Grammar.php index 3b1d33a8..11c901d8 100644 --- a/src/Query/Grammar.php +++ b/src/Query/Grammar.php @@ -4,6 +4,7 @@ use Illuminate\Database\Query\Grammars\Grammar as BaseGrammar; +// Why override this class? Nothing is different from the base class. class Grammar extends BaseGrammar { } diff --git a/src/Query/Processor.php b/src/Query/Processor.php index 7155c741..57e60452 100644 --- a/src/Query/Processor.php +++ b/src/Query/Processor.php @@ -4,6 +4,7 @@ use Illuminate\Database\Query\Processors\Processor as BaseProcessor; +// Why override this class? Nothing is different from the base class. class Processor extends BaseProcessor { } diff --git a/src/Schema/Blueprint.php b/src/Schema/Blueprint.php index 7e7fb678..46d7fc81 100644 --- a/src/Schema/Blueprint.php +++ b/src/Schema/Blueprint.php @@ -253,7 +253,7 @@ public function create($options = []) { $collection = $this->collection->getCollectionName(); - $db = $this->connection->getMongoDB(); + $db = $this->connection->getDatabase(); // Ensure the collection is created. $db->createCollection($collection, $options); diff --git a/src/Schema/Builder.php b/src/Schema/Builder.php index e681b3e4..c8e129e6 100644 --- a/src/Schema/Builder.php +++ b/src/Schema/Builder.php @@ -30,7 +30,7 @@ public function hasColumns($table, array $columns) */ public function hasCollection($name) { - $db = $this->connection->getMongoDB(); + $db = $this->connection->getDatabase(); $collections = iterator_to_array($db->listCollections([ 'filter' => [ @@ -135,7 +135,7 @@ protected function createBlueprint($collection, Closure $callback = null) */ public function getCollection($name) { - $db = $this->connection->getMongoDB(); + $db = $this->connection->getDatabase(); $collections = iterator_to_array($db->listCollections([ 'filter' => [ @@ -154,7 +154,7 @@ public function getCollection($name) protected function getAllCollections() { $collections = []; - foreach ($this->connection->getMongoDB()->listCollections() as $collection) { + foreach ($this->connection->getDatabase()->listCollections() as $collection) { $collections[] = $collection->getName(); } diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php index fd8003be..94d16535 100644 --- a/tests/ConnectionTest.php +++ b/tests/ConnectionTest.php @@ -37,7 +37,7 @@ public function testReconnect() public function testDb() { $connection = DB::connection('mongodb'); - $this->assertInstanceOf(Database::class, $connection->getMongoDB()); + $this->assertInstanceOf(Database::class, $connection->getDatabase()); $this->assertInstanceOf(Client::class, $connection->getMongoClient()); } @@ -127,7 +127,7 @@ public function testConnectionConfig(string $expectedUri, string $expectedDataba $client = $connection->getMongoClient(); $this->assertSame($expectedUri, (string) $client); - $this->assertSame($expectedDatabaseName, $connection->getMongoDB()->getDatabaseName()); + $this->assertSame($expectedDatabaseName, $connection->getDatabase()->getDatabaseName()); } public function testConnectionWithoutConfiguredDatabase(): void