Hello,
thank you for your review. I tried to use the style of the existing code
but I guess I missed some spaces.
The problem with your modifications is that it defeats the purpose of my
patch. In .NET I can host a method with a definition like:
[OperationContract]
[WebInvoke(UriTemplate="set?name={name}&value={value}")]
void Set(string name, string value);
In Mono I couldn't. This is why I created the patch. With your
modifications the problem still exists, but now in a different place.
ValidateStyleForMessageFormatter shouldn't reject a method like the one
above. Like the modified Validate method it should consider the
UriTemplate. In the method above there is no need to wrap because
everything is passed in the URI. That works for .NET and also works for
Mono if the validation is less restrictive. I think the major problem
with my patch is that it doesn't work if the UriTemplate is null. With
only that fixed the test RejectTwoParametersWhenNotWrapped will not
break and it also works for my purpose.
I also checked the .NET behavior for WrappedRequest and WrappedResponse.
In .NET the BodyStyle WrappedRequest resolves problems with multiple
inputs and WrappedResponse with multiple outputs. That is what I
expected and it is the opposite to what is done in
ValidateStyleForMessageFormatter.
So I created a new version that hopefully resolves all issues.
On 12.10.2010 13:30, Atsushi Eno wrote:
Hello,
Thanks for the patch, I very much appreciate it. Though there were
some problems in your patch.
- your implementation had an assumptiopn that WebGetAttribute and
WebInvokeAttribute has non-null UriTemplate. They can indeed be
null.
- Your change broke RejectTwoParametersWhenNotWrapped() in
WebInvokeAttributeTest. It is because, in your
WebHttpBehavior.Validate()
change you just removed existing code that checked what this
test exactly
verifies i.e. invalid multiple arguments. You might think it is
"very strange"
and "wrong" but that's what .NET indeed does (looks like it is
done in
GetClientMessageFormatter() in current version of .NET though).
- There was a couple of coding style fixage needed. You had right
and wrong
style, so I assume you might have already known the style, but
in case you
haven't, have a look at http://mono-project.com/Coding_Guidelines
(I found this page was not linked from our "Contributing" page,
which was
our bad, just fixed it.)
I'm attaching the modified fix here. It includes some additional
tests. I'll commit the changes unless you have further comments.
(As a cosmetic excuse, WebMessageBodyStyle.Bare was new in 3.5 SP1
AFAIR, so it was left ignorant in our implementation from 3.5 era.)
Atsushi Eno
On 2010/10/09 18:27, Frank Wilhelm wrote:
Hello Mono devs,
I tried running my web service on Mono and ran into several issues. I
use the WebHttpBinding to create REST web services by hosting that
with the ServiceHost class. This works fine on .NET but the Mono
implementation of System.ServiceModel.Web shows very different, and
very limiting, behavior. So I decided instead of waiting for a fix I
try to track down the issues. Here is the first I discovered.
The Validation of WebHttpBehavior is very limiting. If a POST
operation has several parameters it will only host them if you define
'wrapped' body format. But this isn't required if the parameters are
passed in the UriTemplate. Also the requirements for WrappedRequest
and WrappedResponse look very strange. I have to wrap my response if
I have multiple input parameters? That looks wrong.
I looked up in the MSDN what the Validate method does for .NET, and
then I tried to make the .NET implementation fail. It just doesn't
behave like specified in the documentation, it never fails no matter
what.
I attach a patch that will make the Validate method less picky. It
will look for placeholders in the URI template and only after that
decides whether wrapping is needed or not. That is still more
restrictive than the .NET implementation but for me it looks like the
right thing to do. I also added a test that reproduces the problem.
Please accept the patch and give me some feedback on what I can
improve for further contributions.
_______________________________________________
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
diff --git
a/mcs/class/System.ServiceModel.Web/System.ServiceModel.Description/WebHttpBehavior.cs
b/mcs/class/System.ServiceModel.Web/System.ServiceModel.Description/WebHttpBehavior.cs
index 38b804f..32554b9 100644
---
a/mcs/class/System.ServiceModel.Web/System.ServiceModel.Description/WebHttpBehavior.cs
+++
b/mcs/class/System.ServiceModel.Web/System.ServiceModel.Description/WebHttpBehavior.cs
@@ -222,6 +222,61 @@ namespace System.ServiceModel.Description
}
#endif
+ WebMessageBodyStyle GetBodyStyle (WebAttributeInfo wai)
+ {
+ return wai != null && wai.IsBodyStyleSetExplicitly ?
wai.BodyStyle : DefaultBodyStyle;
+ }
+
+ protected void ValidateOperation (OperationDescription
operation)
+ {
+ var wai = operation.GetWebAttributeInfo ();
+ if (wai.Method == "GET")
+ return;
+ var style = GetBodyStyle (wai);
+
+ // if the style is wrapped there won't be problems
+ if (style == WebMessageBodyStyle.Wrapped)
+ return;
+
+ string [] parameters;
+ if (wai.UriTemplate != null) {
+ // find all variables in the URI
+ var uri = new UriTemplate (wai.UriTemplate);
+ parameters = new string
[uri.PathSegmentVariableNames.Count + uri.QueryValueVariableNames.Count];
+ uri.PathSegmentVariableNames.CopyTo
(parameters, 0);
+ uri.QueryValueVariableNames.CopyTo (parameters,
uri.PathSegmentVariableNames.Count);
+
+ // sort because Array.BinarySearch is the
easiest way for case-insensitive search
+ Array.Sort (parameters,
StringComparer.InvariantCultureIgnoreCase);
+ } else
+ parameters = new string [0];
+
+ bool hasBody = false;
+
+ foreach (var msg in operation.Messages) {
+ if (msg.Direction == MessageDirection.Input) {
+ // the message is for a request
+ // if requests are wrapped there is
nothing to check
+ if (style ==
WebMessageBodyStyle.WrappedRequest)
+ continue;
+
+ foreach (var part in msg.Body.Parts) {
+ if (Array.BinarySearch
(parameters, part.Name, StringComparer.InvariantCultureIgnoreCase) < 0) {
+ // this part of the
message is not covered by a variable in the URI
+ // so it must be passed
in the body
+ if (hasBody)
+ throw new
InvalidOperationException (String.Format ("Operation '{0}' has multiple message
body parts. Add parameters to the UriTemplate or change the BodyStyle to
'Wrapped' or 'WrappedRequest' on the WebInvoke/WebGet attribute.",
operation.Name));
+ hasBody = true;
+ }
+ }
+ } else {
+ // the message is for a response
+ if (style !=
WebMessageBodyStyle.WrappedResponse && msg.Body.Parts.Count > 0)
+ throw new
InvalidOperationException (String.Format ("Operation '{0}' has output
parameters. BodyStyle must be 'Wrapped' or 'WrappedResponse' on the operation
WebInvoke/WebGet attribute.", operation.Name));
+ }
+ }
+ }
+
[MonoTODO ("check UriTemplate validity")]
public virtual void Validate (ServiceEndpoint endpoint)
{
@@ -229,28 +284,7 @@ namespace System.ServiceModel.Description
throw new ArgumentNullException ("endpoint");
foreach (var oper in endpoint.Contract.Operations) {
- var wai = oper.GetWebAttributeInfo ();
- if (wai.Method == "GET")
- continue;
- var style = wai != null &&
wai.IsBodyStyleSetExplicitly ? wai.BodyStyle : DefaultBodyStyle;
- foreach (var msg in oper.Messages)
- switch (style) {
- case WebMessageBodyStyle.Wrapped:
- continue;
- case WebMessageBodyStyle.WrappedRequest:
- if (msg.Direction ==
MessageDirection.Output)
- continue;
- goto case
WebMessageBodyStyle.Bare;
- case
WebMessageBodyStyle.WrappedResponse:
- if (msg.Direction ==
MessageDirection.Input)
- continue;
- goto case
WebMessageBodyStyle.Bare;
- case WebMessageBodyStyle.Bare:
- default:
- if (msg.Body.Parts.Count > 1)
- throw new
InvalidOperationException (String.Format ("{0} message on operation '{1}' has
multiple parameters which is not allowed when the operation indicates no
wrapper element. BodyStyle must be 'wrapped' on the operation WebInvoke/WebGet
attribute.", msg.Direction, oper.Name));
- break;
- }
+ ValidateOperation (oper);
}
ValidateBinding (endpoint);
diff --git
a/mcs/class/System.ServiceModel.Web/Test/System.ServiceModel.Description/WebHttpBehaviorTest.cs
b/mcs/class/System.ServiceModel.Web/Test/System.ServiceModel.Description/WebHttpBehaviorTest.cs
index 1267d08..4ba5014 100644
---
a/mcs/class/System.ServiceModel.Web/Test/System.ServiceModel.Description/WebHttpBehaviorTest.cs
+++
b/mcs/class/System.ServiceModel.Web/Test/System.ServiceModel.Description/WebHttpBehaviorTest.cs
@@ -53,6 +53,20 @@ namespace MonoTests.System.ServiceModel.Description
}
[Test]
+ public void DefaultValues ()
+ {
+ var b = new WebHttpBehavior ();
+ Assert.AreEqual (WebMessageBodyStyle.Bare,
b.DefaultBodyStyle, "#1");
+ Assert.AreEqual (WebMessageFormat.Xml,
b.DefaultOutgoingRequestFormat, "#2");
+ Assert.AreEqual (WebMessageFormat.Xml,
b.DefaultOutgoingResponseFormat, "#3");
+#if NET_4_0
+ Assert.IsFalse (b.AutomaticFormatSelectionEnabled,
"#11");
+ Assert.IsFalse (b.FaultExceptionEnabled, "#12");
+ Assert.IsFalse (b.HelpEnabled, "#13");
+#endif
+ }
+
+ [Test]
public void AddBiningParameters ()
{
var se = CreateEndpoint ();
@@ -234,6 +248,39 @@ namespace MonoTests.System.ServiceModel.Description
formatter.DeserializeRequest (msg, pars);
Assert.IsTrue (pars [0] is Stream, "ret");
}
+
+ [ServiceContract]
+ public interface IMultipleParameters
+ {
+ [OperationContract]
+ [WebGet (UriTemplate = "get")]
+ void Get(string p1, string p2);
+
+ [OperationContract]
+ [WebInvoke (UriTemplate = "posturi?p1={p1}&p2={p2}")]
+ string PostUri (string p1, string p2);
+
+ [OperationContract]
+ [WebInvoke (UriTemplate = "postone?p1={p1}")]
+ string PostOne (string p1, string p2);
+
+ [OperationContract]
+ [WebInvoke (UriTemplate = "postwrapped",
BodyStyle=WebMessageBodyStyle.WrappedRequest)]
+ string PostWrapped (string p1, string p2);
+
+ [OperationContract]
+ [WebInvoke (UriTemplate = "out?p1={p1}&p2={p2}",
BodyStyle=WebMessageBodyStyle.WrappedResponse)]
+ void PostOut (string p1, string p2, out string ret);
+ }
+
+ [Test]
+ public void MultipleParameters ()
+ {
+ var behavior = new WebHttpBehavior ();
+ var cd = ContractDescription.GetContract (typeof
(IMultipleParameters));
+ var se = new ServiceEndpoint (cd, new WebHttpBinding
(), new EndpointAddress ("http://localhost:8080/";));
+ behavior.Validate (se);
+ }
}
[ServiceContract]
_______________________________________________
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
