Avro
  1. Avro
  2. AVRO-1214

Generated protocol's method should return void instead of Void like one-way message

    Details

    • Type: Improvement Improvement
    • Status: Patch Available
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 1.7.2
    • Fix Version/s: 1.8.0
    • Component/s: java
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change

      Description

      Using the following IDL:

      @namespace("org.apache.avro.test")
      protocol Simple {
        error TestError {
          string message;
        }
      
        string hello(string greeting);
        void `error`() throws TestError;
        void ping() oneway;
      }
      

      Will produce the interface:

      
      package org.apache.avro.test;
      
      public interface Simple {
        public static final org.apache.avro.Protocol PROTOCOL = org.apache.avro.Protocol.parse("...");
        java.lang.CharSequence hello(java.lang.CharSequence greeting) throws org.apache.avro.AvroRemoteException;
        java.lang.Void error() throws org.apache.avro.AvroRemoteException, org.apache.avro.test.TestError;
        void ping();
      
        public interface Callback extends Simple {
          public static final org.apache.avro.Protocol PROTOCOL = org.apache.avro.test.Simple.PROTOCOL;
          void hello(java.lang.CharSequence greeting, org.apache.avro.ipc.Callback<java.lang.CharSequence> callback) throws java.io.IOException;
          void error(org.apache.avro.ipc.Callback<java.lang.Void> callback) throws java.io.IOException;
        }
      }
      

      Then one is forced to add return null; statement(s) in the interface implementation for the error() method which can be a bit cumbersome and that's not the case for the oneway ping() method.
      This is fine on the Callback though because the developer can just ignore the callback argument.

        Activity

        Hide
        Sébastien Launay added a comment -

        You are right Apache versioning would require a 2.x major version for that, I can update the patch to add an option to the compiler/maven-plugin like the one for generating String instead of CharSequence.
        Were you thinking of an option to keep Void turned off by default or an option to generate void turned off by default for 1.8.x?

        Show
        Sébastien Launay added a comment - You are right Apache versioning would require a 2.x major version for that, I can update the patch to add an option to the compiler/maven-plugin like the one for generating String instead of CharSequence. Were you thinking of an option to keep Void turned off by default or an option to generate void turned off by default for 1.8.x?
        Hide
        Doug Cutting added a comment -

        This looks like a good change, but it is incompatible and will break applications that currently implement interfaces that return Void, so I marked it for inclusion in 1.8.0, where we might include such incompatible changes. Even then, breaking existing applications is never good. We could implement this as an option to the compiler, so that folks can choose whether or not to generate interfaces that return Void for compatibility.

        Show
        Doug Cutting added a comment - This looks like a good change, but it is incompatible and will break applications that currently implement interfaces that return Void, so I marked it for inclusion in 1.8.0, where we might include such incompatible changes. Even then, breaking existing applications is never good. We could implement this as an option to the compiler, so that folks can choose whether or not to generate interfaces that return Void for compatibility.
        Hide
        Sébastien Launay added a comment -

        Patch for using void instead of Void for two-ways message.
        Method SpecificCompiler#javaUnbox(Schema used in the Velocity template has been kept for compatibility with existing custom templates but is now deprecated in favour of SpecificCompiler#javaUnbox(Schema, boolean.

        This changes generates incompatible Java interface so I guess it should be targeted for 1.8.x.

        Patch is attached but can be found here as well:
        https://github.com/slaunay/avro/commits/enhancement/AVRO-1214-unbox-void

        Show
        Sébastien Launay added a comment - Patch for using void instead of Void for two-ways message. Method SpecificCompiler#javaUnbox(Schema used in the Velocity template has been kept for compatibility with existing custom templates but is now deprecated in favour of SpecificCompiler#javaUnbox(Schema, boolean . This changes generates incompatible Java interface so I guess it should be targeted for 1.8.x. Patch is attached but can be found here as well: https://github.com/slaunay/avro/commits/enhancement/AVRO-1214-unbox-void

          People

          • Assignee:
            Unassigned
            Reporter:
            Sébastien Launay
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development