THRIFT-4236 Support context in go generated code

Client: Go
Patch: taozle <[email protected]>

This closes apache#1309

Author: taozle

Dockerfile

@@ -50,7 +50,7 @@ RUN buildDeps=" \
        /thrift \
     && cmake --build . --config Release \
     && make install \
-    && curl -k -sSL "https://storage.googleapis.com/golang/go1.8.3.linux-amd64.tar.gz" -o /tmp/go.tar.gz \
+    && curl -k -sSL "https://storage.googleapis.com/golang/go1.5.2.linux-amd64.tar.gz" -o /tmp/go.tar.gz \
     && tar xzf /tmp/go.tar.gz -C /tmp \
     && cp /tmp/go/bin/gofmt /usr/bin/gofmt \
     && apt-get purge -y --auto-remove $buildDeps \

build/docker/centos/Dockerfile

@@ -102,7 +102,7 @@ RUN curl -sSL http://packages.erlang-solutions.com/rpm/centos/erlang_solutions.r
       erlang-tools
 
 # Go Dependencies
-RUN curl -sSL https://storage.googleapis.com/golang/go1.8.3.linux-amd64.tar.gz | tar -C /usr/local/ -xz
+RUN curl -sSL https://storage.googleapis.com/golang/go1.4.3.linux-amd64.tar.gz | tar -C /usr/local/ -xz
 ENV PATH /usr/local/go/bin:$PATH
 
 # Haskell Dependencies

build/docker/debian/Dockerfile

@@ -155,7 +155,7 @@ RUN pip2 install -U ipaddress backports.ssl_match_hostname tornado
 RUN pip3 install -U backports.ssl_match_hostname tornado
 
 # Go
-RUN curl -sSL https://storage.googleapis.com/golang/go1.8.3.linux-amd64.tar.gz | tar -C /usr/local/ -xz
+RUN curl -sSL https://storage.googleapis.com/golang/go1.4.3.linux-amd64.tar.gz | tar -C /usr/local/ -xz
 ENV PATH /usr/local/go/bin:$PATH
 
 # Haxe

build/docker/ubuntu/Dockerfile

@@ -177,7 +177,7 @@ RUN pip2 install -U ipaddress backports.ssl_match_hostname tornado
 RUN pip3 install -U backports.ssl_match_hostname tornado
 
 # Go
-RUN curl -sSL https://storage.googleapis.com/golang/go1.8.3.linux-amd64.tar.gz | tar -C /usr/local/ -xz
+RUN curl -sSL https://storage.googleapis.com/golang/go1.4.3.linux-amd64.tar.gz | tar -C /usr/local/ -xz
 ENV PATH /usr/local/go/bin:$PATH
 
 # Haxe

compiler/cpp/src/thrift/generate/t_go_generator.cc

@@ -81,7 +81,7 @@ class t_go_generator : public t_generator {
     gen_package_prefix_ = "";
     package_flag = "";
     read_write_private_ = false;
-    use_context_ = false;
+    legacy_context_ = false;
     ignore_initialisms_ = false;
     for( iter = parsed_options.begin(); iter != parsed_options.end(); ++iter) {
       if( iter->first.compare("package_prefix") == 0) {
@@ -92,8 +92,8 @@ class t_go_generator : public t_generator {
         package_flag = (iter->second);
       } else if( iter->first.compare("read_write_private") == 0) {
         read_write_private_ = true;
-      } else if( iter->first.compare("use_context") == 0) {
-        use_context_ = true;
+      } else if( iter->first.compare("legacy_context") == 0) {
+        legacy_context_ = true;
       } else if( iter->first.compare("ignore_initialisms") == 0) {
         ignore_initialisms_ =  true;
       } else {
@@ -288,7 +288,7 @@ class t_go_generator : public t_generator {
   std::string gen_package_prefix_;
   std::string gen_thrift_import_;
   bool read_write_private_;
-  bool use_context_;
+  bool legacy_context_;
   bool ignore_initialisms_;
 
   /**
@@ -884,7 +884,10 @@ string t_go_generator::go_imports_begin(bool consts) {
       "\t\"database/sql/driver\"\n"
       "\t\"errors\"\n";
   }
-  if (use_context_) {
+  if (legacy_context_) {
+    extra +=
+      "\t\"golang.org/x/net/context\"\n";
+  } else {
     extra +=
       "\t\"context\"\n";
   }
@@ -904,20 +907,13 @@ string t_go_generator::go_imports_begin(bool consts) {
  * This will have to do in lieu of more intelligent import statement construction
  */
 string t_go_generator::go_imports_end() {
-  string extra;
-
-  if (use_context_) {
-    extra +=
-      "var _ = context.Background\n";
-  }
-
   return string(
       ")\n\n"
       "// (needed to ensure safety because of naive import list construction.)\n"
       "var _ = thrift.ZERO\n"
       "var _ = fmt.Printf\n"
+      "var _ = context.Background\n"
       "var _ = reflect.DeepEqual\n"
-      + extra +
       "var _ = bytes.Equal\n\n");
 }
 
@@ -1842,7 +1838,7 @@ void t_go_generator::generate_service_interface(t_service* tservice) {
 
     for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
       generate_go_docstring(f_types_, (*f_iter));
-      f_types_ << indent() << function_signature_if(*f_iter, "", true, use_context_) << endl;
+      f_types_ << indent() << function_signature_if(*f_iter, "", true, true) << endl;
     }
   }
 
@@ -2604,36 +2600,31 @@ void t_go_generator::generate_service_server(t_service* tservice) {
   // Generate the header portion
   string self(tmp("self"));
 
-  string processorFunction("thrift.TProcessorFunction");
-  if (use_context_) {
-    processorFunction = "thrift.TProcessorFunction2";
-  }
-
   if (extends_processor.empty()) {
     f_types_ << indent() << "type " << serviceName << "Processor struct {" << endl;
-    f_types_ << indent() << "  processorMap map[string]" << processorFunction << endl;
+    f_types_ << indent() << "  processorMap map[string]thrift.TProcessorFunction" << endl;
     f_types_ << indent() << "  handler " << serviceName << endl;
     f_types_ << indent() << "}" << endl << endl;
     f_types_ << indent() << "func (p *" << serviceName
-               << "Processor) AddToProcessorMap(key string, processor " << processorFunction << ") {"
+               << "Processor) AddToProcessorMap(key string, processor thrift.TProcessorFunction) {"
                << endl;
     f_types_ << indent() << "  p.processorMap[key] = processor" << endl;
     f_types_ << indent() << "}" << endl << endl;
     f_types_ << indent() << "func (p *" << serviceName
                << "Processor) GetProcessorFunction(key string) "
-                  "(processor "<< processorFunction << ", ok bool) {" << endl;
+                  "(processor thrift.TProcessorFunction, ok bool) {" << endl;
     f_types_ << indent() << "  processor, ok = p.processorMap[key]" << endl;
     f_types_ << indent() << "  return processor, ok" << endl;
     f_types_ << indent() << "}" << endl << endl;
     f_types_ << indent() << "func (p *" << serviceName
-               << "Processor) ProcessorMap() map[string]" << processorFunction << "{" << endl;
+               << "Processor) ProcessorMap() map[string]thrift.TProcessorFunction {" << endl;
     f_types_ << indent() << "  return p.processorMap" << endl;
     f_types_ << indent() << "}" << endl << endl;
     f_types_ << indent() << "func New" << serviceName << "Processor(handler " << serviceName
                << ") *" << serviceName << "Processor {" << endl << endl;
     f_types_
         << indent() << "  " << self << " := &" << serviceName
-        << "Processor{handler:handler, processorMap:make(map[string]" << processorFunction << ")}"
+        << "Processor{handler:handler, processorMap:make(map[string]thrift.TProcessorFunction)}"
         << endl;
 
     for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
@@ -2643,24 +2634,16 @@ void t_go_generator::generate_service_server(t_service* tservice) {
                  << "{handler:handler}" << endl;
     }
 
-    string ctxParam("");
-    string ctxVar("");
-    if (use_context_) {
-        ctxParam = "ctx context.Context, ";
-        ctxVar = "ctx, ";
-    }
-
     string x(tmp("x"));
     f_types_ << indent() << "return " << self << endl;
     f_types_ << indent() << "}" << endl << endl;
     f_types_ << indent() << "func (p *" << serviceName
-               << "Processor) Process(" << ctxParam
-               << "iprot, oprot thrift.TProtocol) (success bool, err "
+               << "Processor) Process(ctx context.Context, iprot, oprot thrift.TProtocol) (success bool, err "
                   "thrift.TException) {" << endl;
     f_types_ << indent() << "  name, _, seqId, err := iprot.ReadMessageBegin()" << endl;
     f_types_ << indent() << "  if err != nil { return false, err }" << endl;
     f_types_ << indent() << "  if processor, ok := p.GetProcessorFunction(name); ok {" << endl;
-    f_types_ << indent() << "    return processor.Process(" << ctxVar << "seqId, iprot, oprot)" << endl;
+    f_types_ << indent() << "    return processor.Process(ctx, seqId, iprot, oprot)" << endl;
     f_types_ << indent() << "  }" << endl;
     f_types_ << indent() << "  iprot.Skip(thrift.STRUCT)" << endl;
     f_types_ << indent() << "  iprot.ReadMessageEnd()" << endl;
@@ -2714,20 +2697,14 @@ void t_go_generator::generate_process_function(t_service* tservice, t_function*
   string argsname = publicize(tfunction->get_name() + "_args", true);
   string resultname = publicize(tfunction->get_name() + "_result", true);
 
-  string ctxParam("");
-  string ctxVar("");
-  if (use_context_) {
-      ctxParam = "ctx context.Context, ";
-      ctxVar = "ctx";
-  }
   // t_struct* xs = tfunction->get_xceptions();
   // const std::vector<t_field*>& xceptions = xs->get_members();
   vector<t_field*>::const_iterator x_iter;
   f_types_ << indent() << "type " << processorName << " struct {" << endl;
   f_types_ << indent() << "  handler " << publicize(tservice->get_name()) << endl;
   f_types_ << indent() << "}" << endl << endl;
   f_types_ << indent() << "func (p *" << processorName
-             << ") Process(" << ctxParam << "seqId int32, iprot, oprot thrift.TProtocol) (success bool, err "
+             << ") Process(ctx context.Context, seqId int32, iprot, oprot thrift.TProtocol) (success bool, err "
                 "thrift.TException) {" << endl;
   indent_up();
   f_types_ << indent() << "args := " << argsname << "{}" << endl;
@@ -2771,13 +2748,11 @@ void t_go_generator::generate_process_function(t_service* tservice, t_function*
   f_types_ << "err2 = p.handler." << publicize(tfunction->get_name()) << "(";
   bool first = true;
 
-  f_types_ << ctxVar;
+  f_types_ << "ctx";
   for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
     if (first) {
       first = false;
-      if (use_context_) {
-        f_types_ << ", ";
-      }
+      f_types_ << ", ";
     } else {
       f_types_ << ", ";
     }
@@ -3459,7 +3434,7 @@ string t_go_generator::function_signature(t_function* tfunction, string prefix)
  * Renders an interface function signature of the form 'type name(args)'
  *
  * @param tfunction Function definition
- * @param disableContext Client doesn't suppport context for now.
+ * @param enableContext Client doesn't suppport context for now.
  * @return String of rendered function definition
  */
 string t_go_generator::function_signature_if(t_function* tfunction, string prefix, bool addError, bool enableContext) {
@@ -3745,5 +3720,5 @@ THRIFT_REGISTER_GENERATOR(go, "Go",
                           "                     Disable automatic spelling correction of initialisms (e.g. \"URL\")\n" \
                           "    read_write_private\n"
                           "                     Make read/write methods private, default is public Read/Write\n" \
-                          "    use_context\n"
-                          "                     Make service method receive a context as first argument.\n")
+                          "    legacy_context\n"
+                          "                     Use legacy x/net/context instead of context in go<1.7.\n")

configure.ac

@@ -397,7 +397,7 @@ if test "$with_go" = "yes";  then
   AC_PATH_PROG([GO], [go])
   if [[ -x "$GO" ]] ; then
     AS_IF([test -n "$GO"],[
-      ax_go_version="1.7"
+      ax_go_version="1.4"
 
       AC_MSG_CHECKING([for Go version])
       golang_version=`$GO version 2>&1 | $SED -e 's/\(go \)\(version \)\(go\)\(@<:@0-9@:>@.@<:@0-9@:>@.@<:@0-9@:>@\)\(@<:@\*@:>@*\).*/\4/'`

doc/install/README.md

@@ -39,5 +39,5 @@ These are only required if you choose to build the libraries for the given langu
     * Bit::Vector
     * Class::Accessor
 * Haxe 3.1.3
-* Go 1.7
+* Go 1.4
 * Delphi 2010

lib/go/Makefile.am

@@ -31,10 +31,12 @@ install:
 	@echo '##############################################################'
 
 check-local:
-	$(GO) test -race ./thrift
+	GOPATH=`pwd` $(GO) get golang.org/x/net/context
+	GOPATH=`pwd` $(GO) test -race ./thrift
 
 all-local:
-	$(GO) build ./thrift
+	GOPATH=`pwd` $(GO) get golang.org/x/net/context
+	GOPATH=`pwd` $(GO) build ./thrift
 
 EXTRA_DIST = \
 	thrift \

lib/go/test/Makefile.am

@@ -18,7 +18,7 @@
 #
 
 THRIFT = $(top_builddir)/compiler/cpp/thrift
-THRIFTARGS = -out gopath/src/ --gen go:thrift_import=thrift
+THRIFTARGS = -out gopath/src/ --gen go:thrift_import=thrift,legacy_context
 THRIFTTEST = $(top_srcdir)/test/ThriftTest.thrift
 
 # Thrift for GO has problems with complex map keys: THRIFT-2063
@@ -57,6 +57,7 @@ gopath: $(THRIFT) $(THRIFTTEST) \
 	$(THRIFT) $(THRIFTARGS),read_write_private DontExportRWTest.thrift
 	$(THRIFT) $(THRIFTARGS),ignore_initialisms IgnoreInitialismsTest.thrift
 	GOPATH=`pwd`/gopath $(GO) get github.com/golang/mock/gomock
+	GOPATH=`pwd`/gopath $(GO) get golang.org/x/net/context
 	ln -nfs ../../../thrift gopath/src/thrift
 	ln -nfs ../../tests gopath/src/tests
 	cp -r ./dontexportrwtest gopath/src

lib/go/test/tests/go17.go

@@ -0,0 +1,47 @@
+// +build go1.7
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package tests
+
+import (
+	"context"
+	"fmt"
+)
+
+var defaultCtx = context.Background()
+
+type FirstImpl struct{}
+
+func (f *FirstImpl) ReturnOne(ctx context.Context) (r int64, err error) {
+	return 1, nil
+}
+
+type SecondImpl struct{}
+
+func (s *SecondImpl) ReturnTwo(ctx context.Context) (r int64, err error) {
+	return 2, nil
+}
+
+type impl struct{}
+
+func (i *impl) Hi(ctx context.Context, in int64, s string) (err error)        { fmt.Println("Hi!"); return }
+func (i *impl) Emptyfunc(ctx context.Context) (err error)                     { return }
+func (i *impl) EchoInt(ctx context.Context, param int64) (r int64, err error) { return param, nil }