Skip to content

Commit

Permalink
Addressed addtional CR comments, primarily:
Browse files Browse the repository at this point in the history
- Changing to use payloadUrlConverter to validate referencing uri for the case of Multipart batch with implicit dependsOnIds.
- Tests added.
  • Loading branch information
biaol-odata committed Jan 10, 2018
1 parent 69b81cf commit fb2998f
Show file tree
Hide file tree
Showing 8 changed files with 150 additions and 26 deletions.
2 changes: 1 addition & 1 deletion src/Microsoft.OData.Core/Microsoft.OData.Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
<Compile Include="AnnotationFilterPattern.cs" />
<Compile Include="AsyncBufferedStream.cs" />
<Compile Include="ContainerBuilderExtensions.cs" />
<Compile Include="DependsOnIdsTracker.cs" />
<Compile Include="EdmExtensionMethods.cs" />
<Compile Include="Evaluation\ODataConventionalResourceMetadataBuilder.cs" />
<Compile Include="JsonLight\ODataJsonLightBatchAtomicGroupCache.cs" />
Expand All @@ -54,6 +53,7 @@
<Compile Include="JsonLight\ODataJsonLightBatchReader.cs" />
<Compile Include="JsonLight\ODataJsonLightBatchReaderStream.cs" />
<Compile Include="JsonLight\ODataJsonLightBatchWriter.cs" />
<Compile Include="MultipartMixed\DependsOnIdsTracker.cs" />
<Compile Include="MultipartMixed\ODataMultipartMixedBatchFormat.cs" />
<Compile Include="MultipartMixed\ODataMultipartMixedBatchInputContext.cs" />
<Compile Include="MultipartMixed\ODataMultipartMixedBatchOutputContext.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
//---------------------------------------------------------------------
//---------------------------------------------------------------------
// <copyright file="DependsOnIdsTracker.cs" company="Microsoft">
// Copyright (C) Microsoft Corporation. All rights reserved. See License.txt in the project root for license information.
// </copyright>
//---------------------------------------------------------------------

namespace Microsoft.OData
{
using System.Collections.Generic;
using System.Diagnostics;
using System.Collections.Generic;
using System.Diagnostics;

namespace Microsoft.OData.MultipartMixed
{
/// <summary>
/// This class is to keep track of dependsOn ids and associated change set state,
/// and to return dependsOn ids pertaining to current state of the batch processing.
Expand Down
8 changes: 8 additions & 0 deletions src/Microsoft.OData.Core/ODataBatchPayloadUriConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ internal IODataPayloadUriConverter BatchMessagePayloadUriConverter
}
}

/// <summary>
/// A read-only enumeration of <code>contentIdCache</code>.
/// </summary>
internal IEnumerable<string> ContentIdCache
{
get { return this.contentIdCache; }
}

/// <summary>
/// Method to implement a custom URL conversion scheme.
/// This method returns null if not custom conversion is desired.
Expand Down
18 changes: 6 additions & 12 deletions src/Microsoft.OData.Core/ODataBatchReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,8 @@ private set

