Vulnerability in EPiServer.Forms
Title says it all, doesn't it? Absolutely every publicly available service class that has further dependencies on other EPiServer service classes should have a simple interface so they are mockable for tests etc.
Adding it is done in a few minutes, and will save people countless hours in the long run.
It's never few minutes :) You know how many EPiServer employees are needed to add and interface on top of concrete class? :)
A problem with interfaces is that they do not support versioning (you can e.g not add an additonal method without causing a breaking change). A class is also an abstraction as long as it have virtual methods and the usage of the class is through injection (that is do not new it) and hence it should be possible to Mock.
However I can agree that we in many cases have classes that are both the abstraction and the implemenation. Some of these classes have further dependencies in their constructor which makes them sligthly harder to mock (you need to pass in correct numbers of arguments). This can be solved with an abstract baseclass (without dependencies) and then an actual implemetation (which might be internal), this is a pattern we try to use nowadays.
The exact implementation needed to solve the problem isn't really important. The important part here is that I should be able to inject something into my own service classes that I can mock in some way to test things. My initial statement still stands, it will save people countless hours in just fiddling about with structuremap-setups for testing etc.
For simple mocking, a class must have a parameterless constructor. So if all constructors need a bunch of other classes, the problem is just halfway solved - the class will technically be mockable, but it's gonna be boring monkeywork to mock all its dependencies (which in turn might require the same setup, possibly specific for every test case).
So that could also be a potential fix: give the classes an empty constructor, and force Structuremap to use the right one when running properly (aka as a web application, not via Nunit or other test frameworks)
I agree that the mocking should be as simple as possible.
We made a huge step in testability in CMS7, but absolutely there are more to do. For example have abstract classes or default constructors on classes. We have quite a huge code base so it is hard to change everything at once but we try to continuosly improve in testablitiy.
I totally agree that that was a huge step forward from 6 to 7. Arve will Fakes or FakeItEasy or similar lib come handy in these cases? Not sure yet (haven't tested) but there are libs that mock entire class even it's not behind an interface..
The example that triggered this (as far as I remember) was the ContentAssetHelper. And I think the UrlResolver is kindof quirky to manage too.
I'm using Moq for my mocking. There might be other frameworks that can do these things, but the "default requirements" for being mockable is either to be an interface or to have an empty constructor.
Please add the classes that you find most annoying to Mock then we can prioritize them when we work further on with increasing testability. You mentioned ContentAssetHelper and UrlResolver. I guess TemplateResolver might be one also?
I haven't tried to mock the TemplateResolver (yet?), but might be.
I can try and update this thread with the relevant classes as I find them. If I am to prioritize them:
I have now created a tech story that we should look over some of those classes. I guess we might take this story when we are approaching a 8 version since that makes it possible to do some minor breaking things (which might be needed to solve this in a way we think is appropriate, perhaps break the classes into an abstract class and a implementation class).