-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a builtin JSON serializer #80
Comments
@CableZa, can you explain why you need this functionality? My thinking is that from the end-user's perspective, the serialization format is not really relevant, so long as objects serialize and deserialize. Is there a particular use case that, for example, the XML serializer isn't meeting? |
Oh, and I should've mentioned before, it's always possible for clients to plug in their own serializers, so it's possible (in the interim, or forever) to benefit from a custom serializer without building it into the main package. |
@blairconrad I think I need to rewind my commits to see exactly why, but it may have had something to do with the support for Task<>. I will see if I can get it working with the XML/Binary serializer. Otherwise I just have a personal preference for JSON over XML, the recording files then matchup closer to what the real API's return. |
@blairconrad I see now why the builtin serializers didnt work for me, because they require the Serializable attribute is added to the input/output classes for the method being recorded. The Newtonsoft JSON serializer doesn't have this requirement... |
Hi, @CableZa. For actual "production usage", the only interface I fake is public interface IComicFetcher
{
string GetContent(Uri url);
} So, not all primitives, but You're serializing a lot of custom classes? Earlier you'd said
so I imagined (just imagined) that you were mostly dealing with services that returned stringy data that were already serialized JSON or the like. |
The services do indeed return JSON strings representing some DTO, but the code where I'm faking/recording the dependency expects the deserialized type. I have apicontrollers that take in one or more of these services via constructor injection: public class MyController : APIController
{
public MyController(IExternalAPI externalAPI)
{
this.externalAPI = externalAPI;
}
public async Task<IHttpActionResult> GetStuff(int stuffId)
{
var stuffFromExternalAPI = externalAPI.GetStuff(stuffId);
return Ok(stuffFromExternalAPI);
}
}
public interface IExternalAPI
{
Stuff GetStuff(int stuffId);
}
public class Stuff
{
public int Id {get;set;}
public MoreDetails NestedInfo {get;set;}
} So I'm keen to use SelfInitializingFakes to allow testing 'MyController' with a faked/recorded IExternalAPI. |
That clears some things up, @CableZa. The example is really helpful. You're encountering a gap because this is not the intended use of the library. The goal is to replace the remote calls, Quoting Fowler:
I think I see why you'd try to self-initialize a fake
I'm not unsympathetic to your plight, but I just can't yet support adding a JSON serializer for this use case. Especially the Newtonsoft JSON serializer, which would mean bringing in an additional dependency for all users, even if they don't care about JSON serialization. |
Thanks for clarifying @blairconrad. Out of interest do you use any similar approach to record & playback database calls? |
I've never done. Of course, I only use SelfInitializingFakes for one (toy) project. I imagine it would technically work, but I'm not sure it's needed. You'd be trading a database call to a read from the filesystem (I assume). Like remote services, databases can be a little slower than other operations, but generally I'd think they'd be more reliable, so I'm not sure there's that much benefit. |
Split from #79, as proposed by @CableZa.
We currently have support for XML and Binary serialization only.
The text was updated successfully, but these errors were encountered: