mbraceproject / FsPickler

A fast multi-format message serializer for .NET
http://mbraceproject.github.io/FsPickler/
MIT License
324 stars 52 forks source link

CustomPickler forces other projects to reference FsPickler #32

Closed Bananas-Are-Yellow closed 9 years ago

Bananas-Are-Yellow commented 9 years ago

I have an F# record and one of its fields is the result of an expensive calculation derived from the other fields. Currently this is not lazy, although it might be in the future, but even so the idea is to avoid performing the calculation more than once.

The field is quite a large data structure, so I don't want to save it. Instead I want to recalculate it when I restore the object. Since this is an immutable F# record, ISerializable and DataContract are unsuitable, so the CustomPickler attribute seems like the only way to go.

The problem is that even if the static CreatePickler method is made private, calling projects need to include FsPickler due to the IPicklerResolver method argument. The compiler error is:

error FS0074:
The type referenced through 'Nessos.FsPickler.IPicklerResolver' is defined in an assembly that is not referenced.
You must add a reference to assembly 'FsPickler'.

My calling projects are not concerned with data serialization, so it doesn't seem right that they should have to include FsPickler. Can you think of a way around this problem?

David.

eiriktsarpalis commented 9 years ago

Unfortunately, no. You cannot define a custom pickler without depending on a whole lot of FsPickler types. If you want to stick with POCOs you should probably use patterns like this.

Bananas-Are-Yellow commented 9 years ago

One solution would be for the CustomPickler attribute to take an optional argument specifying an ancillary type that provides the CreatePickler method. Then this ancillary type could be internal so that it does not affect calling projects.

Bananas-Are-Yellow commented 9 years ago

I'm imagining something like this:

type internal BananaHelper = class end

[<CustomPickler(typeof<BananaHelper>)>]
type Banana = {
   ...
}

...

type BananaHelper with
    static member CreatePickler (resolver: IPicklerResolver) =
        ...

Any good?

eiriktsarpalis commented 9 years ago

The problem with using factories is that it doesn't work with generic types. You need to somehow recover the type parameters and potential constraints in the factory method body. Also, serialization usually requires access to internal state which is not given when using an external factory.

The example that you give uses an extension method, which are is not recoverable using reflection on the declaring type.

Bananas-Are-Yellow commented 9 years ago

Actually, that's supposed to be an intrinsic extension, as it's in the same namespace declaration group. I'm already doing that in order to place my CreatePickler method later in the file so that it can make use of a module that occurs after the type. This works fine (the method does get called).

Can't the helper type have the same type parameters?

eiriktsarpalis commented 9 years ago

No, for example the code:

type FooAttribute(t : System.Type) =
    inherit System.Attribute()

