Allow any any options for connections in Configuration #218
Conversation
| ->scalarNode('db')->end() | ||
| ->end() | ||
| ->end() | ||
| ->variableNode('options')->defaultValue(array())->end() |
There was a problem hiding this comment.
this is a bad idea. It should be a prototyped array node if you want to allow any option, not a variable node. You don't want to allow anything else than arrays for options.
and using a variable will be very unfriendly to XML users, as it is not possible to define an associative array in XML (Symfony has a way to deal with it for prototyped array nodes).
the rule of thumbs is: if you start using a variable node, think twice about it. Most of the time, it is a bad choice.
There was a problem hiding this comment.
Thanks for pointing out issue with XML, I wasn't aware of that. Basically I tried to find how to allow any key in array but with no luck so I followed jmikola comment about VariableNode #203 (comment) - I'll look after prototyped array node
There was a problem hiding this comment.
@stof is this what you've meant?
->arrayNode('options')
->useAttributeAsKey('name')
->prototype('scalar')->end()
->end()
|
@stof: I suppose in this case (using array of scalar prototypes), we leave XML users with no way to define non-scalar options for MongoClient::construct()'s options. @malarzm: On second thought, would you be willing to revise the configuration schema to support all of the existing options? I would suggest supporting everything listed on the php.net documentation (even deprecated options for now), and removing the |
|
No problem, I'll do it tomorrow |
|
Generally speaking done. readPreferenceTags is variableNode at the moment but as soon as I'll figure out how to use arrayNode and remove it if it's empty I'll change it :) |
Changed VariableNode to prototyped ArrayNode Listing all options for connection in Configuration Removed \MongoClient consts Changed readPreferenceTags to arrayNode
|
readPreferenceTags is arrayNode now |
|
@malarzm: Thank you. I'll take a look at this later today. |
| ->arrayNode('readPreferenceTags') | ||
| ->performNoDeepMerging() | ||
| ->prototype('array') | ||
| ->prototype('scalar')->end() |
There was a problem hiding this comment.
is it really an array of array of scalars ?
There was a problem hiding this comment.
hmm, according to the tests, the inner array seems to be associative, so you need a call to getAttributeAsKey('...') (replace ... by the name of the XML attribute used to pass the key)
|
The xml fixture file for the full config should be added IMO, so that the normalization of XML is covered as well. It would have catched some of the issues in the config tree here and given that the bundle provide an XSD schema, it should be updated as well, otherwise XML will still be unsupported |
|
Life would be much easier without XML configs ;) Anyway I'll create full.xml, add it to test and adjust XSD. I think I'll have problems with thenUnset() thing as I've tried setting it on almost every level which made sense but I'll give it another shot (at some moment it was almost working but not setting options at all blew everything - that's why it has landed in its current place) |
|
full.xml is added and passing tests, XSD is slightly updated. |
|
@malarzm: I'm going through this now. |
Fixes #215 (which points to #214 and #203).