Issues with DelegateClass.

[tomasmiguez]

We started using DelegateClass on our codebase recently, and are experiencing some compatibility issues with Oj. The main problem has already been reported here https://github.com/ohler55/oj/issues/477, but the proposed solution does not work for our use case. We currently use this options on the dump method calls

      options = {
        create_additions: false,
        mode: :compat,
        time_format: :xmlschema,
        second_precision: 0,
        use_to_hash: true
      }

but if we change it to compat, we have 2 problems:

• time_format is not valid for that mode, so we can't keep this change transparent.
• Decimal numbers are getting serialized as strings. We thought this might be fixed by setting compat_bigdecimal to false or true, but that did not work either.

How can we get delegate objects to serialize only the underlying object? And not the attributes of the delegate that are anecdotic?

For aditional context, Sequel uses Delegate classes in their implementation and this is causing our problems here: https://github.com/jeremyevans/sequel/discussions/2153

Thanks in advance, Tomas.

[ohler55]

Can you provide more information about what you are looking for. I got the basics but am not sure if the app is a rails app or something else. Also, what is the preferred mode? It sounds like you are not using rails or else the :compat time format would be okay.

If you can use some other mode that does not try to be compatible with the JSON gem there are a few more options to deal with a custom delegate marshalling.

[tomasmiguez]

Sure!

Yes, it's not a rails app, we use plain Ruby, with a routing microframework called Cuba.

The mode we are currently using is custom, as it's the one that allowed us to get the parsing as we wanted it.

Based on that, it would probably be allright to use a mode that doesn't try to be JSON gem compatible.

[ohler55]

Would it be possible to monkey-patch SimpleDelegate to add a as_json() or to_hash() method? If so then that method could just call the delegate.

[tomasmiguez]

Yes, we have currently a patch based on that idea, but we where looking for cleaner alternatives.

The Sequel gem currently offers a module called JSONObject, which all the classes that inherit from ClassDelegator and we are interested in include. We are monkeypatching it with a method like

def to_hash
  __getobj__
end

and with the option use_to_hash is working.

But we have some concerns with this approach. The first one is conceptual, those delegate inner objects are not necessarily a Hash, so we are breaking a very basic Ruby contract. The other is more practical, when onboarding someone to our codebase, it will for sure be a source of frustration understanding how and why is the dumping to JSON working, or when it fails how to fix it. It is also hard to mantain, as we would have to monkeypatch each single class we are interested in that is a delegator.

This would be fixed by patching Delegator with this method (which is not correct as it won't return a Hash), but we try to not patch core Ruby classes, as it usually leads to conflicts in definitions down the road, even more when talking about a really generic method as to_hash. It also wouldn't fix the conceptual problem.

as_json has the same issues.

[ohler55]

I would advise against just returning __getobj__ because as you pointed out it is not a hash. as_json would be better but again. as you pointed out monkey patching is inherently dangerous and especially so on core classes. We are hemmed in by the language and the classes though. Let me see if I can break down the issue a bit more. What you want is something in Oj that when it sees an object that has a class that inherits from Delegate it runs some special code. Right now Oj supports special or odd encoding based on class but not based on inherited class. Brainstorming it kind of sounds like implementing a parallel class inheritance with associated methods is what you are looking for. I'm not sure to implement that right now. It is an interesting train of thought to follow but non-trivial.

[tomasmiguez]

Yea, that's exactly it.

I did not follow you on the bit about the parallel class inheritance, but looking at the code and if you were open to make modifications to Oj to address this issue, I was thinking about an option that when set checks for Delegate with rb_obj_is_kind_of around here and just calls oj_dump_custom_val on the delegate object if it is a delegated object. But I guess that might not be a generic enough solution, and I have no clue about the possible performance implications of such a change.

[ohler55]

If every call to dump an object had to check for whether the instance was a type of delegate that would be expensive. If a more generic solution was offered it would be worse. I do not want to make a special case for Delegates but a low performance overhead option might be to provide a hook that is called for each object. If the developer was willing to sacrifice performance they could define hook for what ever classes they wanted.

[tomasmiguez]

Yes, that makes sense. The check would have been behind the option, so the cost of it would only have been payed by those interested in this behaviour. But I can see how a hook to modify the object can be a more generic solution for these issue.

Although, now that I think about it, this can be done outside of Oj as well, like before serialization you could ask if the object is a delegate and in that case serialize the internal object instead of the delegate.

[ohler55]

It could be done outside Oj but the before hook/code would have to be able to traverse all object attributes to look for delegates and then create a duplicate tree of objects. It will be expensive unless you have a very specific case where delegates are alway top level objects.

[tomasmiguez]

Yea you are right, for our current use case we only need to check the top level object, but that won't help for a general solution.

Would you be open to a PR submission exploring the hooks idea from where we could iterate a solution?

[ohler55]

As long as it covers the general case, that would be great as long as you don't mind me being a bit picky.