Skip to content

Split API classes and interfaces #2

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.

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 5 commits into from
Jun 16, 2014
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
61 changes: 0 additions & 61 deletions docs/api/MongoDB/CRUD.php

This file was deleted.

14 changes: 14 additions & 0 deletions docs/api/MongoDB/Command/Command.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

namespace MongoDB\Command;

final class Command
Copy link
Member Author

Choose a reason for hiding this comment

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

This and Query do not have getter methods, since I assume their data will be accessed internally. Users shouldn't have any reason to read these objects after creating them.

{
/**
* @param array|object $document Command document
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really want to have this as both array and object again?

*/
public function __construct($document)
{
// ...
}
}
58 changes: 58 additions & 0 deletions docs/api/MongoDB/Command/CommandCursor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

namespace MongoDB\Command;

// Note: consider combining implementation with \MongoDB\Query\QueryCursor
final class CommandCursor implements \MongoDB\Cursor
{
private $server;
private $batchSize;
private $cursorId;
private $firstBatch;

/**
* @param Server $server
* @param integer $cursorId
* @param array $firstBatch
*/
public function __construct(Server $server, $cursorId, array $firstBatch)
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it make more sense to just pass in a document that was returned from the "runCommand" method (+ $server)? Is there a need to split it up into cursorId and firstBatch yourself?

{
$this->server = $server;
$this->cursorId = (integer) $cursorId;
Copy link
Contributor

Choose a reason for hiding this comment

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

Sadly, you can't do this. On a 32bit platform (and Windows), $cursorID has to be a MongoInt64 object.

$this->firstBatch = $firstBatch;
}

// Iterator methods...

/**
* @see \MongoDB\Cursor::getId()
*/
public function getId()
{
return $this->cursorId;
}

/**
* @see \MongoDB\ServerResult::getServer()
*/
public function getServer()
{
return $this->server;
}

/**
* @see \MongoDB\Cursor::setBatchSize()
*/
public function setBatchSize($batchSize)
{
$this->batchSize = (integer) $batchSize;
}

// Note: this expects consistent command response documents from the server
static public function createFromCommandResult(CommandResult $result)
Copy link
Member Author

Choose a reason for hiding this comment

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

These factory methods may not even be necessary, as the userland layer can simply interpret the command response and construct the cursors outright.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, don't add this if it's not necessary... but, alternatively the constructor could just accept Server and CommandResult instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

As discussed in video chat, the constructor cannot take a CommandResult directly, since that leaves us with now way to create command cursors from parallelCollectionScan result documents. We agreed to remove this factory method completely.

{
// extract $cursorId and $firstBatch from $result->getResponse()

return new static($result->getServer(), $cursorId, $firstBatch);
}
}
11 changes: 11 additions & 0 deletions docs/api/MongoDB/Command/CommandResult.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace MongoDB\Command;

interface CommandResult extends \MongoDB\ServerResult
{
/**
* @return array Original response document from the server
*/
function getResponse();
Copy link
Contributor

Choose a reason for hiding this comment

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

I would probably suggest calling this getResponseDocument() — it's more descriptive as to what it does. Right now, it could be interpreted as some pre-processed response.

}
16 changes: 16 additions & 0 deletions docs/api/MongoDB/Cursor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

namespace MongoDB;

interface Cursor extends ServerResult, \Iterator
Copy link
Contributor

Choose a reason for hiding this comment

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

What is it extending ServerResult for?

{
/**
* @return integer Cursor identifier
*/
function getId();

/**
* @param integer $batchSize
*/
function setBatchSize($batchSize);
}
55 changes: 0 additions & 55 deletions docs/api/MongoDB/Management.php

This file was deleted.

50 changes: 50 additions & 0 deletions docs/api/MongoDB/Manager.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<?php

namespace MongoDB;

final class Manager extends \SplObjectStorage
{
/**
* @param Server[] $servers
* @param string $db Database name
* @param array $options Connection options
*/
public function __construct(array $servers, $db, $options)
Copy link
Member Author

Choose a reason for hiding this comment

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

@bjori: What was the intention of the database and connection options here? If we have Server instances, aren't the connections already established?

Copy link
Member Author

Choose a reason for hiding this comment

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

We discussed that both arguments should be removed, and we're be swapping the constructor with fromDsn(), since DSN-based construction is the default convention. Servers can be manually constructed with their own options, and we'll have a general factory method to create a Manager with a specific collection of servers.

{
}

static public function fromDSN($dsn)
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this intended to be comparable to MongoClient's constructor? If so, wouldn't we need a DSN and connection options?

Copy link
Contributor

Choose a reason for hiding this comment

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

Technically the DSN should be able to include all connection options, but I would prefer it being a second (array) argument too.

{
}

final public function executeCommand($namespace, \MongoDB\Query $query, \MongoDB\ReadPreference $rp)
Copy link
Member Author

Choose a reason for hiding this comment

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

ReadPreference and WriteOptions classes need to be defined. Commands don't always require a ReadPreference, so I'm not sure how best to handle this. Additionally, commands and queries alike probably need general options to handle socket timeouts (something I imagine was intended to be handled with WriteOptions). Perhaps we need a ClientOptions argument, too?

Copy link
Contributor

Choose a reason for hiding this comment

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

$namespace should be \MongoDB\Namespace $namespace

{
foreach($this->getConnectedServers() as $server) {
if ($server->matchesReadPreference($rp)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is likely the place to apply the white list of commands that must be sent to a primary.

return $server->executeQuery($namespace, $query);
}
}
throw new NoServerMatchingReadPreference($rp);
}

final public function executeQuery($namespace, \MongoDB\Query $query, \MongoDB\ReadPreference $rp)
{
foreach($this->getConnectedServers() as $server) {
if ($server->matchesReadPreference($rp)) {
return $server->executeQuery($namespace, $query);
}
}
throw new NoServerMatchingReadPreference($rp);
}

final function executeWrite($namespace, \MongoDB\WriteBatch $batch, \MongoDB\WriteOptions $wo)
{
foreach($this->getConnectedServers() as $server) {
if ($server->isPrimary()) {
return $server->executeWrite($namespace, $batch, $wo);
}
}

throw new NoPrimaryAvailable;
Copy link
Member Author

Choose a reason for hiding this comment

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

NoPrimaryAvailable and NoServerMatchingReadPreference need to be defined.

}
}
18 changes: 18 additions & 0 deletions docs/api/MongoDB/Query/Query.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

