Introduce StringValues to replace string[] usage.

[Tratcher]

#361

Open issues:

• Name and location of the package containing StringValues. It needs to make sense to reference even from Microsoft.Net.Http.Server (WebListener's low level API without Asp.Net dependencies). Microsoft.AspNet.Primitives is just a placeholder.

Related breaking changes:

• IHeaderDictionary:
  • Removal of AppendValues, Get_, and Set_
  • Append now functions like AppendValues instead of AppendCommaSeperatedValues.
• IReadableStringCollection
  • Removal of Get*, use the indexer instead.

Here are some of the necessary changes in downstream repos. Please use them only as references to evaluate this PR, lets avoid full reviews on them until we have settled open issues on this one.

• aspnet/Hosting@49b8b29
• https://github.com/aspnet/Helios/commit/724cb5823199d93987a4cb97f8fdfffbb498bde4
• aspnet/HttpSysServer@d858bf7
• aspnet/KestrelHttpServer@2abc406
• aspnet/Security@25af3ee
• aspnet/Mvc@6331a9c

@lodejard @davidfowl @muratg @rynowak @halter73 @Eilon

[dougbu]

@Tratcher note aspnet/Mvc@6331a9c needs to be updated now that @rynowak has pushed aspnet/Mvc@6d365e9

[dougbu]

??

[Tratcher]

@dougbu updated: aspnet/Mvc@b42d501

[davidfowl]

Why?

[Tratcher]

It's an http header with 0-many values.

[Tratcher]

Offline review notes:

Rename AspNet.Primitives -> Framework.Primitives

Add StringValues.ToArray() for explicit typing (instead of casting), mirrors ToString

IHeaderDictionary:

• Move to extension methods:
  • StringValues GetCommaSeparatedValues(string key); - Return IList<string>
  • void Append(string key, StringValues value);
  • void AppendCommaSeparatedValues(string key, params string[] values);
  • void SetCommaSeparatedValues(string key, params string[] values);

[davidfowl]

I think this usage shows some of the benefits of the API. Checking for null or !any is pretty annoying.

[lodejard]

❤️ IsNullOrEmpty

[Tratcher]

Updated.

[Tratcher]

@davidfowl @lodejard Can we get this wrapped up today so we can start pushing it through the other repos?

[lodejard]

This method should probably check for _values == null instead of _value != null. In all of the other methods the _values being null or not is the decision maker

[Tratcher]

In this case you still need to check if _value != null because you don't want to do return new[] { _value }; with a null _value:

            if (_values == null)
            {
                if (_value != null)
                {
                    return new[] { _value };
                }
            }
            return _values;

or

            if (_value == null)
            {
                return _values;
            }
            return new[] { _value };

or

            if (_value != null)
            {
                return new[] { _value };
            }
            return _values;

[lodejard]

I dunno, I'd be really nervous about having one method with a different logical order to determine state. It's if values !=null then values, else if value!=null then string[1]{value}, else string[0]. See also Count property. A comment next to the fields would have been nice, sorry about that :/

Tbh that means the string ctor could really detect "null" and assign values to a string[1]{null} instead.

That said, it can be refined after the merge, so treat this like a followup remark and keep running with the shipit

[Tratcher]

Offline from @lodejard

[davidfowl]

it is time

[lodejard]

Finally found some cellular data!