Skip to content
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

Compiling pdsc to java bindings includes interfaces #52

Open
kevinkyyro opened this issue Aug 6, 2015 · 3 comments
Open

Compiling pdsc to java bindings includes interfaces #52

kevinkyyro opened this issue Aug 6, 2015 · 3 comments
Labels
proposal Proposals for new features or functionality

Comments

@kevinkyyro
Copy link

It could be really useful to introduce some additional abstraction when dealing with models that include some common properties.

For instance, consider the following records

{
  "name": "Person",
  "type": "record",
  // ...
  "fields": [
    { "name": "username", "type": "string" },
    { "name": "thumbnail", "type": "image" },
    { "name": "dateAdded", "type": "timestamp" },
    { "name": "email", "type": "string" },
    // ...
  ]
}

{
  "name": "User",
  "type": "record",
  // ...
  "include": [ "Person" ],
  "fields": [
    // user-specific fields
  ]
}

{
  "name": "Admin",
  "type": "record",
  // ...
  "include": [ "Person" ],
  "fields": [
    // admin-specific fields
  ]
}

I am not aware of a way to treat Users and Admins as Persons currently. I experience pain around this issue when I am dealing with unions that have common, deeply-nested fields. We have no good way to reduce repetition in this code in a type-safe manner.

If Person was compiled to an interface, and User and Admin implement it, then the generated implementations aught to look basically the same and client code could manipulate these records more abstractly.

I cannot say this wasn't avoidable at the time of the original design, but now it's tough to get around and having common interfaces would alleviate some of this pain.

@kevinkyyro
Copy link
Author

Not sure how to label this an enhancement, but that's what it was intended to be...

@CrendKing
Copy link
Contributor

So what you are suggesting is that for models that are included by any other model, we will need to generate an interface class, making the class itself and whichever includes it implements it. This is discussed internally in our team, and I think we do have some use cases to justify it. But there are technical problems.

First, until now, every named schema will generate exactly one template file. What I mean is, if you have a record Foo, you will get Foo.java. (If Foo internally embed another record Bar, we consider them two separate named schemas, and there will be two java files) Since our data model processor guarantees that within the effective scope there will be no duplicate name, and since the schema to generated template is one-to-one mapping, we automatically have the guarantee that all generated template files are out of duplicate. Now if we not only generate Foo.java for record Foo, but also a Foo_interface.java, what if there is a legitimate record named Foo_interface in the scope? This breaks the nice guarantee we used to have, and could potentially give users confusion when dealing with large amount of pdsc files.

Second, Rest.li has not only Java binding, but also bindings for many other languages. We try to make the binding of every language feels homogeneous to each other. That means if we introduce feature in data layer (which is supposed to be language agnostic) that is specifically designed for one language, then many other languages which do not support such feature will suffer inconsistency. I know majority of languages have concepts of interface/protocol/abstract class, but there is no guarantee every single language we will add binding to will.

@kevinkyyro kevinkyyro changed the title Compile pdsc includes to interfaces Compiling pdsc to java bindings includes interfaces May 4, 2016
@kevinkyyro
Copy link
Author

@CrendKing so I can think of at least one way to guarantee are no collisions, but it wouldn't be backward source compatible.

The idea is to compile a pdsc such as Person.pdsc to an interface with a static implementation within it. Person.java might contain something along the lines of:

interface Person { // extends whatever it ought to
  String getName();
  Person setName(String name);
  // etc.

  // the Person implementation, also extending whatever other base classes/interfaces it currently would
  class Record implements Person {
    // an implementation that looks very similar to current generated classes
  }

  // optional: static factory methods
  static Foo create() { return new Record(); }
  static Foo create(String name, Param anotherField) { return new Record(name, anotherField); }
}

Then, if Admin.pdsc includes Person, you get something like:

interface Admin extends Person { // And any other interfaces for included types
  // admin-specific fields

  class Record implements Admin {
    // ...
  }

  // etc
}

There might be some holes in this other than logistical issues with shipping a breaking change such as this and the general complexity of implementing this feature request. For instance, there might be certain inheritance structures that are permitted in a pdsc but would be illegal in java; I am not familiar enough with the specifications of pdscs to determine if there are any concrete examples.

I don't know how the various layers of rest.li are structured, but it seems to me that language-specific bindings should not need to be concerned with the capabilities of other languages, so long as they serialize and deserialize appropriately to maintain compatibility across bindings.

All that said, I don't have a stake in rest.li at the moment since I no longer use it in my day job, so I would not be upset if this is closed, but I think it would be a nice improvement and people would appreciate it.

@CrendKing CrendKing added the proposal Proposals for new features or functionality label May 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal Proposals for new features or functionality
Projects
None yet
Development

No branches or pull requests

2 participants