On 2 November 2014 14:26, Kornel Pal <kornel...@gmail.com> wrote: > Hi, > > I've noticed that new functionality is going into the wrappers, while in > my opinion that the functionality belongs to HttpRequest and HttpResponse: > > - HttpRequestBase.ReadEntityBodyMode: returning 0 instead of > ReadEntityBodyMode.Classic made more sense > - HttpRequestWrapper.ReadEntityBodyMode: wrapper is not supposed to > implement functionality, that belongs to HttpRequest > > 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.
> > - HttpRequestWrapper.Abort: wrapper is not supposed to implement > functionality, that belongs to HttpRequest > > 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 BeginGetRequestStream or BeginGetResponse to be called, so it really needs a fair bit of work. > > - HttpResponseWrapper.ClientDisconnectedToken: wrapper is not supposed > to implement functionality, that belongs to HttpResponse > > This should be an easy fix, to simply do the return CancellationToken.None from the HttpResponse instead. > > > As for the GetBuffer* methods: > > - Mono already implements HttpRequest.GetBufferlessInputStream, so > hard coding ReadEntityBodyMode.Classic limits Mono's compatibility > > 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. > > - Implementing GetBufferedInputStream based on the existing > GetBufferlessInputStream implementation doesn't seem to be too difficult > > Difficulty is in the eye of the beholder. > > - 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 > > 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 ASP.NET. 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. > > > Thank you. > > Kornel > > > On 11/2/2014 1:52 AM, Miguel de Icaza wrote: > > >> PR1349: https://github.com/mono/mono/pull/1349 >> >> *This is the machine key work, and needs a small tweak before it can be >> merged that I will do this week. * >> > > I believe the TODO can be removed. Can you do that? See comments on > pull request. > > >> PR1363: https://github.com/mono/mono/pull/1363 >> >> *Another of mine with the MembershipPasswordAttribute * >> > > Do you mind resending this? It can no longer be auto-merged from the UI. > > >> PR1365: https://github.com/mono/mono/pull/1365 >> >> *This is Kornel Pal's around the HttpTaskAsyncHandler, and Miguel has >> said he'll take a look at it. * >> > > Manually aded > > >> PR1370: https://github.com/mono/mono/pull/1370 >> >> *Small one implementing a default of the ReadEntityBodyMode * >> > > Got this one by hand. > > >> PR1371: https://github.com/mono/mono/pull/1371 >> >> *Another small one, implementing the ClientDisconnectedToken * >> > > And this one automatically. > > >> PR1372: https://github.com/mono/mono/pull/1372 >> >> *A final small one around the GetBuffer* methods in the httprequest. * >> > > I do not like this one. Is there a reason we can not implement the > required functionality instead? > > Miguel > >> 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. >> >> Anyway, after applying all of these, my large WebAPI solution not only >> compiles, but it also runs! >> >> 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). >> >> https://github.com/martinjt/mono/tree/mvc_allfixes >> >> Thanks for everyone's help. >> >> Martin >> >> On 20 October 2014 20:42, Martin Thwaites <monofo...@my2cents.co.uk> >> wrote: >> >>> Hi Miguel, >>> >>> 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. >>> >>> 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. >>> >>> 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. >>> >>> 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. >>> >>> Martin >>> On 20 Oct 2014 20:04, "Miguel de Icaza" <mig...@xamarin.com> wrote: >>> >>>> 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. >>>>> >>>> Nope, all they do is allow some code to be compiled, and then get the >>>> wrong result. >>>> >>>> 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. >>>> >>>> Miguel >>>> >>> >> >> _______________________________________________ >> Mono-devel-list mailing list >> Mono-devel-list@lists.ximian.com >> http://lists.ximian.com/mailman/listinfo/mono-devel-list >> >> > > > _______________________________________________ > Mono-devel-list mailing > listMono-devel-list@lists.ximian.comhttp://lists.ximian.com/mailman/listinfo/mono-devel-list > > >
_______________________________________________ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