[<FooAttribute(typeof<Foo<'T>>)>]
type Foo<'T when 'T : equality> = class end

would cause the warning warning FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'T has been constrained to be type 'obj'.

I didn't know about intrinsic extensions. Pretty cool!

Bananas-Are-Yellow commented 9 years ago

I think it should be:

typeof<Foo<_>>

That's usually how you get the generic type.

Bananas-Are-Yellow commented 9 years ago

Wait ... I think I've got that wrong. But there is a way ...

Bananas-Are-Yellow commented 9 years ago

This is how:

typedefof<Foo<_>>

For example, in F# Interactive, try this:

typedefof<List<_>>.IsGenericTypeDefinition;;

The result is true.

eiriktsarpalis commented 9 years ago

The problem with this approach is that you need to explicitly replicate the generic parameters of the type definition. For instance:

[<CustomPickler(typedefof<FooFactory<_,_>>)>]
type Foo<'T, 'S when 'T : equality and 'S :> IEnumerable> = ...

and FooFactory<'T, 'S when 'T : equality and 'S :> IEnumerable> () = ...

which is awkward and unsafe.

I doubt this would help solve your problem, since the type definition still carries a dependency on FsPickler, namely the CustomPickler attribute.

Bananas-Are-Yellow commented 9 years ago

Actually it does solve the problem. I commented out my CreatePickler methods, but left the CustomPickler attribute, and the calling project builds without the reference to FsPickler.

Matching the parameter constraints is not unreasonable. You often have to do that in general.

eiriktsarpalis commented 9 years ago

What about runtime? Can you initialize the type if FsPickler.dll is missing?

In any case I don't think this qualifies as POCO, so it is extremely unlikely I will be including a feature like this.

Bananas-Are-Yellow commented 9 years ago

FsPickler.dll won't be missing. It is used by the project that does the pickling, so it will be gathered up as a required DLL in the usual way. I'm just trying to avoid my calling projects having to reference FsPickler when they should not have to know how I've chosen to implement serialization. That's reasonable, isn't it? I'm trying to encapsulate my use of FsPickler.

For some reason, even though I've made CreatePickler private (which your reflection code supports), and therefore the calling project cannot see any requirements on FsPickler types, the compiler nevertheless requires the project reference due to the argument and return types of the private static method.

I've seen this 'trick' with an attribute specifying a companion type done before, but I can't remember the example right now. Hopefully it will come to me.

I don't understand your comments about POCO.

Bananas-Are-Yellow commented 9 years ago

The TypeConverter attribute is similar to what I am describing, but I'm not sure what the motivation is for separating out the conversion into a separate type. I'm sure this is a standard design pattern.

eiriktsarpalis commented 9 years ago

Yes, it's pretty common but intentionally not employed here.

POCO stands for Plain Old CLR Object, an object whose definition does not depend on any third-party framework. It's pretty similar to what you're trying to do. Adding a CustomPickler attribute stops it from being a POCO.

Bananas-Are-Yellow commented 9 years ago

I don't understand your reluctance. It doesn't add any runtime overhead, because CreatePickler is called only once for the type, and calling it on a companion type is no slower. The argument to CustomPickler can be optional, so it doesn't affect other people. Any type parameter constraints are easy enough to copy and paste (and maintain) when creating the companion type, and these are compile-time, so they do not affect calling the method using reflection. Don't you agree that my calling projects should not have to reference FsPickler?

It's not as if I'm stuck if you don't do this, but it seems like a good idea and a small amount of work.

By the way, as I've mentioned, I'm using F# records, so there is no issue with the companion type accessing fields.

eiriktsarpalis commented 9 years ago

It's not an issue of runtime overhead. It's an issue of design. Adding CustomPickler makes the type definition depend on FsPickler, there's no denying that. Hiding this fact under the carpet and providing the implicit assurance that the dll will be available at runtime is not sane design.

Bananas-Are-Yellow commented 9 years ago

I'm not sure we are on the same page, so let me summarize:

ProjectA contains types Banana (public) and BananaHelper (internal).

Banana has the CustomPickler attribute, and BananaHelper provides the CreatePickler static method. So ProjectA has to reference FsPickler in order to build, and the output folder will contain FsPickler.dll.

ProjectB references ProjectA and makes use of Banana. ProjectB should not have to reference FsPickler, because the implementation of serialization for types inside ProjectA ought to be encapsulated. This is not the case today, because the requirement to provide the CreatePickler method on the Banana class itself means FsPickler types escape ProjectA, which is what I'm trying to solve.

If ProjectB had some reason to want to deal with Pickler<Banana>, then of course it would add a reference to FsPickler, but even then it is of no concern to ProjectB whether pickling of Banana is done using a custom pickler. In my case, ProjectB provides end-user functionality using types in ProjectA, so ProjectB does not care about pickling at all.

Since ProjectB references ProjectA, the build process will put all DLLs for ProjectA into ProjectB's output folder, including FsPickler.dll.

The only part I can understand that you might have concerns about is whether ProjectB will require a reference to FsPickler just because Banana carries the CustomPickler attribute. My experimentation shows that this is not the case, and this is probably because the attribute is not a compile-time statement about the Banana type. Attributes attach data to be accessed at runtime.

By the way, regarding the constraints, since BananaHelper provides the pickler for Banana and it will therefore read its fields, I think the compiler will force the constraints on BananaHelper to match (or more) the constraints on Banana.

eiriktsarpalis commented 9 years ago

Well, not quite. Consider the example:

namespace Foo

[<Nessos.FsPickler.CustomPickler>]
type Foo = class end

Try compiling the above with reference to FsPickler and then isolate the assembly. Now from F# interactive, try the following:

#r "foo.dll"

typeof<Foo.Test>.GetCustomAttributes(true)

This will result in a runtime exception. Attributes are bound to their type definitions at compile time.

Bananas-Are-Yellow commented 9 years ago

By isolate the assembly, I assume you mean place foo.dll in a folder on its own without FsPickler.dll. Yes, if I do that, I get an exception. If FsPickler.dll is present, it works fine. But why would FsPickler.dll not be present in practice?

The output folder for ProjectB will contain all of the DLLs for ProjectA, including FsPickler.dll. I am not saying that ProjectB should be able to function at runtime without FsPickler.dll being present. Of course it can't. All I'm saying is that ProjectB should not have to reference the FsPickler library at build time.

Bananas-Are-Yellow commented 9 years ago

Referring to my original posting, I am still puzzled as to why a private static method in ProjectA should affect ProjectB at all. I just tried a private instance method instead, and I get the same problem. So I tried a C# version of ProjectA instead, and it worked! There was no requirement to add the reference to FsPickler in ProjectB.

Bananas-Are-Yellow commented 9 years ago

I've posted a question on Stack Overflow about this:

F# compiler requires project reference, but method is private

eiriktsarpalis commented 9 years ago

This has been partially addressed as of 1.2.5, closing.