Skip to content

PHPLIB-1128: Move generate_index_name() to private method within IndexInput #1092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 1, 2023
Merged

PHPLIB-1128: Move generate_index_name() to private method within IndexInput #1092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c4569a1
Move generate_index_name() to private method within IndexInput
GromNaN May 31, 2023
1e093cf
Update psalm baseline
GromNaN Jun 1, 2023
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
9 changes: 6 additions & 3 deletions psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,9 +274,13 @@
<MixedArgument occurrences="1">
<code>$fieldName</code>
</MixedArgument>
<MixedAssignment occurrences="1">
<MixedAssignment occurrences="2">
<code>$fieldName</code>
<code>$type</code>
</MixedAssignment>
<MixedOperand occurrences="1">
<code>$type</code>
</MixedOperand>
<MixedReturnStatement occurrences="1">
<code>$this-&gt;index['name']</code>
</MixedReturnStatement>
Expand Down Expand Up @@ -774,10 +778,9 @@
<code>$typeMap['fieldPaths'][$fieldPath]</code>
<code>$typeMap['fieldPaths'][substr($fieldPath, 0, -2)]</code>
</MixedArrayAssignment>
<MixedAssignment occurrences="6">
<MixedAssignment occurrences="5">
<code>$element[$key]</code>
<code>$type</code>
<code>$type</code>
<code>$typeMap['fieldPaths'][$fieldPath . '.' . $existingFieldPath]</code>
<code>$typeMap['fieldPaths'][$fieldPath]</code>
<code>$value</code>
Expand Down
24 changes: 22 additions & 2 deletions src/Model/IndexInput.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
use function is_int;
use function is_object;
use function is_string;
use function MongoDB\generate_index_name;
use function MongoDB\document_to_array;
use function sprintf;

/**
Expand Down Expand Up @@ -64,7 +64,7 @@ public function __construct(array $index)
}

if (! isset($index['name'])) {
$index['name'] = generate_index_name($index['key']);
$index['name'] = $this->generateIndexName($index['key']);
}

if (! is_string($index['name'])) {
Expand Down Expand Up @@ -92,4 +92,24 @@ public function bsonSerialize(): array
{
return $this->index;
}

/**
* Generate an index name from a key specification.
*
* @param array|object $document Document containing fields mapped to values,
* which denote order or an index type
* @throws InvalidArgumentException if $document is not an array or object
*/
private function generateIndexName($document): string
{
$document = document_to_array($document);

$name = '';

foreach ($document as $field => $type) {
$name .= ($name !== '' ? '_' : '') . $field . '_' . $type;
}

return $name;
}
}
21 changes: 0 additions & 21 deletions src/functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,27 +140,6 @@ function document_to_array($document): array
return $document;
}

/**
* Generate an index name from a key specification.
*
* @internal
* @param array|object $document Document containing fields mapped to values,
* which denote order or an index type
* @throws InvalidArgumentException if $document is not an array or object
*/
function generate_index_name($document): string
{
$document = document_to_array($document);

$name = '';

foreach ($document as $field => $type) {
$name .= ($name != '' ? '_' : '') . $field . '_' . $type;
}

return $name;
}

/**
* Return a collection's encryptedFields from the encryptedFieldsMap
* autoEncryption driver option (if available).
Expand Down
16 changes: 0 additions & 16 deletions tests/FunctionsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
use function MongoDB\apply_type_map_to_document;
use function MongoDB\create_field_path_type_map;
use function MongoDB\document_to_array;
use function MongoDB\generate_index_name;
use function MongoDB\is_first_key_operator;
use function MongoDB\is_last_pipeline_operator_write;
use function MongoDB\is_mapreduce_output_inline;
Expand Down Expand Up @@ -121,14 +120,6 @@ public function testDocumentToArrayArgumentTypeCheck($document): void
document_to_array($document);
}

/** @dataProvider provideDocumentCasts */
public function testGenerateIndexName($cast): void
{
$this->assertSame('x_1', generate_index_name($cast(['x' => 1])));
$this->assertSame('x_-1_y_1', generate_index_name($cast(['x' => -1, 'y' => 1])));
$this->assertSame('x_2dsphere_y_1', generate_index_name($cast(['x' => '2dsphere', 'y' => 1])));
}

public function provideDocumentCasts(): array
{
// phpcs:disable SlevomatCodingStandard.ControlStructures.JumpStatementsSpacing
Expand All @@ -143,13 +134,6 @@ public function provideDocumentCasts(): array
// phpcs:enable
}

/** @dataProvider provideInvalidDocumentValues */
public function testGenerateIndexNameArgumentTypeCheck($document): void
{
$this->expectException(InvalidArgumentException::class);
generate_index_name($document);
}

/** @dataProvider provideDocumentCasts */
public function testIsFirstKeyOperator(callable $cast): void
{
Expand Down
14 changes: 11 additions & 3 deletions tests/Model/IndexInputTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

namespace MongoDB\Tests\Model;

use MongoDB\BSON\Document;
use MongoDB\BSON\Serializable;
use MongoDB\Exception\InvalidArgumentException;
use MongoDB\Model\BSONDocument;
use MongoDB\Model\IndexInput;
use MongoDB\Tests\TestCase;
use stdClass;
Expand Down Expand Up @@ -40,16 +42,22 @@ public function testConstructorShouldRequireNameToBeString(): void
new IndexInput(['key' => ['x' => 1], 'name' => 1]);
}

/** @dataProvider provideExpectedNameAndKey */
public function testNameGeneration($expectedName, array $key): void
/**
* @dataProvider provideExpectedNameAndKey
* @param array|object $key
*/
public function testNameGeneration($expectedName, $key): void
{
$this->assertSame($expectedName, (string) new IndexInput(['key' => $key]));
}

public function provideExpectedNameAndKey()
public function provideExpectedNameAndKey(): array
{
return [
['x_1', ['x' => 1]],
['x_1', (object) ['x' => 1]],
['x_1', new BSONDocument(['x' => 1])],
['x_1', Document::fromPHP(['x' => 1])],
Copy link
Member

Choose a reason for hiding this comment

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

I noted that the original tests for the function were more comprehensive (the four possible type casts are applied to each index specification), but I don't think there's any significant benefit to doing that. Testing a basic index spec with the non-array types seems sufficient.

['x_1_y_-1', ['x' => 1, 'y' => -1]],
['loc_2dsphere', ['loc' => '2dsphere']],
['loc_2dsphere_x_1', ['loc' => '2dsphere', 'x' => 1]],
Expand Down