/// <summary>
/// Public property for the current group id the reader is processing.
/// The primary usage of this property is, for Json batch, to correlate atomic group id
/// in request and response operation messages.
/// For multipart batch, since there are no correlations between the changeset boundaries in
/// request and response, this value is ignored / not used, is always null.
/// The primary usage of this to correlate atomic group id in request and
/// response operation messages as needed.
/// </summary>
public string CurrentGroupId
{
Expand Down Expand Up @@ -258,9 +256,7 @@ protected void ThrowODataException(string errorMessage)

/// <summary>
/// Gets the group id for the current request.
/// This is for Json batch format only, which requires atomic group ids from corresponding request and
/// response should be the same.
/// Default implementation here is provided for multipart batch.
/// Default implementation here is provided returning null.
/// </summary>
/// <returns>The group id for the current request.</returns>
[SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate", Justification = "A method for consistency with the rest of the API.")]
Expand Down Expand Up @@ -317,9 +313,8 @@ protected virtual string GetCurrentGroupIdImplementation()
/// <param name="contentId">The contentId of this request message.</param>
/// <param name="groupId">The group id that this request belongs to. Can be null.</param>
/// <param name="dependsOnRequestIds">
/// The prerequisite request Ids of this request message. For batch in Json format,
/// some of these request Ids are resolved from prerequisite atomic groups that are
/// specified in the dependsOn attribute of the request.
/// The prerequisite request Ids of this request message that could be specified by caller
/// explicitly.
/// </param>
/// <param name="dependsOnIdsValidationRequired">
/// Whether the <code>dependsOnIds</code> value needs to be validated.</param>
Expand Down Expand Up @@ -362,8 +357,7 @@ protected ODataBatchOperationRequestMessage BuildOperationRequestMessage(
/// <param name="statusCode">The status code for the response.</param>
/// <param name="headers">The headers for this response message.</param>
/// <param name="contentId">The contentId of this request message.</param>
/// <param name="groupId">The groupId of this request message.
/// Value is null for multipart batch operation response message.</param>
/// <param name="groupId">The groupId of this request message.</param>
/// <returns>The <see cref="ODataBatchOperationResponseMessage"/> instance.</returns>
protected ODataBatchOperationResponseMessage BuildOperationResponseMessage(
Func<Stream> streamCreatorFunc,
Expand Down
3 changes: 2 additions & 1 deletion src/Microsoft.OData.Core/ODataBatchUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,8 @@ internal static void ValidateReferenceUri(Uri uri, IEnumerable<string> dependsOn
if (dependsOnRequestIds == null || !dependsOnRequestIds.Contains(referenceId))
{
throw new ODataException(Strings.ODataBatchReader_ReferenceIdNotIncludedInDependsOn(
referenceId, UriUtils.UriToString(uri), string.Join(",", dependsOnRequestIds.ToArray())));
referenceId, UriUtils.UriToString(uri),
dependsOnRequestIds != null ? string.Join(",", dependsOnRequestIds.ToArray()) : "null"));
}
}
}
Expand Down
12 changes: 8 additions & 4 deletions src/Microsoft.OData.Core/ODataBatchWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ public Task FlushAsync()
/// Starts a new changeset.
/// </summary>
/// <param name="groupOrChangesetId">
/// The atomic group id (for Json batch) / changeset GUID (for Multipart/Mixed batch) of the batch request.
/// The atomic group id, aka changeset GUID of the batch request.
/// </param>
protected abstract void WriteStartChangesetImplementation(string groupOrChangesetId);

Expand Down Expand Up @@ -538,8 +538,7 @@ protected ODataBatchOperationRequestMessage BuildOperationRequestMessage(Stream

if (dependsOnIds != null)
{
// Validate explicit dependsOnIds cases, which include all cases for JSON batch
// plus explicit dependsOnIds case for Multipart batch.
// Validate explicit dependsOnIds cases.
foreach (string id in convertedDependsOnIds)
{
if (!this.payloadUriConverter.ContainsContentId(id))
Expand All @@ -549,7 +548,12 @@ protected ODataBatchOperationRequestMessage BuildOperationRequestMessage(Stream
}
}

ODataBatchUtils.ValidateReferenceUri(uri, convertedDependsOnIds, this.outputContext.MessageWriterSettings.BaseUri);
// If dependsOnIds is not specified, use the <code>payloadUrlConverter</code>; otherwise use the dependOnIds converted
// from specified value.
IEnumerable<string> requestIdsForUrlReferenceValidation =
dependsOnIds == null ? this.payloadUriConverter.ContentIdCache : convertedDependsOnIds;

ODataBatchUtils.ValidateReferenceUri(uri, requestIdsForUrlReferenceValidation, this.outputContext.MessageWriterSettings.BaseUri);

Func<Stream> streamCreatorFunc = () => ODataBatchUtils.CreateBatchOperationWriteStream(outputStream, this);
ODataBatchOperationRequestMessage requestMessage =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public MultipartMixedBatchDependsOnIdsTests()
}

[Fact]
public void MultipartBatchTestWithTopLevelDependsOnIds()
public void MultipartBatchTestWithTopLevelDependsOnIdsV401()
{
string customizedValidRequest = Regex.Replace(RequestPayloadVerifyDependsOnIdsTemplate,
"__REF_URI_1__",
Expand All @@ -169,6 +169,27 @@ public void MultipartBatchTestWithTopLevelDependsOnIds()
ClientReadSingletonBatchResponse(responsePayload, batchContentTypeMultipartMime);
}

[Fact]
public void MultipartBatchTestWithTopLevelDependsOnIdsV4()
{
string customizedValidRequest = Regex.Replace(RequestPayloadVerifyDependsOnIdsTemplate,
"__REF_URI_1__",
"$2B",
RegexOptions.Multiline);

customizedValidRequest = Regex.Replace(customizedValidRequest,
"__REF_URI_2__",
"$1",
RegexOptions.Multiline);

// For V4 MultipartMixed batch, top-level request "3"'s url "/$1"(referencing a preceding top-level request "1")
// should trigger an exception because request reference scope is limited to change set in V4 implementation.
ODataException ode = Assert.Throws<ODataException>(
() => this.ServiceReadRequestAndWriterResponseForMultipartBatchVerifyDependsOnIds(customizedValidRequest, ODataVersion.V4));
Assert.True(ode.Message.Contains(
"When the relative URI is a reference to a content ID, the content ID does not exist in the current change set."));
}

private void VerifyPayloadForMultipartBatch(byte[] payloadBytes, string expectedPayload)
{
using (MemoryStream stream = new MemoryStream(payloadBytes))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,31 @@ public void MultipartBatchImplicitChangeSetInvalidDependsOnIdShouldThrow()
}
}

[Fact]
public void MultipartBatchVerifyDependsOnIdsForTopLevelRequestV4()
{
// In V4, referencing preceding top-level request Id from another request
// after change set should throw.
string topLevelContentId = "1";

ODataException ode = Assert.Throws<ODataException>(
() => ClientWriteRequestForMultipartBatchVerifyDependsOnIdsForTopLevelRequest(
topLevelContentId, ODataVersion.V4));
Assert.True(ode.Message.Contains(referenceIdNotIncludedInDependsOn));
}

[Fact]
public void MultipartBatchVerifyDependsOnIdsForTopLevelRequestV401()
{
string topLevelContentId = "1";

byte[] requestBytes = ClientWriteRequestForMultipartBatchVerifyDependsOnIdsForTopLevelRequest(
topLevelContentId, ODataVersion.V401);

Assert.NotNull(requestBytes);
Assert.True(requestBytes.Length > 0);
}

[Fact]
public void MultipartBatchImplicitTopLevelDependsOnIdsTest()
{
Expand Down Expand Up @@ -1017,6 +1042,22 @@ public void BatchJsonLightContentIdUniquenessScopeForJsonBatchV401ShouldThrow()
Assert.Contains(expectedPartialErrorMesage, ode.Message);
}

private static void PopulateWebResourceId(ODataBatchOperationRequestMessage dataModificationRequestMessage, int webId)
{
// Use a new message writer to write the body of this operation.
using (ODataMessageWriter operationMessageWriter = new ODataMessageWriter(dataModificationRequestMessage))
{
ODataWriter entryWriter = operationMessageWriter.CreateODataResourceWriter();
ODataResource entry = new ODataResource
{
TypeName = "NS.Web",
Properties = new[] { new ODataProperty { Name = "WebId", Value = webId } }
};
entryWriter.WriteStart(entry);
entryWriter.WriteEnd();
}
}

private void BatchJsonLightTestUsingBatchFormat(BatchFormat batchFormat, int idx)
{
byte[] requestPayload = null;
Expand Down Expand Up @@ -1745,7 +1786,7 @@ private byte[] ClientWriteMuitipartChangesetsWithSameContentId(ODataMessageWrite
}
}

private byte[] ClientWriteRequestForMultipartBatchVerifyDependsOnIds(string contendIdRef, ODataVersion version)
private byte[] ClientWriteRequestForMultipartBatchVerifyDependsOnIds(string contentIdRef, ODataVersion version)
{
MemoryStream stream = new MemoryStream();

Expand Down Expand Up @@ -1808,7 +1849,7 @@ private byte[] ClientWriteRequestForMultipartBatchVerifyDependsOnIds(string cont
entryWriter.WriteEnd();
}

Uri referenceUri = new Uri("$" + contendIdRef, UriKind.Relative);
Uri referenceUri = new Uri("$" + contentIdRef, UriKind.Relative);
updateOperationMessage = batchWriter.CreateOperationRequestMessage("PATCH", referenceUri, "2C",
BatchPayloadUriOption.RelativeUri);

Expand Down Expand Up @@ -1863,6 +1904,61 @@ private byte[] ClientWriteRequestForMultipartBatchVerifyDependsOnIds(string cont
}
}

private byte[] ClientWriteRequestForMultipartBatchVerifyDependsOnIdsForTopLevelRequest(string contentIdRef, ODataVersion version)
{
// Batch consists of one top-level request, one change set, and one more top-level request referencing the first top-level request.
MemoryStream stream = new MemoryStream();

IODataRequestMessage requestMessage = new InMemoryMessage { Stream = stream };
requestMessage.SetHeader("Content-Type", batchContentTypeMultipartMime);

using (ODataMessageWriter messageWriter = new ODataMessageWriter(requestMessage,
new ODataMessageWriterSettings
{
Version = version,
BaseUri = new Uri(serviceDocumentUri)
}))
{
ODataBatchWriter batchWriter = messageWriter.CreateODataBatchWriter();

batchWriter.WriteStartBatch();

// A create operation.
ODataBatchOperationRequestMessage createOperationMessage =
batchWriter.CreateOperationRequestMessage("POST", new Uri(serviceDocumentUri + "MySingleton"), contentIdRef);
PopulateWebResourceId(createOperationMessage, 8);

// Header modification on inner payload.
// createOperationMessage.SetHeader("Accept", "application/json;odata.metadata=full");
Assert.NotNull(createOperationMessage.ContentId);

// A change set with multi update operation.
batchWriter.WriteStartChangeset();
{
// Create a update operation in the change set.
ODataBatchOperationRequestMessage updateOperationMessage =
batchWriter.CreateOperationRequestMessage("PATCH", new Uri(serviceDocumentUri + "/MySingleton"), "2A" /*written*/);
PopulateWebResourceId(updateOperationMessage, 9);
}
batchWriter.WriteEndChangeset();

// An update operation referencing the preceding create operation.
Uri referenceUri = new Uri("$" + contentIdRef, UriKind.Relative);
ODataBatchOperationRequestMessage topLevelOperationMessage =
batchWriter.CreateOperationRequestMessage("PATCH", referenceUri, "3", BatchPayloadUriOption.AbsoluteUri );
PopulateWebResourceId(topLevelOperationMessage, 10);

// Header modification on inner payload.
// topLevelOperationMessage.SetHeader("Accept", "application/json;odata.metadata=full");
Assert.NotNull(topLevelOperationMessage.ContentId);

batchWriter.WriteEndBatch();

stream.Position = 0;
return stream.ToArray();
}
}

private byte[] ServiceReadRequestAndWriterResponseForMultipartBatchVerifyDependsOnIds(byte[] requestPayload, ODataVersion odataVersion)
{
IODataRequestMessage requestMessage = new InMemoryMessage() { Stream = new MemoryStream(requestPayload) };
Expand Down

0 comments on commit fb2998f

Please sign in to comment.