Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix temp parameters duplicate issue + Allow relative URIs where appropriate #197

Merged
merged 6 commits into from
Feb 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@ public static class JsonPointerExtensions
/// <summary>
/// Finds the YAML node that corresponds to this JSON pointer based on the base YAML node.
/// </summary>
public static YamlNode Find(this JsonPointer currentpointer, YamlNode baseYamlNode)
public static YamlNode Find(this JsonPointer currentPointer, YamlNode baseYamlNode)
{
if (currentpointer.Tokens.Length == 0)
if (currentPointer.Tokens.Length == 0)
{
return baseYamlNode;
}

try
{
var pointer = baseYamlNode;
foreach (var token in currentpointer.Tokens)
foreach (var token in currentPointer.Tokens)
{
var sequence = pointer as YamlSequenceNode;

Expand All @@ -47,9 +47,9 @@ public static YamlNode Find(this JsonPointer currentpointer, YamlNode baseYamlNo

return pointer;
}
catch (Exception ex)
catch (Exception)
{
throw new ArgumentException("Failed to dereference pointer", ex);
return null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still a little nervous about this change, but let's try it and see if it causes any issues.

}
}
}
Expand Down

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

3 changes: 3 additions & 0 deletions src/Microsoft.OpenApi.Readers/Properties/SRResource.resx
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@
<data name="CannotResolveRemoteReferencesSynchronously" xml:space="preserve">
<value>"Cannot resolve remote references automatically in a syncronous call."</value>
</data>
<data name="JsonPointerCannotBeResolved" xml:space="preserve">
<value>JSON pointer '{0}' does not point to an object in the document.</value>
</data>
<data name="LoadReferencedObjectFromExternalNotImplmented" xml:space="preserve">
<value>Not implemented to find referenced element from external resource.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ internal static partial class OpenApiV2Deserializer
{
"url", (o, n) =>
{
o.Url = new Uri(n.GetScalarValue());
o.Url = new Uri(n.GetScalarValue(), UriKind.RelativeOrAbsolute);
}
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ internal static partial class OpenApiV2Deserializer
},
{
"consumes",
(o, n) => n.Context.SetTempStorage("globalconsumes", n.CreateSimpleList(s => s.GetScalarValue()))
(o, n) => n.Context.SetTempStorage(TempStorageKeys.GlobalConsumes, n.CreateSimpleList(s => s.GetScalarValue()))
},
{
"produces",
(o, n) => n.Context.SetTempStorage("globalproduces", n.CreateSimpleList(s => s.GetScalarValue()))
(o, n) => n.Context.SetTempStorage(TempStorageKeys.GlobalProduces, n.CreateSimpleList(s => s.GetScalarValue()))
},
{"paths", (o, n) => o.Paths = LoadPaths(n)},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal static partial class OpenApiV2Deserializer
{
"url", (o, n) =>
{
o.Url = new Uri(n.GetScalarValue());
o.Url = new Uri(n.GetScalarValue(), UriKind.RelativeOrAbsolute);
}
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal static partial class OpenApiV2Deserializer
{
"termsOfService", (o, n) =>
{
o.TermsOfService = new Uri(n.GetScalarValue());
o.TermsOfService = new Uri(n.GetScalarValue(), UriKind.RelativeOrAbsolute);
}
},
{
Expand Down
32 changes: 20 additions & 12 deletions src/Microsoft.OpenApi.Readers/V2/OpenApiOperationDeserializer.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license.

using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.OpenApi.Extensions;
Expand Down Expand Up @@ -58,12 +59,12 @@ internal static partial class OpenApiV2Deserializer
},
{
"consumes", (o, n) => n.Context.SetTempStorage(
"operationconsumes",
TempStorageKeys.OperationConsumes,
n.CreateSimpleList(s => s.GetScalarValue()))
},
{
"produces", (o, n) => n.Context.SetTempStorage(
"operationproduces",
TempStorageKeys.OperationProduces,
n.CreateSimpleList(s => s.GetScalarValue()))
},
{
Expand Down Expand Up @@ -104,27 +105,33 @@ internal static partial class OpenApiV2Deserializer

internal static OpenApiOperation LoadOperation(ParseNode node)
{
var mapNode = node.CheckMapNode("OpenApiOperation");
// Reset these temp storage parameters for each operation.
node.Context.SetTempStorage(TempStorageKeys.BodyParameter, null);
node.Context.SetTempStorage(TempStorageKeys.FormParameters, null);
node.Context.SetTempStorage(TempStorageKeys.OperationProduces, null);
node.Context.SetTempStorage(TempStorageKeys.OperationConsumes, null);

var mapNode = node.CheckMapNode("Operation");

var operation = new OpenApiOperation();

ParseMap(mapNode, operation, _operationFixedFields, _operationPatternFields);

// Build request body based on information determined while parsing OpenApiOperation
var bodyParameter = node.Context.GetFromTempStorage<OpenApiParameter>("bodyParameter");
var bodyParameter = node.Context.GetFromTempStorage<OpenApiParameter>(TempStorageKeys.BodyParameter);
if (bodyParameter != null)
{
operation.RequestBody = CreateRequestBody(node.Context, bodyParameter);
}
else
{
var formParameters = node.Context.GetFromTempStorage<List<OpenApiParameter>>("formParameters");
var formParameters = node.Context.GetFromTempStorage<List<OpenApiParameter>>(TempStorageKeys.FormParameters);
if (formParameters != null)
{
operation.RequestBody = CreateFormBody(node.Context, formParameters);
}
}

return operation;
}

Expand Down Expand Up @@ -156,9 +163,9 @@ private static OpenApiRequestBody CreateFormBody(ParsingContext context, List<Op
Required = formParameters.Where(p => p.Required).Select(p => p.Name).ToList()
}
};

var consumes = context.GetFromTempStorage<List<string>>("operationconsumes") ??
context.GetFromTempStorage<List<string>>("globalconsumes") ??
var consumes = context.GetFromTempStorage<List<string>>(TempStorageKeys.OperationConsumes) ??
context.GetFromTempStorage<List<string>>(TempStorageKeys.GlobalConsumes) ??
new List<string> {"application/x-www-form-urlencoded"};

var formBody = new OpenApiRequestBody
Expand All @@ -175,8 +182,9 @@ private static OpenApiRequestBody CreateRequestBody(
ParsingContext context,
OpenApiParameter bodyParameter)
{
var consumes = context.GetFromTempStorage<List<string>>("operationconsumes") ??
context.GetFromTempStorage<List<string>>("globalconsumes") ?? new List<string> {"application/json"};
var consumes = context.GetFromTempStorage<List<string>>(TempStorageKeys.OperationConsumes) ??
context.GetFromTempStorage<List<string>>(TempStorageKeys.GlobalConsumes) ??
new List<string> {"application/json"};

var requestBody = new OpenApiRequestBody
{
Expand All @@ -186,7 +194,7 @@ private static OpenApiRequestBody CreateRequestBody(
k => k,
v => new OpenApiMediaType
{
Schema = bodyParameter.Schema // Should we clone this?
Schema = bodyParameter.Schema
})
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ private static void LoadStyle(OpenApiParameter p, string v)
{
switch (v)
{
// TODO: Handle "csv" for query / form parameter. The style should be Form, not Simple.
case "csv":
p.Style = ParameterStyle.Simple;
return;
Expand Down Expand Up @@ -197,8 +198,6 @@ private static void ProcessIn(OpenApiParameter o, ParseNode n)
var value = n.GetScalarValue();
switch (value)
{
// TODO: There could be multiple body/form parameters, so setting it to a global storage
// will overwrite the old parameter. Need to handle this on a per-parameter basis.
case "body":
n.Context.SetTempStorage("bodyParameter", o);
break;
Expand Down
21 changes: 11 additions & 10 deletions src/Microsoft.OpenApi.Readers/V2/OpenApiResponseDeserializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ internal static partial class OpenApiV2Deserializer
{
"schema", (o, n) =>
{
n.Context.SetTempStorage("operationschema", LoadSchema(n));
n.Context.SetTempStorage(TempStorageKeys.ResponseSchema, LoadSchema(n));
}
},
};
Expand All @@ -50,22 +50,21 @@ internal static partial class OpenApiV2Deserializer

private static void ProcessProduces(OpenApiResponse response, ParsingContext context)
{
var produces = context.GetFromTempStorage<List<string>>("operationproduces") ??
context.GetFromTempStorage<List<string>>("globalproduces") ?? new List<string> {"application/json"};
var produces = context.GetFromTempStorage<List<string>>(TempStorageKeys.OperationProduces) ??
context.GetFromTempStorage<List<string>>(TempStorageKeys.GlobalProduces) ?? new List<string> {"application/json"};

response.Content = new Dictionary<string, OpenApiMediaType>();
foreach (var mt in produces)
foreach (var produce in produces)
{
var schema = context.GetFromTempStorage<OpenApiSchema>("operationschema");
OpenApiMediaType mediaType = null;
if (schema != null)
var responseSchema = context.GetFromTempStorage<OpenApiSchema>(TempStorageKeys.ResponseSchema);
if (responseSchema != null)
{
mediaType = new OpenApiMediaType
var mediaType = new OpenApiMediaType
{
Schema = schema
Schema = responseSchema
};

response.Content.Add(mt, mediaType);
response.Content.Add(produce, mediaType);
}
}
}
Expand Down Expand Up @@ -102,6 +101,8 @@ private static void LoadExample(OpenApiResponse response, string mediaType, Pars

public static OpenApiResponse LoadResponse(ParseNode node)
{
node.Context.SetTempStorage(TempStorageKeys.ResponseSchema, null);

var mapNode = node.CheckMapNode("response");

var pointer = mapNode.GetReferencePointer();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ public static OpenApiSecurityRequirement LoadSecurityRequirement(ParseNode node)
else
{
node.Diagnostic.Errors.Add(
new OpenApiError(node.Context.GetLocation(), $"Scheme {property.Name} is not found"));
new OpenApiError(node.Context.GetLocation(),
$"Scheme {property.Name} is not found"));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,14 @@ internal static partial class OpenApiV2Deserializer
"authorizationUrl",
(o, n) =>
{
_flow.AuthorizationUrl = new Uri(n.GetScalarValue());
_flow.AuthorizationUrl = new Uri(n.GetScalarValue(), UriKind.RelativeOrAbsolute);
}
},
{
"tokenUrl",
(o, n) =>
{
_flow.TokenUrl = new Uri(n.GetScalarValue());
_flow.TokenUrl = new Uri(n.GetScalarValue(), UriKind.RelativeOrAbsolute);
}
},
{
Expand All @@ -83,6 +83,7 @@ internal static partial class OpenApiV2Deserializer
public static OpenApiSecurityScheme LoadSecurityScheme(ParseNode node)
{
// Reset the local variables every time this method is called.
// TODO: Change _flow to a tempStorage variable to make the deserializer thread-safe.
_flowValue = null;
_flow = new OpenApiOAuthFlow();

Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.OpenApi.Readers/V2/OpenApiXmlDeserializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ internal static partial class OpenApiV2Deserializer
{
"namespace", (o, n) =>
{
o.Namespace = new Uri(n.GetScalarValue());
o.Namespace = new Uri(n.GetScalarValue(), UriKind.Absolute);
}
},
{
Expand Down
19 changes: 19 additions & 0 deletions src/Microsoft.OpenApi.Readers/V2/TempStorageKeys.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license.

namespace Microsoft.OpenApi.Readers.V2
{
/// <summary>
/// Strings to be used as keys for the temporary storage.
/// </summary>
internal static class TempStorageKeys
{
public const string ResponseSchema = "responseSchema";
public const string BodyParameter = "bodyParameter";
public const string FormParameters = "formParameters";
public const string OperationProduces = "operationProduces";
public const string OperationConsumes = "operationConsumes";
public const string GlobalConsumes = "globalConsumes";
public const string GlobalProduces = "globalProduces";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal static partial class OpenApiV3Deserializer
{
"url", (o, n) =>
{
o.Url = new Uri(n.GetScalarValue());
o.Url = new Uri(n.GetScalarValue(), UriKind.RelativeOrAbsolute);
}
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal static partial class OpenApiV3Deserializer
{
"url", (o, n) =>
{
o.Url = new Uri(n.GetScalarValue());
o.Url = new Uri(n.GetScalarValue(), UriKind.RelativeOrAbsolute);
}
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ internal static partial class OpenApiV3Deserializer
{
"termsOfService", (o, n) =>
{
o.TermsOfService = new Uri(n.GetScalarValue());
o.TermsOfService = new Uri(n.GetScalarValue(), UriKind.RelativeOrAbsolute);
}
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ internal static partial class OpenApiV3Deserializer
{
"url", (o, n) =>
{
o.Url = new Uri(n.GetScalarValue());
o.Url = new Uri(n.GetScalarValue(), UriKind.RelativeOrAbsolute);
}
},
};
Expand Down
21 changes: 18 additions & 3 deletions src/Microsoft.OpenApi.Readers/V3/OpenApiOAuthFlowDeserializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,24 @@ internal static partial class OpenApiV3Deserializer
private static readonly FixedFieldMap<OpenApiOAuthFlow> _oAuthFlowFixedFileds =
new FixedFieldMap<OpenApiOAuthFlow>
{
{"authorizationUrl", (o, n) => o.AuthorizationUrl = new Uri(n.GetScalarValue())},
{"tokenUrl", (o, n) => o.TokenUrl = new Uri(n.GetScalarValue())},
{"refreshUrl", (o, n) => o.RefreshUrl = new Uri(n.GetScalarValue())},
{
"authorizationUrl", (o, n) =>
{
o.AuthorizationUrl = new Uri(n.GetScalarValue(), UriKind.RelativeOrAbsolute);
}
},
{
"tokenUrl", (o, n) =>
{
o.TokenUrl = new Uri(n.GetScalarValue(), UriKind.RelativeOrAbsolute);
}
},
{
"refreshUrl", (o, n) =>
{
o.RefreshUrl = new Uri(n.GetScalarValue(), UriKind.RelativeOrAbsolute);
}
},
{"scopes", (o, n) => o.Scopes = n.CreateSimpleMap(LoadString)}
};

Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.OpenApi.Readers/V3/OpenApiXmlDeserializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ internal static partial class OpenApiV3Deserializer
{
"namespace", (o, n) =>
{
o.Namespace = new Uri(n.GetScalarValue());
o.Namespace = new Uri(n.GetScalarValue(), UriKind.Absolute);
}
},
{
Expand Down
Loading