namespace MongoDB\Query;

final class Query
{
/**
* @param array|object $query Query document
* @param array|object $selector Selector document
* @param integer $flags Query flags
* @param integer $limit Limit
* @param integer $skip Skip
*/
public function __construct($query, $selector, $flags, $skip, $limit)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is essentially everything that makes up an OP_QUERY (i.e. everything necessary to create a cursor on the server).

Copy link
Contributor

Choose a reason for hiding this comment

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

If that's the case, we need to define the bitfields constants for all the flags. Or alternatively, we can make query flags an array with the options in there.

Copy link
Member Author

Choose a reason for hiding this comment

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

As discussed, I left $flags an integer and added constants for the wire protocol flags.

{
// ...
}
}
47 changes: 47 additions & 0 deletions docs/api/MongoDB/Query/QueryCursor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

namespace MongoDB\Query;

// Note: consider combining implementation with \MongoDB\Command\CommandCursor
Copy link
Member Author

Choose a reason for hiding this comment

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

Both cursor classes can probably share most all of their code, especially if libmongoc is already abstracting command and query cursors for us.

The "first batch" handling for a command cursor may still apply for query cursors, as it's comparable to the documents returned in the first OP_REPLY after OP_QUERY.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that's pretty much what firstBatch would mean for OP_QUERY results.

final class QueryCursor implements \MongoDB\Cursor
{
private $server;
private $batchSize;
private $cursorId;

/**
* @param Server $server
* @param integer $cursorId
*/
public function __construct(Server $server, $cursorId)
{
$this->server = $server;
$this->cursorId = (integer) $cursorId;
}

// Iterator methods...

/**
* @see \MongoDB\Cursor::getId()
*/
public function getId()
Copy link
Contributor

Choose a reason for hiding this comment

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

If we do export this, it should always return a MongoInt64 object.

{
return $this->cursorId;
}

/**
* @see \MongoDB\ServerResult::getServer()
*/
public function getServer()
{
return $this->server;
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do?

}

/**
* @see \MongoDB\Cursor::setBatchSize()
*/
public function setBatchSize($batchSize)
{
$this->batchSize = (integer) $batchSize;
}
}
7 changes: 7 additions & 0 deletions docs/api/MongoDB/Query/QueryResult.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

namespace MongoDB\Query;

interface QueryResult extends \MongoDB\ServerResult, \Traversable
{
}
30 changes: 30 additions & 0 deletions docs/api/MongoDB/Server.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

namespace MongoDB;

class Server
{
const TYPE_MONGOS = 0x01;
const TYPE_STANDALONE = 0x02;
const TYPE_ARBITER = 0x03;
const TYPE_SECONDARY = 0x04;
const TYPE_PRIMARY = 0x05;

/* These need to be final as they will not be called by the driver to
* retrieve the information; these getters are only useful for userland.
*/
Copy link
Member Author

Choose a reason for hiding this comment

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

@bjori: Can you elaborate on this? Did you mean to imply that the extension is just going to read internal properties directly?

final public function getHostname() {}
final public function getPort() {}
final public function getLatency() {}
final public function getMaxWireVersion() {}
final public function getMinWireVersion() {}
final public function getServerType() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we previous discussed just to have a "getServerInfo" method instead of all those getters?


// extracting config info
final public function isPassive() {}
final public function isDelayed() {}

final public function executeCommand($namespace, \MongoDB\Command\Command $command, \MongoDB\ReadPreference $rp) {}
final public function executeQuery($namespace, \MongoDB\Query\Query $query, \MongoDB\ReadPreference $rp) {}
final public function executeWrite($namespace, \MongoDB\Write\WriteBatch $batch, \MongoDB\WriteOptions $wo) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

And this needs the insert, update and delete methods too. With $namespace being our MongoDB\Namespace value object.

}
16 changes: 16 additions & 0 deletions docs/api/MongoDB/ServerError.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

namespace MongoDB;

interface ServerError extends ServerResult
{
/**
* @return integer Server error code
*/
function getCode();

/**
* @return string Server error message
*/
function getMessage();
Copy link
Contributor

Choose a reason for hiding this comment

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

There is sometimes more than one message. wnote, jnote, msg, etc.

}
Loading