Skip to content

Commit

Permalink
Updating validation rule BodyTopLevelProperties to reuse resource def…
Browse files Browse the repository at this point in the history
…inition (#2152)
dsgouda authored Apr 18, 2017

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
1 parent daae3cf commit 204c91d
Showing 6 changed files with 45 additions and 84 deletions.
2 changes: 1 addition & 1 deletion src/core/AutoRest.Core/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/core/AutoRest.Core/Properties/Resources.resx
Original file line number Diff line number Diff line change
@@ -308,7 +308,7 @@ Modelers:
<value>Booleans are not descriptive and make them hard to use. Instead use string enums with allowed set of values defined.</value>
</data>
<data name="AllowedTopLevelProperties" xml:space="preserve">
<value>Top level properties should be one of name, type, id, location, properties, tags, plan, sku, etag, managedBy, identity. Extra properties found: "{0}".</value>
<value>Top level properties should be one of name, type, id, location, properties, tags, plan, sku, etag, managedBy, identity. Model definition '{0}' has extra properties ['{1}'].</value>
</data>
<data name="TrackedResourceGetOperationMissing" xml:space="preserve">
<value>Tracked resource '{0}' must have a get operation.</value>
Original file line number Diff line number Diff line change
@@ -28,17 +28,7 @@
"in": "body",
"required": true,
"schema": {
"type": "object",
"allOf": [
{
"$ref": "#/definitions/Resource"
}
],
"properties": {
"extraprop": {
"type": "string"
}
}
"$ref": "#/definitions/ParameterWithExtraProperty"
},
"description": "The parameters to provide for the updated account."
}
@@ -99,6 +89,14 @@
"type": "string"
},
"description": "Gets or sets a list of key value pairs that describe the resource. These tags can be used in viewing and grouping this resource (across resource groups). A maximum of 15 tags can be provided for a resource. Each tag must have a key no greater than 128 characters and value no greater than 256 characters."
},
"properties": {
"extraProperty1": {
"description": "a property inside the props bag",
"schema": {
"type": "string"
}
}
}
},
"required": [
@@ -144,20 +142,20 @@
"x-ms-azure-resource": true
}
},
"parameters": {
"SubscriptionIdParameter": {
"name": "subscriptionId",
"in": "path",
"required": true,
"type": "string",
"description": "test subscription id"
},
"ApiVersion": {
"name": "api-version",
"in": "path",
"required": true,
"type": "string",
"description": "test api version"
}
"parameters": {
"SubscriptionIdParameter": {
"name": "subscriptionId",
"in": "path",
"required": true,
"type": "string",
"description": "test subscription id"
},
"ApiVersion": {
"name": "api-version",
"in": "path",
"required": true,
"type": "string",
"description": "test api version"
}
}
}
}
Original file line number Diff line number Diff line change
@@ -230,7 +230,7 @@ public void CollectionObjectsPropertiesNamingValidation()
public void BodyTopLevelPropertiesValidation()
{
var messages = ValidateSwagger(Path.Combine(Core.Utilities.Extensions.CodeBaseDirectory, "Resource", "Swagger", "Validation", "body-top-level-properties.json"));
messages.AssertOnlyValidationMessage(typeof(BodyTopLevelProperties), 2);
messages.AssertOnlyValidationMessage(typeof(BodyTopLevelProperties), 1);
}

[Fact]
2 changes: 1 addition & 1 deletion src/modeler/AutoRest.Swagger/Model/ServiceDefinition.cs
Original file line number Diff line number Diff line change
@@ -87,7 +87,6 @@ public ServiceDefinition()
[Rule(typeof(OperationsAPIImplementationValidation))]
[Rule(typeof(ProvidersPathValidation))]
[Rule(typeof(PutResponseResourceValidation))]
[CollectionRule(typeof(BodyTopLevelProperties))]
[CollectionRule(typeof(HttpVerbValidation))]
[CollectionRule(typeof(DeleteMustNotHaveRequestBody))]
[CollectionRule(typeof(BodyPropertiesNamesCamelCase))]
@@ -125,6 +124,7 @@ public ServiceDefinition()
[Rule(typeof(TrackedResourcePatchOperationValidation))]
[Rule(typeof(DescriptionMissing))]
[Rule(typeof(PatchBodyParametersSchemaValidation))]
[Rule(typeof(BodyTopLevelProperties))]
[CollectionRule(typeof(RequiredReadOnlyPropertiesValidation))]
public Dictionary<string, Schema> Definitions { get; set; }

