<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On 2 November 2014 14:26, Kornel Pal <span dir="ltr"><<a href="mailto:kornelpal@gmail.com" target="_blank">kornelpal@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF" text="#000000">
    Hi,<br>
    <br>
    I've noticed that new functionality is going into the wrappers,
    while in my opinion that the functionality belongs to HttpRequest
    and HttpResponse:<br>
    <ul>
      <li>HttpRequestBase.ReadEntityBodyMode: returning 0 instead of
        ReadEntityBodyMode.Classic made more sense<br>
      </li>
      <li>HttpRequestWrapper.ReadEntityBodyMode: wrapper is not supposed
        to implement functionality, that belongs to HttpRequest</li></ul></div></blockquote><div>These are a bit of a fudge granted, at the moment, what we're doing is forcing the client (calling application) to use the Request.InputStream stream instead of using the Buffer methods.  This is important until we have a full implementation of the methods required to do all the other logic. <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><ul>
      <li>HttpRequestWrapper.Abort: wrapper is not supposed to implement
        functionality, that belongs to HttpRequest</li></ul></div></blockquote><div>This is a crude implementation, and will need to be properly implemented in HttpRequest at some point. I opted to do it in the Wrapper so it's known that it's not a proper implementation.  There is a bug here that I'm not sure whether it causes the  <span>BeginGetRequestStream</span> or <span>BeginGetResponse to be called, so it really needs a fair bit of work.</span> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><ul>
      <li>HttpResponseWrapper.ClientDisconnectedToken: wrapper is not
        supposed to implement functionality, that belongs to
        HttpResponse</li></ul></div></blockquote><div>This should be an easy fix, to simply do the return CancellationToken.None from the HttpResponse instead. <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><ul>
    </ul>
    As for the GetBuffer* methods: <br>
    <ul>
      <li>Mono already implements HttpRequest<span>.</span>GetBufferlessInputStream,
        so hard coding ReadEntityBodyMode.Classic limits Mono's
        compatibility</li></ul></div></blockquote><div>Essentially, I had choose only 1 value as I didn't have time to implement the other options.  Choosing Classic meant that we could ensure that new clients revert to functionality we know works.  We're not saying that this is the way it will always work, just that this is a starting point for people.  Once all the methods have been implemented, this will obviously be changed.<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><ul>
      <li>Implementing GetBufferedInputStream based on the existing
        GetBufferlessInputStream implementation doesn't seem to be too
        difficult</li></ul></div></blockquote><div>Difficulty is in the eye of the beholder. <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><ul>
      <li>HttpWorkerRequest in 4.5 has support for asynchronous
        operations, but that does not have to be implemented for this to
        work because the Stream base class implements async operations
        using the sync methods</li></ul></div></blockquote><div>I've simply not had the time to look into this part yet.  I've been thinking of a minimal viable product that allows MVC/WebAPI to work out of the box on mono, not looking at what methods need to be implemented to complete a full implementation of <a href="http://ASP.NET">ASP.NET</a>.<br><br></div><div>This whole area is something that is coupled with the ReadEntityBodyMode, it will be great to get them all in, and I will be looking at them, they are just not part of the MVP work. <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><ul>
    </ul>
    Thank you.<span class=""><font color="#888888"><br>
    <br>
    Kornel</font></span><div><div class="h5"><br>
    <br>
    <div>On 11/2/2014 1:52 AM, Miguel de Icaza
      wrote:<br>
    </div>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div class="gmail_quote">
            <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
              <div dir="ltr">
                <div><i><br>
                  </i></div>
                <div>PR1349: <a href="https://github.com/mono/mono/pull/1349" target="_blank">https://github.com/mono/mono/pull/1349</a>
                  <br>
                  <i>This is the machine key work, and needs a small
                    tweak before it can be merged that I will do this
                    week.<br>
                  </i></div>
              </div>
            </blockquote>
            <div><br>
            </div>
            <div>I believe the TODO can be removed.   Can you do that? 
              See comments on pull request.</div>
            <div> </div>
            <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
              <div dir="ltr">
                <div>PR1363: <a href="https://github.com/mono/mono/pull/1363" target="_blank">https://github.com/mono/mono/pull/1363</a>
                  <br>
                  <i>Another of mine with the
                    MembershipPasswordAttribute<br>
                  </i></div>
              </div>
            </blockquote>
            <div><br>
            </div>
            <div>Do you mind resending this?  It can no longer be
              auto-merged from the UI.</div>
            <div> </div>
            <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
              <div dir="ltr">
                <div>PR1365: <a href="https://github.com/mono/mono/pull/1365" target="_blank">https://github.com/mono/mono/pull/1365</a>
                  <br>
                  <i>This is Kornel Pal's around the
                    HttpTaskAsyncHandler, and Miguel has said he'll take
                    a look at it.<br>
                  </i></div>
              </div>
            </blockquote>
            <div><br>
            </div>
            <div>Manually aded</div>
            <div> </div>
            <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
              <div dir="ltr">
                <div>PR1370: <a href="https://github.com/mono/mono/pull/1370" target="_blank">https://github.com/mono/mono/pull/1370</a>
                  <br>
                  <i>Small one implementing a default of the
                    ReadEntityBodyMode<br>
                  </i></div>
              </div>
            </blockquote>
            <div><br>
            </div>
            <div>Got this one by hand.</div>
            <div> </div>
            <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
              <div dir="ltr">
                <div>PR1371: <a href="https://github.com/mono/mono/pull/1371" target="_blank">https://github.com/mono/mono/pull/1371</a><br>
                </div>
                <div><i>Another small one, implementing the
                    ClientDisconnectedToken<br>
                  </i></div>
              </div>
            </blockquote>
            <div><br>
            </div>
            <div>And this one automatically.</div>
            <div> </div>
            <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
              <div dir="ltr">
                <div>PR1372: <a href="https://github.com/mono/mono/pull/1372" target="_blank">https://github.com/mono/mono/pull/1372</a><br>
                </div>
                <div><i>A final small one around the GetBuffer* methods
                    in the httprequest.<br>
                  </i></div>
              </div>
            </blockquote>
            <div><br>
            </div>
            <div>I do not like this one.  Is there a reason we can not
              implement the required functionality instead?</div>
            <div><br>
            </div>
            <div>Miguel</div>
            <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
              <div dir="ltr">
                <div>There is 1 final small piece that either myself of
                  Chris Carroll will get done this week which is around
                  the AppendTrailing slash and lowercaseUrls properties
                  in RouteBase class.  We just need to put the
                  implementation together.<br>
                  <br>
                  Anyway, after applying all of these, my large WebAPI
                  solution not only compiles, but it also runs!<br>
                  <br>
                </div>
                <div>If you want to checkout what it looks like with all
                  the patches applied, that would be great, I'd love to
                  have some more information on whether it does work. 
                  I'm sure there will still be bugs, but if it works
                  mostly, then bug fixing is easy (famous last words).<br>
                  <br>
                  <a href="https://github.com/martinjt/mono/tree/mvc_allfixes" target="_blank">https://github.com/martinjt/mono/tree/mvc_allfixes</a><br>
                  <br>
                </div>
                <div>Thanks for everyone's help.<span><font color="#888888"><br>
                    </font></span></div>
                <span><font color="#888888">
                    <div><br>
                    </div>
                    <div>Martin</div>
                  </font></span></div>
              <div>
                <div>
                  <div class="gmail_extra"><br>
                    <div class="gmail_quote">On 20 October 2014 20:42,
                      Martin Thwaites <span dir="ltr"><<a href="mailto:monoforum@my2cents.co.uk" target="_blank">monoforum@my2cents.co.uk</a>></span>
                      wrote:<br>
                      <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
                        <p dir="ltr">Hi Miguel,</p>
                        <p dir="ltr">The code that I'm referring to here
                          is that of the aspnetwebstack on codeplex. 
                          That is to say that they are not something
                          where you can remove the code and recompile
                          (unless there as a specific mono
                          implementation which is not ideal).  The goal
                          is to have the compiled dlls that are
                          available on nuget work, without tweaking to a
                          person's application.  </p>
                        <p dir="ltr">I'll have a look and see if I can
                          see where it would be used, but still as
                          you've said on one of my pulls, a half done
                          implementation is better than none.</p>
                        <p dir="ltr">Having the application throw a
                          missing method exception should not be the
                          recommended approach when we can add the
                          property and default it to false.</p>
                        <p dir="ltr">Thanks, and please don't think that
                          things won't getting better with my reviews. 
                          I'm learning what you want so I can review
                          better and help reduce the burden on you and
                          your staff.</p>
                        <span><font color="#888888">
                            <p dir="ltr">Martin</p>
                          </font></span>
                        <div>
                          <div>
                            <div class="gmail_quote">On 20 Oct 2014
                              20:04, "Miguel de Icaza" <<a href="mailto:miguel@xamarin.com" target="_blank">miguel@xamarin.com</a>>
                              wrote:<br type="attribution">
                              <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
                                <div dir="ltr">
                                  <div class="gmail_extra">
                                    <div class="gmail_quote">
                                      <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
                                        <p dir="ltr">As for the
                                          properties, although they
                                          should do something to the
                                          generated urls, simply adding
                                          them should surely be a valid
                                          pull?  the issue at the moment
                                          is that without them, you get
                                          an exception even if it should
                                          be false.  I actually think
                                          that these are used by other
                                          classes when generating urls,
                                          not the route collection
                                          itself, but I don't know for
                                          sure.  Considering that adding
                                          them is very low risk, can we
                                          not just accept the pull and
                                          ask for further work.</p>
                                      </blockquote>
                                      <div>Nope, all they do is allow
                                        some code to be compiled, and
                                        then get the wrong result.</div>
                                      <div><br>
                                      </div>
                                      <div>You might as well remove the
                                        dependency of those properties,
                                        and see what else breaks on
                                        whatever piece of code you are
                                        trying to build.</div>
                                      <div><br>
                                      </div>
                                      <div>Miguel</div>
                                    </div>
                                  </div>
                                </div>
                              </blockquote>
                            </div>
                          </div>
                        </div>
                      </blockquote>
                    </div>
                    <br>
                  </div>
                </div>
              </div>
              <br>
              _______________________________________________<br>
              Mono-devel-list mailing list<br>
              <a href="mailto:Mono-devel-list@lists.ximian.com" target="_blank">Mono-devel-list@lists.ximian.com</a><br>
              <a href="http://lists.ximian.com/mailman/listinfo/mono-devel-list" target="_blank">http://lists.ximian.com/mailman/listinfo/mono-devel-list</a><br>
              <br>
            </blockquote>
          </div>
          <br>
        </div>
      </div>
      <br>
      <fieldset></fieldset>
      <br>
      <pre>_______________________________________________
Mono-devel-list mailing list
<a href="mailto:Mono-devel-list@lists.ximian.com" target="_blank">Mono-devel-list@lists.ximian.com</a>
<a href="http://lists.ximian.com/mailman/listinfo/mono-devel-list" target="_blank">http://lists.ximian.com/mailman/listinfo/mono-devel-list</a>
</pre>
    </blockquote>
    <br>
  </div></div></div>

</blockquote></div><br></div></div>