Skip to content

PHPLIB-1238: Order internal methods after non-internal methods #1162

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
Sep 15, 2023
Merged

PHPLIB-1238: Order internal methods after non-internal methods #1162

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
9eebbd7
PHPLIB-1238: Order internal methods after non-internal methods
jmikola Sep 15, 2023
bd098e7
Merge branch 'master' into phplib-1238
jmikola Sep 15, 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
32 changes: 16 additions & 16 deletions src/ChangeStream.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,22 +87,6 @@ class ChangeStream implements Iterator

private ?DocumentCodec $codec;

/**
* @internal
*
* @param ResumeCallable $resumeCallable
*/
public function __construct(ChangeStreamIterator $iterator, callable $resumeCallable, ?DocumentCodec $codec = null)
{
$this->iterator = $iterator;
$this->resumeCallable = $resumeCallable;
$this->codec = $codec;

if ($codec) {
$this->iterator->getInnerIterator()->setTypeMap(['root' => 'bson']);
}
}

/**
* @see https://php.net/iterator.current
* @return array|object|null
Expand Down Expand Up @@ -200,6 +184,22 @@ public function valid()
return $this->iterator->valid();
}

/**
* @internal
*
* @param ResumeCallable $resumeCallable
*/
public function __construct(ChangeStreamIterator $iterator, callable $resumeCallable, ?DocumentCodec $codec = null)
{
$this->iterator = $iterator;
$this->resumeCallable = $resumeCallable;
$this->codec = $codec;

if ($codec) {
$this->iterator->getInnerIterator()->setTypeMap(['root' => 'bson']);
}
}

/**
* Determines if an exception is a resumable error.
*
Expand Down
28 changes: 14 additions & 14 deletions src/MapReduceResult.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,6 @@ class MapReduceResult implements IteratorAggregate

private array $timing;

/**
* @internal
* @param callable $getIterator Callback that returns a Traversable for mapReduce results
* @param stdClass $result Result document from the mapReduce command
* @psalm-param MapReduceCallable $getIterator
*/
public function __construct(callable $getIterator, stdClass $result)
{
$this->getIterator = $getIterator;
$this->executionTimeMS = isset($result->timeMillis) ? (integer) $result->timeMillis : 0;
$this->counts = isset($result->counts) ? (array) $result->counts : [];
$this->timing = isset($result->timing) ? (array) $result->timing : [];
}

/**
* Returns various count statistics from the mapReduce command.
*
Expand Down Expand Up @@ -108,4 +94,18 @@ public function getTiming()
{
return $this->timing;
}

/**
* @internal
* @param callable $getIterator Callback that returns a Traversable for mapReduce results
* @param stdClass $result Result document from the mapReduce command
* @psalm-param MapReduceCallable $getIterator
*/
public function __construct(callable $getIterator, stdClass $result)
{
$this->getIterator = $getIterator;
$this->executionTimeMS = isset($result->timeMillis) ? (integer) $result->timeMillis : 0;
$this->counts = isset($result->counts) ? (array) $result->counts : [];
$this->timing = isset($result->timing) ? (array) $result->timing : [];
}
}
56 changes: 28 additions & 28 deletions src/Model/BSONIterator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,34 +51,6 @@ class BSONIterator implements Iterator

private array $options;

/**
* Constructs a BSON Iterator.
*
* Supported options:
*
* * typeMap (array): Type map for BSON deserialization.
*
* @internal
* @see https://php.net/manual/en/function.mongodb.bson-tophp.php
* @param string $data Concatenated, valid, BSON-encoded documents
* @param array $options Iterator options
* @throws InvalidArgumentException for parameter/option parsing errors
*/
public function __construct(string $data, array $options = [])
{
if (isset($options['typeMap']) && ! is_array($options['typeMap'])) {
throw InvalidArgumentException::invalidType('"typeMap" option', $options['typeMap'], 'array');
}

if (! isset($options['typeMap'])) {
$options['typeMap'] = [];
}

$this->buffer = $data;
$this->bufferLength = strlen($data);
$this->options = $options;
}