69 changes: 16 additions & 53 deletions src/modeler/AutoRest.Swagger/Validation/BodyTopLevelProperties.cs
Original file line number Diff line number Diff line change
@@ -11,12 +11,12 @@

namespace AutoRest.Swagger.Validation
{
public class BodyTopLevelProperties : TypedRule<Dictionary<string, Operation>>
public class BodyTopLevelProperties : TypedRule<Dictionary<string, Schema>>
{

private readonly Regex resourceRefRegEx = new Regex(@".+/Resource$", RegexOptions.IgnoreCase);
private readonly string[] allowedTopLevelProperties = { "name", "type", "id", "location", "properties", "tags", "plan", "sku", "etag",
"managedBy", "identity", "kind"};
private static readonly IEnumerable<string> AllowedTopLevelProperties = new List<string>()
{ "name", "type", "id", "location", "properties", "tags", "plan", "sku", "etag",
"managedBy", "identity", "kind"};

/// <summary>
/// Id of the Rule.
@@ -28,62 +28,25 @@ public class BodyTopLevelProperties : TypedRule<Dictionary<string, Operation>>
/// </summary>
public override ValidationCategory ValidationCategory => ValidationCategory.RPCViolation;


/// <summary>
/// This rule passes if the body parameter contains top level properties only from the allowed set: name, type,
/// id, location, properties, tags, plan, sku, etag, managedBy, identity
/// </summary>
/// <param name="paths"></param>
/// <returns></returns>
public override bool IsValid( Dictionary<string, Operation> path, RuleContext context, out object[] formatParameters)
/// <param name="definitions">The model definitions</param>
/// <param name="context">The context object</param>
/// <returns>validation messages</returns>
public override IEnumerable<ValidationMessage> GetValidationMessages(Dictionary<string, Schema> definitions, RuleContext context)
{
List<string> notAllowedProperties = new List<string>();
foreach (string operation in path.Keys)
var resModels = context.ResourceModels;
var violatingModels = resModels.Where(resModel => definitions[resModel].Properties?.Keys.Except(AllowedTopLevelProperties).Any() == true);
foreach (var violatingModel in violatingModels)
{
if ((operation.ToLower().Equals("get") ||
operation.ToLower().Equals("put") ||
operation.ToLower().Equals("patch")) && path[operation]?.Parameters != null)
{
foreach (SwaggerParameter param in path[operation].Parameters)
{
if (param.In == ParameterLocation.Body)
{
if (param?.Schema?.Reference != null)
{
string defName = Extensions.StripDefinitionPath(param.Schema.Reference);
var definition = ((ServiceDefinition)context.Root).Definitions[defName];
if (definition?.AllOf != null && definition.Properties != null &&
definition.AllOf.Select(s => s.Reference).Where(reference => resourceRefRegEx.IsMatch(reference)) != null)
{
//Model is allOf Resource
foreach (KeyValuePair<string, Schema> prop in definition.Properties)
{
if (!allowedTopLevelProperties.Contains(prop.Key.ToLower()))
{
notAllowedProperties.Add(defName + "/" + prop.Key);
}
}
}
}
if (param?.Schema?.AllOf != null && param.Schema.Properties != null &&
param.Schema.AllOf.Select(s => s.Reference).Where(reference => resourceRefRegEx.IsMatch(reference)) != null)
{
//Model is allOf Resource
foreach (KeyValuePair<string, Schema> prop in param.Schema.Properties)
{
if (!allowedTopLevelProperties.Contains(prop.Key.ToLower()))
{
notAllowedProperties.Add(path[operation].OperationId + ":" + prop.Key);
}
}
}
}
}
}
}
formatParameters = new[] { string.Join(", ", notAllowedProperties.ToArray()) };
return (notAllowedProperties.Count() == 0);
yield return new ValidationMessage(new FileObjectPath(context.File, context.Path.AppendProperty(violatingModel).AppendProperty("properties")), this,
violatingModel, string.Join(",", definitions[violatingModel].Properties.Keys.Except(AllowedTopLevelProperties)));
}
}

/// <summary>
/// The template message for this Rule.
/// </summary>

0 comments on commit 204c91d

Please sign in to comment.