Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixed PHP-793: Add deprecation notice to non-array options for MongoDB::createCollection. #439

Closed
wants to merge 1 commit into from

2 participants

@derickr
Owner

I've added some tests to test the new array options and renamed the old tests
to "-old". arginfo was still incorrect, so fixed that as well.

db.c
@@ -948,9 +949,7 @@ MONGO_ARGINFO_STATIC ZEND_BEGIN_ARG_INFO_EX(arginfo_selectCollection, 0, ZEND_RE
MONGO_ARGINFO_STATIC ZEND_BEGIN_ARG_INFO_EX(arginfo_createCollection, 0, ZEND_RETURN_VALUE, 1)
ZEND_ARG_INFO(0, collection_name)
- ZEND_ARG_INFO(0, capped)
- ZEND_ARG_INFO(0, capped_size)
- ZEND_ARG_INFO(0, max_elements)
+ ZEND_ARG_INFO(0, options)
@bjori Owner
bjori added a note

will we ever be able to typehint this now?

@derickr Owner
derickr added a note
@bjori Owner
bjori added a note

This change in itself breaks classes that overwrite this method.
And then we will be breaking it again when the deprecation period is over?

@derickr Owner
derickr added a note

Good point, but actually, this fixes something as well. Before this change, it was already broken:

class MyDBNew extends MongoDB
{
    function createCollection($name, array $options = NULL)
    {
        return parent::createCollection($name, $options);
    }
}

generates:

Strict standards: Declaration of MyDBNew::createCollection() should be compatible with MongoDB::createCollection($collection_name, $capped = NULL, $capped_size = NULL, $max_elements = NULL) in /tmp/quick-test.php on line 16

After my change to only have collection_name and options, that one is indeed still broken (as it misses the array bit). But this one works fine still:

class MyDBOld extends MongoDB
{
    function createCollection($name, $capped = NULL, $size = NULL, $max = NULL)
    {
        return parent::createCollection($name, $capped, $size, $max);

If I change the arginfo hint to:

ZEND_ARG_ARRAY_INFO

then

function createCollection($name, $capped = NULL, $size = NULL, $max = NULL)

stops working of course.

Leaving the second "options" out of the argument makes both work. I've updated the PR with this. We can put the typehint back there when we remove support for $capped, $size, $max.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@derickr derickr Fixed PHP-793: Add deprecation notice to non-array options for MongoD…
…B::createCollection.

I've added some tests to test the new array options and renamed the old tests
to "-old". arginfo was still incorrect, so fixed that as well.
d1f42e1
@bjori bjori commented on the diff
@@ -948,9 +949,6 @@ MONGO_ARGINFO_STATIC ZEND_BEGIN_ARG_INFO_EX(arginfo_selectCollection, 0, ZEND_RE
MONGO_ARGINFO_STATIC ZEND_BEGIN_ARG_INFO_EX(arginfo_createCollection, 0, ZEND_RETURN_VALUE, 1)
ZEND_ARG_INFO(0, collection_name)
- ZEND_ARG_INFO(0, capped)
- ZEND_ARG_INFO(0, capped_size)
- ZEND_ARG_INFO(0, max_elements)
ZEND_END_ARG_INFO()
@bjori Owner
bjori added a note

Thats still wrong... but since the rest of the arguemtsn are optional I guess it doesn't break anything?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@bjori
Owner

I think this really should be included if the current arginfo is fixed.
LGTM

@derickr derickr referenced this pull request from a commit
@derickr derickr Merged pull request #439 7e5d988
@derickr derickr closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 9, 2013
  1. @derickr

    Fixed PHP-793: Add deprecation notice to non-array options for MongoD…

    derickr authored
    …B::createCollection.
    
    I've added some tests to test the new array options and renamed the old tests
    to "-old". arginfo was still incorrect, so fixed that as well.
This page is out of date. Refresh to see the latest.
View
4 db.c
@@ -368,6 +368,7 @@ PHP_METHOD(MongoDB, createCollection)
}
if (capped) {
+ php_error_docref(NULL TSRMLS_CC, MONGO_E_DEPRECATED, "This method now accepts arguments as an options array instead of the three optional arguments for capped, size and max elements");
add_assoc_bool(data, "capped", 1);
if (max) {
add_assoc_long(data, "max", max);
@@ -948,9 +949,6 @@ ZEND_END_ARG_INFO()
MONGO_ARGINFO_STATIC ZEND_BEGIN_ARG_INFO_EX(arginfo_createCollection, 0, ZEND_RETURN_VALUE, 1)
ZEND_ARG_INFO(0, collection_name)
- ZEND_ARG_INFO(0, capped)
- ZEND_ARG_INFO(0, capped_size)
- ZEND_ARG_INFO(0, max_elements)
ZEND_END_ARG_INFO()
@bjori Owner
bjori added a note

Thats still wrong... but since the rest of the arguemtsn are optional I guess it doesn't break anything?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
MONGO_ARGINFO_STATIC ZEND_BEGIN_ARG_INFO_EX(arginfo_dropCollection, 0, ZEND_RETURN_VALUE, 1)
View
42 tests/generic/database-createcollection-with-size-and-max-old.phpt
@@ -0,0 +1,42 @@
+--TEST--
+Database: Create collection with max size and items (old)
+--SKIPIF--
+<?php require_once "tests/utils/standalone.inc"; ?>
+--FILE--
+<?php
+require_once "tests/utils/server.inc";
+
+$a = mongo_standalone();
+$d = $a->selectDb("phpunit");
+
+// cleanup
+$d->selectCollection("createcol1")->drop();
+
+$ns = $d->selectCollection('system.namespaces');
+var_dump($ns->findOne(array('name' => 'phpunit.createcol1')));
+
+// create
+$c = $d->createCollection('createcol1', true, 1000, 5);
+$retval = $ns->findOne(array('name' => 'phpunit.createcol1'));
+var_dump($retval["name"]);
+
+// test cap
+for ($i = 0; $i < 10; $i++) {
+ $c->insert(array('x' => $i), array("safe" => true));
+}
+foreach($c->find() as $res) {
+ var_dump($res["x"]);
+}
+var_dump($c->count());
+?>
+--EXPECTF--
+NULL
+
+%s: MongoDB::createCollection(): This method now accepts arguments as an options array instead of the three optional arguments for capped, size and max elements in %sdatabase-createcollection-with-size-and-max-old.php on line 14
+string(18) "phpunit.createcol1"
+int(5)
+int(6)
+int(7)
+int(8)
+int(9)
+int(5)
View
2  tests/generic/database-createcollection-with-size-and-max.phpt
@@ -16,7 +16,7 @@ $ns = $d->selectCollection('system.namespaces');
var_dump($ns->findOne(array('name' => 'phpunit.createcol1')));
// create
-$c = $d->createCollection('createcol1', true, 1000, 5);
+$c = $d->createCollection('createcol1', array('capped' => true, 'size' => 1000, 'max' => 5));
$retval = $ns->findOne(array('name' => 'phpunit.createcol1'));
var_dump($retval["name"]);
View
48 tests/generic/database-createcollection-with-size-old.phpt
@@ -0,0 +1,48 @@
+--TEST--
+Database: Create collection with max size (old)
+--SKIPIF--
+<?php require_once "tests/utils/standalone.inc"; ?>
+--FILE--
+<?php
+require_once "tests/utils/server.inc";
+$a = mongo_standalone();
+$d = $a->selectDb("phpunit");
+$ns = $d->selectCollection('system.namespaces');
+
+// cleanup
+$d->dropCollection('create-col1');
+var_dump($ns->findOne(array('name' => 'phpunit.create-col1')));
+
+// create
+// * even though we're only setting this to 100, it allocates 1 extent, so we
+// can fit 4096, not 100, bytes of data in the collection.
+
+$c = $d->createCollection('create-col1', true, 100);
+var_dump($ns->findOne(array('name' => 'phpunit.create-col1')));
+
+// test cap
+for ($i = 0; $i < 100; $i++) {
+ $c->insert(array('x' => $i));
+}
+var_dump($c->count());
+var_dump($c->count() < 100);
+?>
+--EXPECTF--
+NULL
+
+%s: MongoDB::createCollection(): This method now accepts arguments as an options array instead of the three optional arguments for capped, size and max elements in %sdatabase-createcollection-with-size-old.php on line %d
+array(2) {
+ ["name"]=>
+ string(19) "phpunit.create-col1"
+ ["options"]=>
+ array(3) {
+ ["create"]=>
+ string(11) "create-col1"
+ ["size"]=>
+ int(100)
+ ["capped"]=>
+ bool(true)
+ }
+}
+int(%d)
+bool(true)
View
2  tests/generic/database-createcollection-with-size.phpt
@@ -17,7 +17,7 @@ var_dump($ns->findOne(array('name' => 'phpunit.create-col1')));
// * even though we're only setting this to 100, it allocates 1 extent, so we
// can fit 4096, not 100, bytes of data in the collection.
-$c = $d->createCollection('create-col1', true, 100);
+$c = $d->createCollection('create-col1', array('size' => 100, 'capped' => true));
var_dump($ns->findOne(array('name' => 'phpunit.create-col1')));
// test cap
View
2  tests/generic/database-profiling-collection-drop.phpt
@@ -9,7 +9,7 @@ $a = mongo_standalone();
$d = $a->selectDb("phpunit");
$ns = $d->selectCollection('system.namespaces');
-$sp = $d->createCollection("system.profile", true, 5000);
+$sp = $d->createCollection("system.profile", array('size' => 5000, 'capped' => true));
var_dump($ns->findOne(array('name' => 'phpunit.system.profile')));
View
2  tests/generic/database-profiling-level.phpt
@@ -7,7 +7,7 @@ Database: Profiling (turning on and off)
require_once "tests/utils/server.inc";
$a = mongo_standalone();
$d = $a->selectDb("phpunit");
-$sp = $d->createCollection("system.profile", true, 5000);
+$sp = $d->createCollection("system.profile", array('capped' => true, 'size' => 5000));
$prev = $d->setProfilingLevel(MongoDB::PROFILING_ON);
$level = $d->getProfilingLevel();
View
5 tests/no-servers/bug00270-arginfo.phpt
@@ -112,11 +112,8 @@ MongoDB
1: backup_original_files (optional)
Method selectCollection expects 1 parameters
0: collection_name
- Method createCollection expects 4 parameters
+ Method createCollection expects 1 parameters
0: collection_name
- 1: capped (optional)
- 2: capped_size (optional)
- 3: max_elements (optional)
Method dropCollection expects 1 parameters
0: collection_name
Method listCollections expects 1 parameters
Something went wrong with that request. Please try again.