Ability to extend or implement external type that is an interface or class with no-arg, default constructor

[johanneshiry]

Hello again :-)

it has been a while but we still make extensive use of this library.
Today we discovered some flaws that relate to the implementation of shared objects / inheritance in the config generation.
The main issue is, that defined parent classes using #@define abstract do not implement java.io.Serializable and hence their serialization is prevented which might be required in some situations (which we are currently facing when we want to setup an akka cluster and send the configuration around).

So when we discussed this in our team we came up with three different approaches that would solve our issue, but we would love to hear your @carueda opinion on this and which one you would prefer to have handed in as a PR.

1. Always extend abstract class with java.io.Serializable
   The most easiest approach is to just add java.io.Serializable to all classes marked as #@define abstract. AFAIK this wouldn't break anything but just adds serialization capabilities to all abstract superclasses.

2. Extend the check to allow java.io.Serializable as parent in ModelBuilder:168
   This would be another way to allow java.io.Serializable by exclude it from the superclass check. With this adaption, we would allow something like

#@define extends java.io.Serializable
ChildStruct {
  c: string
  d: string
}

if a struct needs to be serializable.

3. Fully remove the check on existing parent classes in ModelBuilder:168
   This would be the biggest possible change that would not only solve our problem but somehow introduce a new feature. Without this check it would be possible to mix in any trait that is available in the existing codebase because we do not check anymore if the superclass is also defined within the config definition template. This would leverage the capabilities of superclasses to a new level and would allow for a huge flexibility. However, this would also introduce the possibility of non-compiling code as we lose control over the resulting code and cannot guarantee anymore that the resulting config class compiles as it may depend on third-party code at the specific project.

That said I'm open to provide a PR for one these solutions if you would like to.

Looking forward on your feedback and further discussions.

Best,

Johannes

[carueda]

hi! Thanks for the continued use and interest.

In principle it all sounds like a desirable feature, but let me take take a closer look to assess what this would entail, at least to some level.

[carueda]

@johanneshiry

I just pushed an initial exploration in this branch : https://github.com/carueda/tscfg/tree/issue73 . Please have a look. Thx.

[johanneshiry]

@carueda
that was blazingly fast - thank you. I took a look and I think this looks quite good. At least it seems that is solves our issue and as far as I can tell does not break anything ... I think we can give this approach a try - if it breaks something I think we can always report this back ASAP.

I wanted to add some minor stylistic stuff (in particular removing the braces of the interface extension) but I cannot push on the issue73 branch. Would you like me to hand in a PR for this via fork or can you give me contributor permissions? I'm fine with both ways - just let me know what you prefer.

[carueda]

Thanks @johanneshiry -- just added you as collaborator. Yes, let's continue the discussion around that branch. As you may have noticed there are a couple of pieces that need proper resolution, including: 1) determine that something is external; 2) determine that something is an interface so an implements is used instead of extends for the java case . I have some ideas (like using a special character prefix to indicate that something is external (like #@define extends !java.foo.Baz); and, to avoid too many changes, some other special prefix for when something is an interface ...

[johanneshiry]

Cool, thanks.

Considering your questions I'll try to find some time within the next few days in order to provide some proposals considering the open points. In particular, I see some difficulties when providing an external class or interface which requires specific implementations. Haven't thought about this in detail yet, but that might cause some difficulties and may not be so error prone as we lose kind of control to ensure everything is correctly provided. But maybe I just miss something here. Will report back as soon as I made some tests.

[carueda]

I wouldn't worry too much about "control" -- the new mechanisms are simply provided for extra flexibility but it is really up to the user to make sure their use is correct, that is, that everything will eventually compile correctly. It is not tscfg's responsibility to necessarily incorporate logic for all added extensions and related verifications; that would be unmanageable (and unnecessary imo).

[johanneshiry]

I thought about different scenarios and this is how far I've come:

determine that something is external

I like the ! as external class indicator

determine that something is an interface or an abstract class

I would prefer using a familiar syntax. Why not doing the following:

• for internal #define extends java.foo.Baz and #define implements java.foo.Baz (last one might not be needed though)
• for external #define implements !java.foo.Baz and #define extends !java.foo.Baz

This would not break existing stuff, but would allow for a clean way of adding implements/abstract class support for internal and external parent classes.

Use cases / potential scenarios

