Include SharedAccessSignature in SB & EH connection string model

[ramya-rao-a]

#11893 and #11894 are issues requesting a parser for Service Bus and Event Hubs connection strings.
We already have a method parseConnectionString<T>(connectionString): T exposed from @azure/core-amqp that does something like this. The differences are

• This method uses a type variable. The user is expected to pass either EventHubConnectionStringModel or ServiceBusConnectionStringModel or any type of their choice. If a user does not pass any type variable, they get a json object representing all the key value pairs in the connection string, but no help on the names of the keys.
• The models allow more than the restricted set of values listed in Implement a Service Bus Connection String Parser #11893 and Implement an Event Hubs Connection String Parser #11894
• The models reflect the keys in the connection string as is and does not use the custom names like FullyQualifiedNamespace or EventHubName
• The method does not do any validation listed in Implement a Service Bus Connection String Parser #11893 and Implement an Event Hubs Connection String Parser #11894

The main motivation behind the issues that were logged is to assist with transforming a connection string for use with credential-based client creation. With this PR, the parseConnectionString method along with the EH & SB connection string models are good enough for the time being, though not perfect.

@richardpark-msft, @HarshaNalluru, @bterlson, @jsquire, Thoughts?

[ramya-rao-a]

Looks like the catch is going to be in making the SharedAccessKey and SharedAccessKeyName optional in the EH & SB connection string models. That would be a breaking change for core-amqp

[HarshaNalluru]

Should we be exposing parseConnectionString methods from service-bus and event-hubs separately?
Which would be

export const parseSBConnectionString = (connectionString: string) =>
  parseConnectionString<ServiceBusConnectionStringModel>(connectionString);

This way, users would also get the type info if they use parseSBConnectionString method.

[ramya-rao-a]

I would do that if the parseSBConnectionString method would then follow all the requirements logged in #11893 and #11894. If we are going with an intermediate offering in the meantime, then I would keep this as is in core-amqp

Also, this might be a good opportunity to gather all our core-amqp v2 requirements and see if we want to make the jump

[richardpark-msft]

The other benefit of what @HarshaNalluru 's suggesting is that we can slim down the interface that we return since, as you've pointed out, the current interfaces have the [x: string]: any; which I'd rather not be on the hook for in the future.

[jsquire]

• The models allow more than the restricted set of values listed in Implement a Service Bus Connection String Parser #11893 and Implement an Event Hubs Connection String Parser #11894
• The models reflect the keys in the connection string as is and does not use the custom names like FullyQualifiedNamespace or EventHubName
• The method does not do any validation listed in Implement a Service Bus Connection String Parser #11893 and Implement an Event Hubs Connection String Parser #11894

I've no objections to writing this off as a language idiom, if that's the will of the JS team. The intent behind the issues is:

• Ensure that the connection string parsed is a well-formed and valid connection string for the service. If the client would reject it, the parse method should likewise reject it. Without the specifics of the domain logic, the value of having the parser in each client library is weakened; it would potentially be better as a general-purpose parser in the Core library.

• Ensure that the names from the output of the parser map to the input of creating the clients. As a caller, knowing that I have to translate EntityPath to EventHubName isn't overly obvious, for example. Knowing that you need to parse the host from the EndpointAddress and call it FullyQualifiedNamespace is even less obivous.

[ramya-rao-a]

Thanks all

It does look like there are limitations to the parseConnectionString in core-amqp that we would not want to tie ourselves to.
Closing in favor of #11949