PHPLIB-1348 Add tests on Custom Aggregation Expression Operators #35
Conversation
| $this->init = $init; | ||
| if (is_string($accumulate)) { | ||
| $accumulate = new Javascript($accumulate); |
There was a problem hiding this comment.
The MongoDB\BSON\Javascript constructor accepts a 2nd parameter $scope. I don't think we need to set a value.
There was a problem hiding this comment.
The scope has been deprecated since 4.4 (SERVER-45460), so no need to support it.
There was a problem hiding this comment.
Thanks, that's what I just concluded by reading the list of types.
Automatically convert string into Javascript BSON
| @@ -80,6 +80,8 @@ private function addMethod(GeneratorDefinition $definition, OperatorDefinition $ | |||
| } else { | |||
| if ($argument->optional) { | |||
| $parameter->setDefaultValue(new Literal('Optional::Undefined')); | |||
| } elseif ($argument->default !== null) { | |||
| $parameter->setDefaultValue($argument->default); | |||
There was a problem hiding this comment.
Is this the first time this logic is required? The default value in the schema was previously "string", "number", "boolean" so I would have expected some handling for this already.
There was a problem hiding this comment.
This was a bug. The default value was set on the operator class contructor, but not on the factory method. There is duplicated code between both generators.
| @@ -64,14 +64,29 @@ tests: | |||
| avgCopies: | |||
| $accumulator: | |||
| init: | |||
| $code: 'function () { return { count: 0, sum: 0 } }' | |||
| $code: |- | |||
There was a problem hiding this comment.
Noted that this has no impact on tests/Builder/Accumulator/Pipelines.php apart from whitespace changes. Likewise for query/where.yaml.
Fix PHPLIB-1348
I updated the operators accepting
javascripttype, to automatically convert strings into aMongoDB\BSON\Javascriptobject.The
$functionoperator has default values forargs: []andlang: 'js', because this parameters are required. The lang is the only one supported.https://www.mongodb.com/docs/manual/reference/operator/aggregation/#custom-aggregation-expression-operators
$accumulatorexample updated to use automatic creation ofJavascriptobjects from string$function$wherefixed examples to beJavascriptobjects