[GODRIVER-1498] Refactor URI parsing to use pointers Created: 27/Feb/20  Updated: 08/Jan/24

Status: Backlog
Project: Go Driver
Component/s: Options & Configuration
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Major - P3
Reporter: David Bartley Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
is depended on by TOOLS-2585 Simplify normalization logic for URI ... Accepted
Epic Link: GODRIVER-1588
Quarter: FY25Q1
Documentation Changes Summary:

1. What would you like to communicate to the user about this feature?
2. Would you like the user to see examples of the syntax and/or executable code and its output?
3. Which versions of the driver/connector does this apply to?


 Description   

New Description: The URI parser uses two variables per option: one for the value and one for determining if the option is set. This code that uses the parser is inconsistent. Sometimes, it checks the *Set variant but for some boolean options that are only useful if set to true (e.g. SSL), it will directly check cs.SSL rather than cs.SSLSet && cs.SSL. Let's refactor this to use pointers like the rest of our options code for consistency.

 

Previous Description: Pretty minor, but ConnString.SSLSet is never read by anything. I believe https://github.com/mongodb/mongo-go-driver/blob/a96725e2d7cb39d0c31f7f6605305ad3f92787c1/x/mongo/driver/topology/topology_options.go#L112 should actually be "if cs.SSLSet && cs.SSL {".



 Comments   
Comment by Divjot Arora (Inactive) [ 27/Feb/20 ]

bartle Thanks for the quick reply. I've edited the ticket title and description to reflect the work that should be done.

Comment by David Bartley [ 27/Feb/20 ]

Yes, that seems like a great approach.

Comment by Divjot Arora (Inactive) [ 27/Feb/20 ]

bartle I'd like to do a larger piece of work and re-write the connstring parser to use pointers rather than having Option and OptionSet for each URI option. With the way it is now, it's easy to be inconsistent in code that uses the connection string. For options that default to false but are only useful when set to true (like SSL/SSLInsecure), we often don't check *Set, which isn't incorrect, but it is inconsistent.

 

Do you mind if I re-purpose this ticket into an improvement for the work I described?

Comment by David Bartley [ 27/Feb/20 ]

SSLInsecureSet is similarly never read.

Comment by David Bartley [ 27/Feb/20 ]

Here too: https://github.com/mongodb/mongo-go-driver/blob/a96725e2d7cb39d0c31f7f6605305ad3f92787c1/mongo/options/clientoptions.go#L268

Generated at Thu Feb 08 08:36:31 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.