Adding validation rules for subscriptionId/api-version params

[dsgouda]

Adding validation rules to enforce:

• Parameters section for operations should not define the subscriptionId and api-version parameters
• Service definition section must contain parameters api-version and subscriptionId

Modified all swagger validation test jsons to adhere to these validation rules
Fixing issue #1515

@tbombach

[fearthecowboy]

If you're changing JSON files, make sure you pull all the latest code and do a gulp regenerate:expected test and then add in any changed files with a separate commit message 👍 #Resolved

[fearthecowboy]

Parameters "subscriptionId" and "api-version" are not allowed in the operations section ? #Resolved

[fearthecowboy]

Braces please. 😀

if (ParametersMap.Count==0 || ParametersMap == null)
{
    return false;
}

[fearthecowboy]

The code

var Names = ParametersMap.Values.Select(Param => Param.Name).ToList();
return Names.Contains(SubscriptionId) && Names.Contains(ApiVersion);

is iterating the ParametersMap.Values enumerable and allocating a List for no benefit.
.ToList() should be reserved for when you actually need to manipulate the result set of an enumeration.

The following will iterate the set only once, and will return at the first true case. If there are a 1000 items in the list, and the first is matched, the iterating stops there. The happy path is the fast path ⏩

return ParametersMap.Values.Any( parameter => parameter.Name == SubscriptionId  || parameter.Name == ApiVersion );

[dsgouda]

This helps my limited knowledge of Linq 😄

[fearthecowboy]

Actually, you don't want to check the count; that presumes either that the dictionary implementation knows the actual item count (which not all do), or you're forcing the dictionary to count the items. The iteration below will be nearly 0 time if the collection is empty anyway.

So replace this with

if( ParametersMap == null ) 
{ 
    return false;
}

[fearthecowboy]

Now, I get to the bottom and see that you could simplify it a bit more, and use a single expression that leverages null-propagation operator (which turns the expression result into a nullable bool result bool? which can be trivially coaxed back into a bool by adding true == at the front of the expression.

  public override bool IsValid(Dictionary<string, SwaggerParameter> ParametersMap) => 
    true == ParametersMap?.Values.Any( parameter => parameter.Name == SubscriptionId  || parameter.Name == ApiVersion );

[fearthecowboy]

rather than performing the separate checks on the Parameter and Parameter.Name, you can use the null-propagation operator to fold it into a single check and let the compiler to the heavy lifting.

public override bool IsValid(SwaggerParameter Parameter) => 
    SubscriptionId  != Parameter?.Name && 
    ApiVersion != Parameter?.Name;

[dsgouda]

New concepts, taking 🗒

[fearthecowboy]

LGTM 👍

[dsgouda]

@azuresdkci retest this please