I know this was already the case before your PR, but the precedence looks wrong to me:
the DSN should have higher precedence over options (I just double-checked, that's how the Cache component does also).

/cc@jderusse maybe we should review all DSN parsing logic to ensure the precedence is consistent everywhere
/cc@Nyholm that's a good case forthe DSN component - formalizing this behavior :)

👍 not only in cache and lock.

messenger Doctrine:

$configuration +=$options +$query +static::DEFAULT_OPTIONS;

messenger Redis:

parse_str($parsedUrl['query'],$options);

messenger Sqs

$configuration = ['buffer_size'] =$options['buffer_size'] ?? (int) ($query['buffer_size'] ??self::DEFAULT_OPTIONS['buffer_size']),

messenger amqp

$amqpOptions =array_replace_recursive([],$options,$parsedQuery);

Bad news, we need to fix this. Up for a PR?

@nicolas-grekas In order to change the precedence, I think we should detectdsn /options where a precedence change would cause an effective option change and insteadtrigger_deprecation for now. Then swap the precedence insymfony6.0. Do you want me to add this detection /trigger_deprecation in this PR forLock\Store\MongoDbStore only or should we make a list of ALLdsn /options precedence affected classes and do them in a separate PR?

I've added the precedence change since you guys seem to be classing this as a bug rather than a deprecation in:#37268

There is no way to deprecate this behavior properly. Also this will very unlikely hit anyone. This precedence issue lessens the capabilities of the ops side of things. Basically, having the options take over the DSN means that the source needs to change (the options) instead of some env vars. That's a management issue at this point, and this change fixes a process issue: the env vars should always win to give control to the ops, which are the ones that know better since they're the ones in touch with the effective infra.

TL;DR, this is a bugfix.

$mongo is not defined

fixed

I'm not comfortable with this method, a lot of complex logic to extract parameter from URI mixing parse_str and strpos.

Givent the format of the initialDSN is defined by symfony, wouldn't it be safer to build the mongo URI instead?

the reason i chose to use strpos to re-assemble was to simplify the re-assembly. there is no reverse of parse_str in php. seehttps://www.php.net/manual/en/function.parse-url.php#106731

I'm trying not to get too involved with touching the URI. What would you suggest that is simpler?

@kralos: I only just came across this PR based on a notification in#37180. I understand what this function is attempting to do, but usingparse_str may silently corrupting a connection string with repeatedreadPreferenceTags keys (a valid use case, as explained in theURI options spec).This comment on PHP.net goes into more detail about that.

I think it would be preferable to capture the URI option with a regular expression and then, if anything was found, strip it from the returned string. While collection names have theirown restrictions, for purposes of URI parsing I think it'd be best to use something like/collection=([^&]*)/i.

see#37864 /#37868