/**
* @see https://php.net/iterator.current
* @return mixed
Expand Down Expand Up @@ -131,6 +103,34 @@ public function valid(): bool
return $this->current !== null;
}

/**
* Constructs a BSON Iterator.
*
* Supported options:
*
* * typeMap (array): Type map for BSON deserialization.
*
* @internal
* @see https://php.net/manual/en/function.mongodb.bson-tophp.php
* @param string $data Concatenated, valid, BSON-encoded documents
* @param array $options Iterator options
* @throws InvalidArgumentException for parameter/option parsing errors
*/
public function __construct(string $data, array $options = [])
{
if (isset($options['typeMap']) && ! is_array($options['typeMap'])) {
throw InvalidArgumentException::invalidType('"typeMap" option', $options['typeMap'], 'array');
}

if (! isset($options['typeMap'])) {
$options['typeMap'] = [];
}

$this->buffer = $data;
$this->bufferLength = strlen($data);
$this->options = $options;
}

private function advance(): void
{
if ($this->position === $this->bufferLength) {
Expand Down
130 changes: 65 additions & 65 deletions src/Model/ChangeStreamIterator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,71 +68,6 @@ class ChangeStreamIterator extends IteratorIterator implements CommandSubscriber

private Server $server;

/**
* @internal
* @param array|object|null $initialResumeToken
* @psalm-param CursorInterface<int, TValue>&Iterator<int, TValue> $cursor
*/
public function __construct(CursorInterface $cursor, int $firstBatchSize, $initialResumeToken, ?object $postBatchResumeToken)
{
if (! $cursor instanceof Iterator) {
throw InvalidArgumentException::invalidType(
'$cursor',
$cursor,
CursorInterface::class . '&' . Iterator::class,
);
}

if (isset($initialResumeToken) && ! is_document($initialResumeToken)) {
throw InvalidArgumentException::expectedDocumentType('$initialResumeToken', $initialResumeToken);
}

parent::__construct($cursor);

$this->batchSize = $firstBatchSize;
$this->isRewindNop = ($firstBatchSize === 0);
$this->postBatchResumeToken = $postBatchResumeToken;
$this->resumeToken = $initialResumeToken;
$this->server = $cursor->getServer();
}

/** @internal */
final public function commandFailed(CommandFailedEvent $event): void
{
}

/** @internal */
final public function commandStarted(CommandStartedEvent $event): void
{
if ($event->getCommandName() !== 'getMore') {
return;
}

$this->batchPosition = 0;
$this->batchSize = 0;
$this->postBatchResumeToken = null;
}

/** @internal */
final public function commandSucceeded(CommandSucceededEvent $event): void
{
if ($event->getCommandName() !== 'getMore') {
return;
}

$reply = $event->getReply();

if (! isset($reply->cursor->nextBatch) || ! is_array($reply->cursor->nextBatch)) {
throw new UnexpectedValueException('getMore command did not return a "cursor.nextBatch" array');
}

$this->batchSize = count($reply->cursor->nextBatch);

if (isset($reply->cursor->postBatchResumeToken) && is_object($reply->cursor->postBatchResumeToken)) {
$this->postBatchResumeToken = $reply->cursor->postBatchResumeToken;
}
}

/**
* @see https://php.net/iteratoriterator.current
* @return array|object|null
Expand Down Expand Up @@ -239,6 +174,71 @@ public function valid(): bool
return $this->isValid;
}

/**
* @internal
* @param array|object|null $initialResumeToken
* @psalm-param CursorInterface<int, TValue>&Iterator<int, TValue> $cursor
*/
public function __construct(CursorInterface $cursor, int $firstBatchSize, $initialResumeToken, ?object $postBatchResumeToken)
{
if (! $cursor instanceof Iterator) {
throw InvalidArgumentException::invalidType(
'$cursor',
$cursor,
CursorInterface::class . '&' . Iterator::class,
);
}

if (isset($initialResumeToken) && ! is_document($initialResumeToken)) {
throw InvalidArgumentException::expectedDocumentType('$initialResumeToken', $initialResumeToken);
}

parent::__construct($cursor);

$this->batchSize = $firstBatchSize;
$this->isRewindNop = ($firstBatchSize === 0);
$this->postBatchResumeToken = $postBatchResumeToken;
$this->resumeToken = $initialResumeToken;
$this->server = $cursor->getServer();
}

/** @internal */
final public function commandFailed(CommandFailedEvent $event): void
{
}

/** @internal */
final public function commandStarted(CommandStartedEvent $event): void
{
if ($event->getCommandName() !== 'getMore') {
return;
}

$this->batchPosition = 0;
$this->batchSize = 0;
$this->postBatchResumeToken = null;
}

/** @internal */
final public function commandSucceeded(CommandSucceededEvent $event): void
{
if ($event->getCommandName() !== 'getMore') {
return;
}

$reply = $event->getReply();

if (! isset($reply->cursor->nextBatch) || ! is_array($reply->cursor->nextBatch)) {
throw new UnexpectedValueException('getMore command did not return a "cursor.nextBatch" array');
}

$this->batchSize = count($reply->cursor->nextBatch);

if (isset($reply->cursor->postBatchResumeToken) && is_object($reply->cursor->postBatchResumeToken)) {
$this->postBatchResumeToken = $reply->cursor->postBatchResumeToken;
}
}

/**
* Extracts the resume token (i.e. "_id" field) from a change document.
*
Expand Down
34 changes: 17 additions & 17 deletions src/Operation/Watch.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,23 @@ public function __construct(Manager $manager, ?string $databaseName, ?string $co
$this->aggregate = $this->createAggregate();
}

/**
* Execute the operation.
*
* @see Executable::execute()
* @return ChangeStream
* @throws UnsupportedException if collation or read concern is used and unsupported
* @throws RuntimeException for other driver errors (e.g. connection errors)
*/
public function execute(Server $server)
{
return new ChangeStream(
$this->createChangeStreamIterator($server),
fn ($resumeToken, $hasAdvanced): ChangeStreamIterator => $this->resume($resumeToken, $hasAdvanced),
$this->codec,
);
}

/** @internal */
final public function commandFailed(CommandFailedEvent $event): void
{
Expand Down Expand Up @@ -317,23 +334,6 @@ final public function commandSucceeded(CommandSucceededEvent $event): void
}
}

/**
* Execute the operation.
*
* @see Executable::execute()
* @return ChangeStream
* @throws UnsupportedException if collation or read concern is used and unsupported
* @throws RuntimeException for other driver errors (e.g. connection errors)
*/
public function execute(Server $server)
{
return new ChangeStream(
$this->createChangeStreamIterator($server),
fn ($resumeToken, $hasAdvanced): ChangeStreamIterator => $this->resume($resumeToken, $hasAdvanced),
$this->codec,
);
}

/**
* Create the aggregate command for a change stream.
*
Expand Down
14 changes: 4 additions & 10 deletions tests/PedantryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use function array_filter;
use function array_map;
use function realpath;
use function str_contains;
use function str_replace;
use function strcasecmp;
use function strlen;
Expand All @@ -37,17 +38,10 @@ public function testMethodsAreOrderedAlphabeticallyByVisibility($className): voi
);

$getSortValue = function (ReflectionMethod $method) {
if ($method->getModifiers() & ReflectionMethod::IS_PRIVATE) {
return '2' . $method->getName();
}

if ($method->getModifiers() & ReflectionMethod::IS_PROTECTED) {
return '1' . $method->getName();
}
$prefix = $method->isPrivate() ? '2' : ($method->isProtected() ? '1' : '0');
$prefix .= str_contains($method->getDocComment(), '@internal') ? '1' : '0';
Copy link
Member Author

Choose a reason for hiding this comment

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

@GromNaN: I saw your comment in PHPLIB-1238 but it didn't seem pertinent. The first character here is used to sort on visibility, and the second reports whether @internal is used.

Copy link
Member

Choose a reason for hiding this comment

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

Ok sounds good


if ($method->getModifiers() & ReflectionMethod::IS_PUBLIC) {
return '0' . $method->getName();
}
return $prefix . $method->getName();
};

$sortedMethods = $methods;
Expand Down