Skip to content

PHPLIB-70: Resource CRUD prototypes #5

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 8 commits into from
Mar 18, 2015
Merged

PHPLIB-70: Resource CRUD prototypes #5

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
786471a
PHPLIB-69: Prototypes for index drop methods
jmikola Mar 18, 2015
0c0d334
PHPLIB-63: Prototypes for index creation methods
jmikola Mar 18, 2015
4b0d5ad
PHPLIB-64: Prototype for collection create method
jmikola Mar 18, 2015
b51e1b3
PHPLIB-65: Prototypes for database drop methods
jmikola Mar 18, 2015
b6a120e
PHPLIB-71: Prototypes for collection drop methods
jmikola Mar 18, 2015
495e46d
Declare Collection methods alphabetically by visibility
jmikola Mar 18, 2015
448800b
PHPLIB-46: Prototypes for index enumeration method
jmikola Mar 18, 2015
ea6da09
PHPLIB-45: Prototype for collection enumeration method
jmikola Mar 18, 2015
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
16 changes: 14 additions & 2 deletions src/Client.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

namespace MongoDB;

use MongoDB\Driver\Manager;
use MongoDB\Database;
use MongoDB\Collection;
use MongoDB\Database;
use MongoDB\Driver\Manager;
use MongoDB\Driver\Result;

class Client
{
Expand All @@ -29,6 +30,17 @@ public function __construct($uri, $options, $driverOptions)
$this->manager = new Manager($uri, $options, $driverOptions);
}

/**
* Drop a database.
*
* @param string $databaseName
* @return Result
*/
public function dropDatabase($databaseName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the Database object have __toString() method and this code explicitly do (string)$databaseName ?

Copy link
Member Author

Choose a reason for hiding this comment

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

If we did that and allowed the argument to be either a string or a Database instance, would we get into trouble with dropCollection()? I would expect a Collection's __toString() method to return its full namespace, as we do in the 1.x driver. I'd rather not invite the inconsistency.

Copy link
Contributor

Choose a reason for hiding this comment

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

The proto should definetly not be string|Database, it should be string - but if it does (string) casting inside the method it implicitly allows the Database object to work seamlessly.

as for dropCollection() returning fully qualified namespace...... mmmmh.....

{
// TODO
}

/**
* Select a database
*
Expand Down
67 changes: 67 additions & 0 deletions src/Collection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,73 @@ public function __construct(Manager $manager, $ns, WriteConcern $wc = null, Read
list($this->dbname, $this->collname) = explode(".", $ns, 2);
}

/**
* Create a single index in the collection.
*
* @see http://docs.mongodb.org/manual/reference/command/createIndexes/
* @see http://docs.mongodb.org/manual/reference/method/db.collection.createIndex/
* @param array|object $keys
* @param array $options
* @return string The name of the created index
*/
public function createIndex($keys, array $options = array())
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 the usecase of having it object as apposed of useful array?

Copy link
Member Author

Choose a reason for hiding this comment

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

The user should be able to specify a BSON object.

I was thinking of the case where they want to index the 0 field ascending. We shouldn't send over a BSON array. While I could force-cast to an object internally, I figure this leaves them the flexibility and also means we can take a BSON object (whenever we create that type) here.

{
// TODO
}

/**
* Create multiple indexes in the collection.
*
* TODO: decide if $models should be an array of associative arrays, using
* createIndex()'s parameter names as keys, or tuples, using parameters in
* order (e.g. [keys, options]).
*
* @see http://docs.mongodb.org/manual/reference/command/createIndexes/
* @see http://docs.mongodb.org/manual/reference/method/db.collection.createIndex/
* @param array $models
* @return string[] The names of the created indexes
*/
public function createIndexes(array $models)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

It needs to throw DuplicateIndex exception or something like that

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 only a problem if the model or name match an existing index with different options, correct? Sounds like something we should document in the spec, too.

Copy link
Contributor

Choose a reason for hiding this comment

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

no I mean, the proto is missing @throws declaration that it can throw various exceptions, such as duplicateindex (should we have explicit exception for that?), or other Exceptions

// TODO
}

/**
* Drop this collection.
*
* @return Result
*/
public function drop()
{
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens to this instance after calling drop?

Copy link
Member Author

Choose a reason for hiding this comment

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

exec('halt -f');
exit(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.

But really, we don't need to do anything. A Collection instance doesn't require anything to actually exist in the database, so there's nothing to clean up in PHP. If we started doing stuff like caching index enumeration class, perhaps, but not now.

Copy link
Contributor

Choose a reason for hiding this comment

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

$collection = $db->createCollection("foo", array("storageEngineOptions" => array("xyz"));
$collection->insert(array("foo" => "bar"));
$collection->drop();
$collection->insert(array("foo" => "bar")); // This will create different configured collection then above

// TODO
}

/**
* Drop a single index in the collection.
*
* @see http://docs.mongodb.org/manual/reference/command/dropIndexes/
* @see http://docs.mongodb.org/manual/reference/method/db.collection.dropIndex/
* @param string $indexName
* @return Result
* @throws InvalidArgumentException if "*" is specified
*/
public function dropIndex($indexName)
{
// TODO
}

/**
* Drop all indexes in the collection.
*
* @see http://docs.mongodb.org/manual/reference/command/dropIndexes/
* @see http://docs.mongodb.org/manual/reference/method/db.collection.dropIndexes/
* @return Result
*/
public function dropIndexes()
{
// TODO
}

/**
* Performs a find (query) on the collection
*
Expand Down
38 changes: 37 additions & 1 deletion src/Database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

namespace MongoDB;

use MongoDB\Driver\Manager;
use MongoDB\Collection;
use MongoDB\Driver\Manager;
use MongoDB\Driver\Result;

class Database
{
Expand Down Expand Up @@ -32,6 +33,41 @@ public function __construct(Manager $manager, $databaseName, WriteConcern $wc =
$this->rp = $rp;
}

/**
* Create a new collection explicitly.
*
* @see http://docs.mongodb.org/manual/reference/command/create/
* @see http://docs.mongodb.org/manual/reference/method/db.createCollection/
* @param string $collectionName
* @param array $options
* @return Result
*/
public function createCollection($collectionName, array $options = array())
{
// TODO
}

/**
* Drop this database.
*
* @return Result
*/
public function drop()
{
// TODO
}

/**
* Drop a collection within this database.
*
* @param string $collectionName
* @return Result
*/
public function dropCollection($collectionName)
{
// TODO
}

/**
* Select a specific collection in this database
*
Expand Down