alexbeletsky / elmah-mvc

Painless integration of ELMAH into ASP.NET MVC application
http://nuget.org/packages/Elmah.MVC
Apache License 2.0
266 stars 61 forks source link

Json and Xml action methods are missing #3

Closed Wolfenator closed 12 years ago

Wolfenator commented 12 years ago

Thanks for the nuget package!

Please add the following http gets to the Elmah controller class: (These are links available in the Error Details section) See also:

Raw/Source data in XML or in JSON

    /// <summary>
    /// Gets the JSON instance for the error detail see also area.
    /// </summary>
    /// <returns></returns>
    public ActionResult json()
    {
        return new ElmahResult("json");
    }

    /// <summary>
    /// Gets the XML instance for the error detail see also area.
    /// </summary>
    /// <returns></returns>
    public ActionResult Xml()
    {
        return new ElmahResult("xml");
    }

I don't know if you're actually editing the config file too yet if you are, could you change the Elmah path to be: path="/admin/elmah.axd" so when you uncomment the authorize attribute on your controller users cannot access the ~/elmah.axd as an unauthorized user. Thanks!

alexbeletsky commented 12 years ago

Hi Wolfenator,

Thanks for info!

  1. New actions - I'll definately do that, haven't seen that is in use.
  2. I'm not modifying config file. Unfortunately, I haven't find a way how to "delete" or "update" something in .config using NuGet transformation. I suggest just delete this elmah.axd Http Handler from config at all.. It have to be done manually.

Please refer for a demo application that uses Elmah.MVC and check web.config.

If you might have ideas of better web.config transformation, I would be happy to hear ! :)

Wolfenator commented 12 years ago

Thanks for the quick response!

Removing the http handler in the two locations of the web.config file is a heck of a lot better answer then mine was! :)

Not to be too nit picky, but I was reviewing your code and found a section of code that is doing two casts to type IHttpAsyncHandler in the ExecuteResult method.

Please see here: http://www.boyet.com/Articles/DoubleCastingAntiPattern.html for a more in-depth explanation.

I propose the following code change to avoid casting twice with the "is" and then the explicit (IHttpAsyncHandler) cast.

        var httpHandler = factory.GetHandler(currentContext, null, null, null);

        var httpAsyncHandler = httpHandler as IHttpAsyncHandler;
        if (httpAsyncHandler != null)
        {
            httpAsyncHandler.BeginProcessRequest(currentContext, r => { }, null);
            return;
        }

        httpHandler.ProcessRequest(currentContext);

I hope you find this suggestion helpful!

alexbeletsky commented 12 years ago

This is really great suggestion, I'm gonna apply that :)

Dude, I would suggest you to fork and do pull request - that's exactly the way how open source on Github works! :))

Wolfenator commented 12 years ago

Ya, I'll let you keep your baby wrapped up tight and in your own hands. That plus I have enough work of my own to do! :-)

Thanks for making other's life better w/ your contributions!