Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

add mongodb writer to writer plugin manager #38

Closed
wants to merge 6 commits into from

Conversation

prolic
Copy link
Contributor

@prolic prolic commented Apr 6, 2016

resolves #37

@@ -146,6 +147,53 @@ public function testRetrievesDatabaseServiceFromServiceManagerWhenEncounteringDb
$this->assertAttributeSame($db, 'db', $writer);
}

public function testRetrievesMongoDBServiceFromServiceManagerWhenEncounteringMongoDbWriter()
{
$mongoClient = $this->getMockBuilder('MongoClient')
Copy link
Member

Choose a reason for hiding this comment

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

This test should check for existence of the mongodb extension prior, and mark the test skipped if it is not. I can make that change on merge.

@weierophinney
Copy link
Member

Ouch... there's another problem.

Evidently, Travis is installing ext/mongodb, vs ext/mongo, with current builds. ext/mongodb does not ship the MongoClient or Mongo classes that were present in the various 1.* builds. As such, tests currently do not run.

I'm not quite sure how to resolve this, but I'm a bit reluctant to expose the mongo writer unless we know it also works with ext/mongodb. Thoughts?

@prolic
Copy link
Contributor Author

prolic commented Apr 7, 2016

I'd say let's make two separate writers: 1) mongo 2) mongodb for ext-mongo and ext-mongodb.

@prolic
Copy link
Contributor Author

prolic commented Apr 7, 2016

About travis: I can install both extensions for travis, this is no big deal.

run hhvm tests on travis
@sandrokeil
Copy link

I guess we run into the same problems as mentioned in the zend-session component. We should use an extra repo (e.g. zend-log-mongodb) for specific writer, which relies on dependencies.

@prolic
Copy link
Contributor Author

prolic commented Apr 8, 2016

@weierophinney If you want me to change this to separate repos, please create those repos for me so I can make the split. Otherwise it should be ready to merge.

@samsonasik
Copy link
Contributor

ping @weierophinney

* @param string $database
* @param string $collection
* @param array $saveOptions
* @throws Exception\InvalidArgumentException
Copy link
Contributor

Choose a reason for hiding this comment

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

add @throws Exception\ExtensionNotLoadedException

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.


$this->mongoCollection->expects($this->once())
->method('save')
->with($this->contains(new MongoDate($date->getTimestamp()), false));
Copy link
Contributor

Choose a reason for hiding this comment

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

MongoDB\BSON\UTCDateTime ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this is MongoTest, not MongoDBTest

$saveOptions = isset($mongo['save_options']) ? $mongo['save_options'] : [];
$collection = isset($mongo['collection']) ? $mongo['collection'] : null;
$database = isset($mongo['database']) ? $mongo['database'] : null;
$mongo = isset($mongo['mongo']) ? $mongo['mongo'] : null;
Copy link
Member

Choose a reason for hiding this comment

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

Wish we were on PHP 7 as the minimum version, so we could use ?? here. 😦

weierophinney added a commit that referenced this pull request Jun 22, 2016
add mongodb writer to writer plugin manager
weierophinney added a commit that referenced this pull request Jun 22, 2016
weierophinney added a commit that referenced this pull request Jun 22, 2016
@weierophinney
Copy link
Member

Merged to develop for release with 2.9.0.

@prolic prolic deleted the mongodb_writer branch June 23, 2016 05:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MongoDB Writer is missing in WriterPluginManager
5 participants