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

Add option to make Content-ID header optional #2713

Open
wants to merge 4 commits into
base: release-7.x
Choose a base branch
from
Open
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 @@ -75,8 +75,9 @@ internal override Task<ODataBatchReader> CreateBatchReaderAsync()
/// <param name="synchronous">If the reader should be created for synchronous or asynchronous API.</param>
/// <returns>The newly created <see cref="ODataCollectionReader"/>.</returns>
private ODataBatchReader CreateBatchReaderImplementation(bool synchronous)
{
return new ODataMultipartMixedBatchReader(this, this.batchBoundary, this.Encoding, synchronous);
{
return new ODataMultipartMixedBatchReader(this, this.batchBoundary, this.Encoding, synchronous,
this.MessageReaderSettings.ThrowIfMissingContentIdInChangeset);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,27 @@ internal sealed class ODataMultipartMixedBatchReader : ODataBatchReader
/// </summary>
private string currentContentId;

/// <summary>
/// Require Content-Id header in changesets
/// If turned off, it allows to reading OData 2.0 requests without Content-Id header present.
/// </summary>
private bool requireContentId = true;

/// <summary>
/// Constructor.
/// </summary>
/// <param name="inputContext">The input context to read the content from.</param>
/// <param name="batchBoundary">The boundary string for the batch structure itself.</param>
/// <param name="batchEncoding">The encoding to use to read from the batch stream.</param>
/// <param name="synchronous">true if the reader is created for synchronous operation; false for asynchronous.</param>
internal ODataMultipartMixedBatchReader(ODataMultipartMixedBatchInputContext inputContext, string batchBoundary, Encoding batchEncoding, bool synchronous)
/// <param name="requireContentId">If turned off, it allows to reading OData 2.0 requests without Content-Id header present.</param>
internal ODataMultipartMixedBatchReader(ODataMultipartMixedBatchInputContext inputContext, string batchBoundary, Encoding batchEncoding, bool synchronous, bool requireContentId)
quanterion marked this conversation as resolved.
Show resolved Hide resolved
: base(inputContext, synchronous)
{
Debug.Assert(inputContext != null, "inputContext != null");
Debug.Assert(!string.IsNullOrEmpty(batchBoundary), "!string.IsNullOrEmpty(batchBoundary)");

this.requireContentId = requireContentId;
this.batchStream = new ODataMultipartMixedBatchReaderStream(this.MultipartMixedBatchInputContext, batchBoundary, batchEncoding);
this.dependsOnIdsTracker = new DependsOnIdsTracker();
}
Expand Down Expand Up @@ -85,7 +93,14 @@ protected override ODataBatchOperationRequestMessage CreateOperationRequestMessa

if (this.currentContentId == null)
{
throw new ODataException(Strings.ODataBatchOperationHeaderDictionary_KeyNotFound(ODataConstants.ContentIdHeader));
if (requireContentId)
{
throw new ODataException(Strings.ODataBatchOperationHeaderDictionary_KeyNotFound(ODataConstants.ContentIdHeader));
}
else
{
this.currentContentId = Guid.NewGuid().ToString();
Copy link
Member

Choose a reason for hiding this comment

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

Is it better to change the type of 'CurrentContentId' to System.Guid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

currentContentId is an arbitrary value of user-provided header. I assign it to guid just for specific case

}
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions src/Microsoft.OData.Core/ODataMessageReaderSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ public ValidationKinds Validations
ThrowOnDuplicatePropertyNames = (validations & ValidationKinds.ThrowOnDuplicatePropertyNames) != 0;
ThrowIfTypeConflictsWithMetadata = (validations & ValidationKinds.ThrowIfTypeConflictsWithMetadata) != 0;
ThrowOnUndeclaredPropertyForNonOpenType = (validations & ValidationKinds.ThrowOnUndeclaredPropertyForNonOpenType) != 0;
ThrowIfMissingContentIdInChangeset = (validations & ValidationKinds.ThrowIfMissingContentIdInChangeset) != 0;
}
}

Expand Down Expand Up @@ -235,6 +236,12 @@ public ODataMessageQuotas MessageQuotas
/// </summary>
internal bool ThrowOnUndeclaredPropertyForNonOpenType { get; private set; }

/// <summary>
/// Require Content-Id header in changesets
/// If turned off allows to read OData 2.0 requests without Content-Id header present.
Copy link
Member

Choose a reason for hiding this comment

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

What do you mean about "OData 2.0" ? It's a OData spec version number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah

Copy link
Contributor

Choose a reason for hiding this comment

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

@xuzhg @mikepizzo Do we really still support pre 4.0 versions on ODL 7.x?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be precise this small addition not to support 4.0 metadata, just for reading requests

quanterion marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
internal bool ThrowIfMissingContentIdInChangeset { get; private set; }

/// <summary>
/// Creates a shallow copy of this <see cref="ODataMessageReaderSettings"/>.
/// </summary>
Expand Down Expand Up @@ -296,6 +303,7 @@ private void CopyFrom(ODataMessageReaderSettings other)
this.ThrowOnDuplicatePropertyNames = other.ThrowOnDuplicatePropertyNames;
this.ThrowIfTypeConflictsWithMetadata = other.ThrowIfTypeConflictsWithMetadata;
this.ThrowOnUndeclaredPropertyForNonOpenType = other.ThrowOnUndeclaredPropertyForNonOpenType;
this.ThrowIfMissingContentIdInChangeset = other.ThrowIfMissingContentIdInChangeset;
this.LibraryCompatibility = other.LibraryCompatibility;
this.Version = other.Version;
this.ReadAsStreamFunc = other.ReadAsStreamFunc;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
Microsoft.OData.ODataResourceBase.SkipPropertyVerification.get -> bool
Microsoft.OData.ODataResourceBase.SkipPropertyVerification.set -> void
Microsoft.OData.ValidationKinds.ThrowIfMissingContentIdInChangeset = 8 -> Microsoft.OData.ValidationKinds
Microsoft.OData.ODataResourceBase.SkipPropertyVerification.get -> bool
Microsoft.OData.ODataResourceBase.SkipPropertyVerification.set -> void
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
Microsoft.OData.ODataResourceBase.SkipPropertyVerification.get -> bool
Microsoft.OData.ODataResourceBase.SkipPropertyVerification.set -> void
Microsoft.OData.ValidationKinds.ThrowIfMissingContentIdInChangeset = 8 -> Microsoft.OData.ValidationKinds
Microsoft.OData.ODataResourceBase.SkipPropertyVerification.get -> bool
Microsoft.OData.ODataResourceBase.SkipPropertyVerification.set -> void
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
Microsoft.OData.ODataResourceBase.SkipPropertyVerification.get -> bool
Microsoft.OData.ODataResourceBase.SkipPropertyVerification.set -> void
Microsoft.OData.ValidationKinds.ThrowIfMissingContentIdInChangeset = 8 -> Microsoft.OData.ValidationKinds
Microsoft.OData.ODataResourceBase.SkipPropertyVerification.get -> bool
Microsoft.OData.ODataResourceBase.SkipPropertyVerification.set -> void
Original file line number Diff line number Diff line change
Expand Up @@ -1962,9 +1962,10 @@ Microsoft.OData.UriParser.WildcardSelectItem.WildcardSelectItem() -> void
Microsoft.OData.ValidationKinds
Microsoft.OData.ValidationKinds.All = -1 -> Microsoft.OData.ValidationKinds
Microsoft.OData.ValidationKinds.None = 0 -> Microsoft.OData.ValidationKinds
Microsoft.OData.ValidationKinds.ThrowIfTypeConflictsWithMetadata = 4 -> Microsoft.OData.ValidationKinds
Microsoft.OData.ValidationKinds.ThrowOnDuplicatePropertyNames = 1 -> Microsoft.OData.ValidationKinds
Microsoft.OData.ValidationKinds.ThrowOnUndeclaredPropertyForNonOpenType = 2 -> Microsoft.OData.ValidationKinds
Microsoft.OData.ValidationKinds.ThrowIfTypeConflictsWithMetadata = 4 -> Microsoft.OData.ValidationKinds
Microsoft.OData.ValidationKinds.ThrowIfMissingContentIdInChangeset = 8 -> Microsoft.OData.ValidationKinds
override Microsoft.OData.HttpHeaderValueElement.ToString() -> string
override Microsoft.OData.ODataError.ToString() -> string
override Microsoft.OData.ODataStreamPropertyInfo.PrimitiveTypeKind.get -> Microsoft.OData.Edm.EdmPrimitiveTypeKind
Expand Down
6 changes: 6 additions & 0 deletions src/Microsoft.OData.Core/ValidationKinds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ public enum ValidationKinds
/// </summary>
ThrowIfTypeConflictsWithMetadata = 4,

/// <summary>
/// Require Content-Id header in changesets
/// If turned off allows to read OData 2.0 requests without Content-Id header present.
quanterion marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
ThrowIfMissingContentIdInChangeset = 8,

/// <summary>
/// Enable all validations.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using Microsoft.OData.Edm;
using Microsoft.OData.MultipartMixed;
Expand Down Expand Up @@ -69,6 +70,37 @@ public class ODataMultipartMixedBatchReaderTests
{""@odata.id"":""$2""}
--changeset_5e368128--
--batch_aed653ab--
";

private const string batchRequestPayloadWithoutContentId = @"--batch_aed653ab
Content-Type: multipart/mixed; boundary=changeset_5e368128

--changeset_5e368128
Content-Type: application/http
Content-Transfer-Encoding: binary

POST http://tempuri.org/Customers HTTP/1.1
OData-Version: 4.0
OData-MaxVersion: 4.0
Content-Type: application/json;odata.metadata=minimal
Accept: application/json;odata.metadata=minimal
Accept-Charset: UTF-8

{""@odata.type"":""NS.Customer"",""Id"":1,""Name"":""Sue""}
--changeset_5e368128
Content-Type: application/http
Content-Transfer-Encoding: binary

POST http://tempuri.org/Orders HTTP/1.1
OData-Version: 4.0
OData-MaxVersion: 4.0
Content-Type: application/json;odata.metadata=minimal
Accept: application/json;odata.metadata=minimal
Accept-Charset: UTF-8

{""@odata.type"":""NS.Order"",""Id"":1,""Amount"":13}
--changeset_5e368128--
--batch_aed653ab--
";

private const string batchResponsePayload = @"--batchresponse_aed653ab
Expand Down Expand Up @@ -205,6 +237,77 @@ public void ReadMultipartMixedBatchRequest()
Assert.Empty(verifyResourceStack);
}

