Introducing ASP.NET Layout Renderer base class

[epignosisx]

This is an initial commit for #20. There is still more work to be done. I just wanted to share and get some initial feedback.

• Introduce AspNetLayoutRendererBase
• Introduce IHttpContextAccessor
• Move existing layout renderers to use new base class
• Add unit tests to existing layout renderers now that mocking HttpContext is easier.

fixes #20

[codecov-io]

Current coverage is 46.47%

Merging #22 into master will increase coverage by +36.89% as of aec90a2

@@            master     #22   diff @@
======================================
  Files           12      14     +2
  Stmts          146     142     -4
  Branches        34      29     -5
  Methods          0       0       
======================================
+ Hit             14      66    +52
- Partial          2       6     +4
+ Missed         130      70    -60

Review entire Coverage Diff as of aec90a2

---

Uncovered Suggestions

1. +4.23% via ...ringTargetWrapper.cs#194...199
2. +3.53% via ...ringTargetWrapper.cs#166...170
3. +3.53% via ...ringTargetWrapper.cs#88...92
4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

[304NotModified]

Looks really nice! No comments :)

[epignosisx]

I'm going to need to take a dependency on a mocking library in order to mock HttpContext. I looked at NLog/NLog but we are not using one. Do you have any preference?

[304NotModified]

Sorry about the delay.

I prefer Moq, but I think NSubstitute is also fine.

[epignosisx]

I'll do Moq then. Moq used to be my favorite but the simplicity of NSubstitute won me over.

[304NotModified]

Well if NSubstitute is in your opinion is a lot easier to read/write, maybe that's a better choice. (because I prefer that also others can add unit tests easily) :)

[epignosisx]

@304NotModified done with all the tasks. Let me know what you think.

[304NotModified]

I'm doubting if this is backwards-compatible when we create a subclass of AspNetItemValueLayoutRenderer. There are two cases:

1 ) override the full behavior: before override Append. Now we need to override DoAppend
2) Call base.Append - this still works, this has now also other behaviour

What do you think?

[epignosisx]

Maybe I'm missing something, but I dont see a backward-compatibility issue. Any subclass that calls base.Append will still experience the same behavior, the implementation changed but the behavior remains the same. If the subclass doesn't call base.Append then it is opting out of the base class behavior.
The DoAppend method is optional to override, but I would add to the docs that is the recommended way to write aspnet layout renderers that rely on the HttpContext since it makes the layout renderers more unit test friendly.

[304NotModified]

Impressive! Just one note about backwards-compatibility.

Really good work!

https://codecov.io/github/NLog/NLog.Web/NLog.Web/LayoutRenderers?ref=aec90a22a34670165d5f1ca134591f6f46c6cd43

[epignosisx]

@304NotModified have u had time to review my comment on your concern about backward compatibility.

[304NotModified]

@epignosisx will look at this now. Thanks for the reminder

[304NotModified]

I made two examples, and indeed, both behave the same:

        private class AspNetSessionIDLayoutRenderer2 : AspNetSessionIDLayoutRenderer
        {

            protected override void Append(StringBuilder builder, LogEventInfo logEvent)
            {
                builder.Append("before");
                //before: AspNetSessionIDLayoutRenderer.append() 
                //after: AspNetLayoutRendererBase.append() -> AspNetSessionIDLayoutRenderer.DoAppend 
                base.Append(builder, logEvent);
                builder.Append("after");
            }
        }

        private class AspNetSessionIDLayoutRenderer3 : AspNetSessionIDLayoutRenderer
        {

            protected override void Append(StringBuilder builder, LogEventInfo logEvent)
            {
                builder.Append("fixed");
            }
        }

I needed a double check, because of semver.