Consider exporting parseDSN

[xaprb]

I've just been writing some tests for an application that uses an environment variable to get its configuration, and one of the variables is a DSN. Part of my test is designed to try making a connection and handle the failures gracefully. The test itself also gets its config from the env var, which we set on CircleCI for example to connect to the database instance there. For these reasons it would be really helpful if my test could easily parse DSNs (maybe it's not obvious why, but that's the end result). For now I'm going to copy/paste parseDSN into my test file, but I do think this would be useful to export.

[arnehormann]

Hi @xaprb, we are already considering that.
Our plan (without a timeline so far) is do export next to everything in a new low-level package and call that from the "old" driver. @julienschmidt hinted at that in #221. We still have to talk over the details - and one of us has to take the time and do it. But it will happen.
This will also help with #66, #157, #178, #180.

[julienschmidt]

My plan is to release v1.2 of the driver soon and then try moving the wire / protocol level stuff into a sub-package. No guarantees, that this is successful.

At this point, I don't think the parseDSN function would be part of it. As you said yourself, it isn't obvious why it is helpful to be able to parse the DSN. There might be only very few corner cases where this is useful. Therefore I don't think parseDSN should be exported, but you can convince me otherwise 😉

[arnehormann]

I should have read our conversation history again, then.
You are right, the new package should only contain stuff used in the wire protocol to keep the import surface minimal for its users.

Still, I can see why exporting the dsn parsing would be useful.
It should probably be exported in the driver or another package, then.

[julienschmidt]

Giving this another thought, I don't think this is going to happen anytime soon.
Exporting parseDSN would also require us to export the config struct. But this struct changes a lot and we don't want external access on it.

[arnehormann]

Or we make it an exported function with an obscene amount of return-values; returning each parameter and a map? I can see the need for it, it's probably pretty tedious and feels brittle to track our implementation.

[julienschmidt]

Or we make it an exported function with an obscene amount of return-values; returning each parameter and a map?

Then the function signature would be changing...

[arnehormann]

I thought about this some more:

What I propose below should be doable, stable and still helpful. At least it removes a heap of parsing if users of the driver replicate its functionality.
The map contents may still fluctuate, but this should already be flexible enough.
The JSON struct tags are nice but optional.

type Dsn struct {
    User string `json:"user"`
    Pass string `json:"password,omitempty"`
    Proto string `json:"protocol"`
    Addr string `json:"address"`
    Schema string `json:"schema,omitempty"`
    Params map[string]string `json:"parameters,omitempty"`
}

func (d *Dsn) Parse(dsn string) error {
    //...
}

Making Parse a function on a pointer receiver means we can embed Dsn in config, create a config and fill it with &(config.Dsn).Parse(dsn) - the fields are still protected from outside meddling as config itself is not exported.

Users can call Parse on a &Dsn{} themselves to get the exported values.

The only con I see is memory usage - and negligible in my opinion: values in the map are duplicated in config, one time in Params, one time in the parsed form (loc, ...).

If desired (and supported by an interface Config to make it accessible from outside), copies are extractable from a config like this - due to Go's pass-by-value semantics for non-pointer values:

func (c *config) Dsn() *Dsn {
    tmp := *(c.Dsn)
    tmp.Params = make(map[string]string, len(c.Params))
    for k, v := range c.Params {
        tmp.Params[k] = v
    }
    return &tmp
}