[Fact]
public void ReadMultipartMixedBatchRequestWthoutContentId()
{
Assert.DoesNotContain("Content-ID", batchRequestPayloadWithoutContentId);

var verifyResourceStack = new Stack<Action<ODataResource>>();
verifyResourceStack.Push((resource) =>
{
Assert.NotNull(resource);
Assert.Equal("NS.Order", resource.TypeName);
var properties = resource.Properties.ToArray();
Assert.Equal(2, properties.Length);
Assert.Equal("Id", properties[0].Name);
Assert.Equal(1, properties[0].Value);
Assert.Equal("Amount", properties[1].Name);
Assert.Equal(13M, properties[1].Value);
});
verifyResourceStack.Push((resource) =>
{
Assert.NotNull(resource);
Assert.Equal("NS.Customer", resource.TypeName);
var properties = resource.Properties.ToArray();
Assert.Equal(2, properties.Length);
Assert.Equal("Id", properties[0].Name);
Assert.Equal(1, properties[0].Value);
Assert.Equal("Name", properties[1].Name);
Assert.Equal("Sue", properties[1].Value);
});

SetupMultipartMixedBatchReaderAndRunTest(
batchRequestPayloadWithoutContentId,
(multipartMixedBatchReader) =>
{
var operationCount = 0;

while (multipartMixedBatchReader.Read())
{
switch (multipartMixedBatchReader.State)
{
case ODataBatchReaderState.Operation:
var operationRequestMessage = multipartMixedBatchReader.CreateOperationRequestMessage();
operationCount++;
using (var messageReader = new ODataMessageReader(operationRequestMessage, new ODataMessageReaderSettings(), this.model))
{
if (operationCount == 3)
quanterion marked this conversation as resolved.
Show resolved Hide resolved
{
var entityReferenceLink = messageReader.ReadEntityReferenceLink();
}
else
{
var jsonLightResourceReader = messageReader.CreateODataResourceReader();
while (jsonLightResourceReader.Read())
{
switch (jsonLightResourceReader.State)
{
case ODataReaderState.ResourceEnd:
Assert.NotEmpty(verifyResourceStack);
var innerVerifyResourceStack = verifyResourceStack.Pop();
innerVerifyResourceStack(jsonLightResourceReader.Item as ODataResource);
break;
}
}
}
}
break;
}
}
},
batchRequestBoundary);
}

[Fact]
public async Task ReadMultipartMixedBatchRequestAsync()
{
Expand Down Expand Up @@ -704,7 +807,8 @@ private void SetupMultipartMixedBatchReaderAndRunTest(
multipartMixedBatchInputContext,
batchBoundary,
MediaTypeUtils.EncodingUtf8NoPreamble,
synchronous: true);
synchronous: true,
requireContentId: false);

action(multipartMixedBatchReader);
}
Expand All @@ -726,7 +830,8 @@ private async Task SetupMultipartMixedBatchReaderAndRunTestAsync(
multipartMixedBatchInputContext,
batchBoundary,
MediaTypeUtils.EncodingUtf8NoPreamble,
synchronous: false);
synchronous: false,
requireContentId: false);

await func(multipartMixedBatchReader);
}
Expand Down
Loading