THRIFT-2632 add "validate" option to generate read/write validation code

Client: PHP
Patch: Stig Bakken & Jens Geyer

Modifications made to the original pull request:
- moved TestValidators.* to lib/php/test
- created new TestValidators.thrift to house the UnionOfStrings union
- modified makefiles accordingly

This closes #159

compiler/cpp/src/generate/t_php_generator.cc

@@ -70,6 +70,9 @@ class t_php_generator : public t_oop_generator {
     iter = parsed_options.find("oop");
     oop_ = (iter != parsed_options.end());
 
+    iter = parsed_options.find("validate");
+    validate_ = (iter != parsed_options.end());
+
     iter = parsed_options.find("nsglobal");
     if(iter != parsed_options.end()) {
       nsglobal_ = iter->second;
@@ -118,6 +121,12 @@ class t_php_generator : public t_oop_generator {
   void generate_php_struct_reader(std::ofstream& out, t_struct* tstruct);
   void generate_php_struct_writer(std::ofstream& out, t_struct* tstruct);
   void generate_php_function_helpers(t_function* tfunction);
+  void generate_php_struct_required_validator(ofstream& out, t_struct* tstruct, std::string method_name, bool write_mode);
+  void generate_php_struct_read_validator(ofstream& out, t_struct* tstruct);
+  void generate_php_struct_write_validator(ofstream& out, t_struct* tstruct);
+  bool needs_php_write_validator(t_struct* tstruct);
+  bool needs_php_read_validator(t_struct* tstruct);
+  int get_php_num_required_fields(const vector<t_field*>& fields, bool write_mode);
 
   void generate_php_type_spec(std::ofstream &out, t_type* t);
   void generate_php_struct_spec(std::ofstream &out, t_struct* tstruct);
@@ -363,6 +372,11 @@ class t_php_generator : public t_oop_generator {
    */
   bool oop_;
 
+  /**
+   * Whether to generate validator code
+   */
+  bool validate_;
+
   /**
    * Global namespace for PHP 5.3
    */
@@ -817,6 +831,12 @@ void t_php_generator::generate_php_struct_definition(ofstream& out,
 
   generate_php_struct_reader(out, tstruct);
   generate_php_struct_writer(out, tstruct);
+  if (needs_php_read_validator(tstruct)) {
+    generate_php_struct_read_validator(out, tstruct);
+  }
+  if (needs_php_write_validator(tstruct)) {
+    generate_php_struct_write_validator(out, tstruct);
+  }
 
   indent_down();
   out <<
@@ -837,8 +857,15 @@ void t_php_generator::generate_php_struct_reader(ofstream& out,
   scope_up(out);
 
   if (oop_) {
-    indent(out) << "return $this->_read('" << tstruct->get_name() << "', self::$_TSPEC, $input);" << endl;
+    if (needs_php_read_validator(tstruct)) {
+      indent(out) << "$tmp = $this->_read('" << tstruct->get_name() << "', self::$_TSPEC, $input);" << endl;
+      indent(out) << "$this->_validateForRead();" << endl;
+      indent(out) << "return $tmp;" << endl;
+    } else {
+      indent(out) << "return $this->_read('" << tstruct->get_name() << "', self::$_TSPEC, $input);" << endl;
+    }
     scope_down(out);
+    out << endl;
     return;
   }
 
@@ -936,6 +963,11 @@ void t_php_generator::generate_php_struct_reader(ofstream& out,
       "$xfer += $input->readStructEnd();" << endl;
   }
 
+  if (needs_php_read_validator(tstruct)) {
+    indent(out) <<
+      "$this->_validateForRead();" << endl;
+  }
+
   indent(out) <<
     "return $xfer;" << endl;
 
@@ -963,9 +995,14 @@ void t_php_generator::generate_php_struct_writer(ofstream& out,
   }
   indent_up();
 
+  if (needs_php_write_validator(tstruct)) {
+    indent(out) << "$this->_validateForWrite();" << endl;
+  }
+
   if (oop_) {
     indent(out) << "return $this->_write('" << tstruct->get_name() << "', self::$_TSPEC, $output);" << endl;
     scope_down(out);
+    out << endl;
     return;
   }
 
@@ -1038,9 +1075,78 @@ void t_php_generator::generate_php_struct_writer(ofstream& out,
     indent() << "return $xfer;" << endl;
 
   indent_down();
-  out <<
-    indent() << "}" << endl <<
-    endl;
+  out << indent() << "}" << endl << endl;
+}
+
+void t_php_generator::generate_php_struct_read_validator(ofstream& out,
+                                                          t_struct* tstruct) {
+  generate_php_struct_required_validator(out, tstruct, "_validateForRead", false);
+}
+
+void t_php_generator::generate_php_struct_write_validator(ofstream& out,
+                                                          t_struct* tstruct) {
+  generate_php_struct_required_validator(out, tstruct, "_validateForWrite", true);
+}
+
+void t_php_generator::generate_php_struct_required_validator(ofstream& out,
+                                                             t_struct* tstruct,
+                                                             std::string method_name,
+                                                             bool write_mode) {
+  indent(out) <<
+    "private function " << method_name << "() {" << endl;
+  indent_up();
+
+  const vector<t_field*>& fields = tstruct->get_members();
+
+  if (fields.size() > 0) {
+    vector<t_field*>::const_iterator f_iter;
+
+    for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
+      t_field* field = (*f_iter);
+      if (field->get_req() == t_field::T_REQUIRED ||
+          (field->get_req() == t_field::T_OPT_IN_REQ_OUT && write_mode)) {
+        indent(out) <<
+          "if ($this->" << field->get_name() << " === null) {" << endl;
+        indent_up();
+        indent(out) <<
+          "throw new TProtocolException('Required field " <<
+          tstruct->get_name() << "." << field->get_name() << " is unset!');" << endl;
+        indent_down();
+        indent(out) <<
+          "}" << endl;
+      }
+    }
+  }
+
+  indent_down();
+  indent(out) << "}" << endl << endl;
+}
+
+int t_php_generator::get_php_num_required_fields(const vector<t_field*>& fields,
+                                                 bool write_mode) {
+  int num_req = 0;
+
+  if (fields.size() > 0) {
+    vector<t_field*>::const_iterator f_iter;
+    for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
+      if ((*f_iter)->get_req() == t_field::T_REQUIRED ||
+          ((*f_iter)->get_req() == t_field::T_OPT_IN_REQ_OUT && write_mode)) {
+        ++num_req;
+      }
+    }
+  }
+  return num_req;
+}
+
+bool t_php_generator::needs_php_write_validator(t_struct* tstruct) {
+  return (validate_ &&
+          !tstruct->is_union() &&
+          get_php_num_required_fields(tstruct->get_members(), true) > 0);
+}
+
+bool t_php_generator::needs_php_read_validator(t_struct* tstruct) {
+  return (validate_ &&
+          (get_php_num_required_fields(tstruct->get_members(), false) > 0));
 }
 
 /**
@@ -2577,5 +2683,6 @@ THRIFT_REGISTER_GENERATOR(php, "PHP",
 "    oop:             Generate PHP with object oriented subclasses\n"
 "    rest:            Generate PHP REST processors\n"
 "    nsglobal=NAME:   Set global namespace\n"
+"    validate:        Generate PHP validator methods\n"
 )
 

lib/php/test/Makefile.am

@@ -19,15 +19,27 @@
 
 THRIFT = $(top_builddir)/compiler/cpp/thrift
 
-stubs: ../../../test/ThriftTest.thrift
+stubs: ../../../test/ThriftTest.thrift  TestValidators.thrift
 	mkdir -p ./packages
 	$(THRIFT) --gen php -r --out ./packages ../../../test/ThriftTest.thrift
+	mkdir -p ./packages/phpv
+	mkdir -p ./packages/phpvo
+	$(THRIFT) --gen php:validate     -r --out ./packages/phpv   TestValidators.thrift 
+	$(THRIFT) --gen php:validate,oop -r --out ./packages/phpvo  TestValidators.thrift 
+	
+check-validator: stubs 
+	php Test/Thrift/TestValidators.php 
+	php Test/Thrift/TestValidators.php -oop 
 
+check-protocol:	stubs
 if HAVE_PHPUNIT
-check: stubs
 	$(PHPUNIT) --log-junit=phpunit.xml Test/Thrift/Protocol/TestTJSONProtocol.php
 	$(PHPUNIT) --log-junit=phpunit.xml Test/Thrift/Protocol/TestBinarySerializer.php
 endif
+	
+check: stubs \
+  check-protocol \
+  check-validator
 
 clean-local:
 	$(RM) -r ./packages

lib/php/test/Test/Thrift/TestValidators.php

@@ -0,0 +1,142 @@
+<?php
+/*
+ * 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.
+ */
+
+namespace test\php;
+
+require_once __DIR__.'/../../../lib/Thrift/ClassLoader/ThriftClassLoader.php';
+
+use Thrift\ClassLoader\ThriftClassLoader;
+use Thrift\Exception\TProtocolException;
+use Thrift\Protocol\TBinaryProtocol;
+use Thrift\Transport\TMemoryBuffer;
+
+$oop_mode = (isset($argv[1]) && $argv[1] === '-oop');
+$GEN_DIR = $oop_mode ? 'phpvo' : 'phpv';
+
+$loader = new ThriftClassLoader();
+$loader->registerNamespace('Thrift', __DIR__ . '/../../../lib');
+$loader->registerDefinition('ThriftTest', __DIR__ . '/../../packages/' . $GEN_DIR);
+$loader->registerDefinition('TestValidators', __DIR__ . '/../../packages/' . $GEN_DIR);
+$loader->register();
+
+// Would be nice to have PHPUnit here, but for now just hack it.
+
+set_exception_handler(function($e) {
+    my_assert(false, "Unexpected exception caught: " . $e->getMessage());
+});
+
+set_error_handler(function($errno, $errmsg) {
+    my_assert(false, "Unexpected PHP error: " . $errmsg);
+});
+
+// Empty structs should not have validators
+assert_has_no_read_validator('ThriftTest\EmptyStruct');
+assert_has_no_write_validator('ThriftTest\EmptyStruct');
+
+// Bonk has only opt_in_req_out fields
+{
+    assert_has_no_read_validator('ThriftTest\Bonk');
+    assert_has_a_write_validator('ThriftTest\Bonk');
+    {
+        // Check that we can read an empty object
+        $bonk = new \ThriftTest\Bonk();
+        $transport = new TMemoryBuffer("\000");
+        $protocol = new TBinaryProtocol($transport);
+        $bonk->read($protocol);
+    }
+    {
+        // ...but not write an empty object
+        $bonk = new \ThriftTest\Bonk();
+        $transport = new TMemoryBuffer();
+        $protocol = new TBinaryProtocol($transport);
+        assert_protocol_exception_thrown(function() use ($bonk, $protocol) { $bonk->write($protocol); },
+                                         'Bonk was able to write an empty object');
+    }
+}
+
+// StructA has a single required field
+{
+    assert_has_a_read_validator('ThriftTest\StructA');
+    assert_has_a_write_validator('ThriftTest\StructA');
+    {
+        // Check that we are not able to write StructA with a missing required field
+        $structa = new \ThriftTest\StructA();
+        $transport = new TMemoryBuffer();
+        $protocol = new TBinaryProtocol($transport);
+        assert_protocol_exception_thrown(function() use ($structa, $protocol) { $structa->write($protocol); },
+                                         'StructA was able to write an empty object');
+    }
+    {
+        // Check that we are able to read and write a message with a good StructA
+        $transport = new TMemoryBuffer(base64_decode('CwABAAAAA2FiYwA='));
+        $protocol = new TBinaryProtocol($transport);
+        $structa = new \ThriftTest\StructA();
+        $structa->read($protocol);
+        $structa->write($protocol);
+    }
+}
+
+// Unions should not get write validators
+assert_has_no_write_validator('TestValidators\UnionOfStrings');
+
+function assert_has_a_read_validator($class) {
+    my_assert(has_read_validator_method($class),
+              $class . ' class should have a read validator');
+}
+
+function assert_has_no_read_validator($class) {
+    my_assert(!has_read_validator_method($class),
+              $class . ' class should not have a read validator');
+}
+
+function assert_has_a_write_validator($class) {
+    my_assert(has_write_validator_method($class),
+              $class . ' class should have a write validator');
+}
+
+function assert_has_no_write_validator($class) {
+    my_assert(!has_write_validator_method($class),
+              $class . ' class should not have a write validator');
+}
+
+function assert_protocol_exception_thrown($callable, $message) {
+    try {
+        call_user_func($callable);
+        my_assert(false, $message);
+    } catch (TProtocolException $e) {
+    }
+}
+
+function has_write_validator_method($class) {
+    $rc = new \ReflectionClass($class);
+    return $rc->hasMethod('_validateForWrite');
+}
+
+function has_read_validator_method($class) {
+    $rc = new \ReflectionClass($class);
+    return $rc->hasMethod('_validateForRead');
+}
+
+function my_assert($something, $message) {
+    if (!$something) {
+        fwrite(STDERR, basename(__FILE__) . " FAILED: $message\n");
+        exit(1);
+    }
+}

lib/php/test/TestValidators.thrift

@@ -0,0 +1,28 @@
+/*
+ * 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.
+ */
+
+namespace php TestValidators
+ 
+include "../../../test/ThriftTest.thrift"
+
+union UnionOfStrings { 
+  1: string aa; 
+  2: string bb; 
+} 
+