I further have the following thoughts on different use cases/scenarios:

external java interfaces

• interfaces with methods cannot be implemented as methods are not overriden
• interfaces methods does not make sense as the config object only holds data structures

--> only empty interfaces w/o any methods are possible as application case

external java abstract classes

• field vals required in parent class needs to be marked with ! additionally in order to call super(...) with the marked values + must have the same order and name as called superclass constructor
• implemented methods in parent class are accessible
• abstract methods cannot be supported (wouldn't compile)

--> abstract classes are of interest as they would allow a mixin of methods to config objects

external scala traits

• field vals need to be marked with ! in order to add override + have the same name as in trait
• traits must be free of unimplemented methods as only fields can be overriden/determined

external scala abstract classes

• field vals need to be marked with ! in order to add override + must have the same order and name as called superclass constructor
• parent abstract super class must not have abstract field vals, but only constructor args (or the other way around, but both is not possible)
• implemented methods in parent class are accessible
• abstract methods cannot be supported (wouldn't compile)

I hope I didn't miss anything here and looking forward on your comments and thoughts on this.

[johanneshiry]

Any thoughts on this @carueda ?

[carueda]

Sorry, extra busy these days.

I may still have to have a closer look at your comments, but ...

I like the ! as external class indicator

Ok. Let's use it.

#define implements !java.foo.Baz

Ok, but maybe just for Java generation

--> only empty interfaces w/o any methods are possible as application case

Yes. I'd suggest that we first allow this use case, that is, the implementation (or extending) of so-called "marker" interfaces/traits (i.e., types with no methods).

I see a significant risk of adding too much complexity if attempting to tackle the other cases. My reaction is that that's actually, intentionally if you like, beyond the scope of tscfg.

--> abstract classes are of interest as they would allow a mixin of methods to config objects
...

Hmm, I have my reservations. I mean, all of this is in principle feasible, but do we really want to address the associated complexity? Perhaps a possible middle-ground "solution" is to allow the generation of an abstract wrapper, that can be extended by application code ...

BTW, FWIW, I have used that mechanism in some projects, that is, I have a generated CFG wrapper which I extend to i) perform some extra validation at construction time (eg., involving regexes), ii) add convenience methods/fields..

Thoughts? Shall we proceed with just allowing the marker types (which btw would address this particular issue), at least as an initial, concrete PR?

[carueda]

Any thoughts @johanneshiry ?

[johanneshiry]

hey @carueda - sorry for not reporting back. I'm afraid I hadn't had a chance to work on this since our last talk as I kept busy by PhD stuff ;-)

With regard to your comments:
I'm fine with everything you wrote.

In particular you're right wrt the complexity. I may have been a little bit overmotivated :D

tl;dr:
I like the idea to just allow the market types and will work on this as soon as I have some time to focus on this. Hopefully during my vacations in august.

Regarding your automated CFG wrapper: is this generic and open source? Or is this all project specific? I could imagine that having automated wrapper generation + validation could be interesting for other people as well :)

[carueda]

Ok, let's see how/when time becomes a bit more available to us. I'll try to look into this again and eventually push something ...

Re that CFG wrapper I mentioned, it is just the one generated by tscfg. What I said is that, in some specific projects, I have created subclasses of that wrapper to add some additional verifications and methods, that's it.

BTW, just came across this https://www.scala-sbt.org/contraband/schema.html, which made me think, again, about #51, and, in fact, has aspects related with our discussion here. In general, I've been thinking about options like GraphQL (and even TypeScript) to possibly base a proper syntax for tscfg schemas. Contraband is inspired by GraphQL and it does seem to incorporate some nice features. Something to keep in mind.

Hope all is going well with your PhD, @johanneshiry !

[carueda]

Hey @johanneshiry

Changed the title of the issue to better reflect the initial intent and the feature eventually implemented.

In summary:

• "!" prefix used to indicate an external extension name
• Besides extends now also implements accepted
• Examples:
  • https://github.com/carueda/tscfg/blob/master/src/main/tscfg/example/issue73a.spec.conf
    with #@define abstract extends !java.lang.Object
  • https://github.com/carueda/tscfg/blob/master/src/main/tscfg/example/issue73b.spec.conf
    with #@define abstract implements !java.io.Serializable

BTW, not documented in the README yet. Also, additional tests would be great.