You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by je...@apache.org on 2014/07/25 00:14:52 UTC
git commit: THRIFT-2632 add "validate" option to generate read/write
validation code Client: PHP Patch: Stig Bakken & Jens Geyer
Repository: thrift
Updated Branches:
refs/heads/master 16e2ed25a -> 577f407df
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
Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/577f407d
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/577f407d
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/577f407d
Branch: refs/heads/master
Commit: 577f407df96ffe15177b5435ba99db56ae0129d8
Parents: 16e2ed2
Author: Jens Geyer <je...@apache.org>
Authored: Wed Jul 23 19:04:12 2014 +0200
Committer: Jens Geyer <je...@apache.org>
Committed: Fri Jul 25 00:13:27 2014 +0200
----------------------------------------------------------------------
compiler/cpp/src/generate/t_php_generator.cc | 115 +++++++++++++++++-
lib/php/test/Makefile.am | 16 ++-
lib/php/test/Test/Thrift/TestValidators.php | 142 ++++++++++++++++++++++
lib/php/test/TestValidators.thrift | 28 +++++
4 files changed, 295 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/thrift/blob/577f407d/compiler/cpp/src/generate/t_php_generator.cc
----------------------------------------------------------------------
diff --git a/compiler/cpp/src/generate/t_php_generator.cc b/compiler/cpp/src/generate/t_php_generator.cc
index 1eb759c..2d8fe00 100644
--- a/compiler/cpp/src/generate/t_php_generator.cc
+++ b/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);
@@ -364,6 +373,11 @@ class t_php_generator : public t_oop_generator {
bool oop_;
/**
+ * Whether to generate validator code
+ */
+ bool validate_;
+
+ /**
* Global namespace for PHP 5.3
*/
std::string nsglobal_;
@@ -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"
)
http://git-wip-us.apache.org/repos/asf/thrift/blob/577f407d/lib/php/test/Makefile.am
----------------------------------------------------------------------
diff --git a/lib/php/test/Makefile.am b/lib/php/test/Makefile.am
index 1292b81..a529d8c 100755
--- a/lib/php/test/Makefile.am
+++ b/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
http://git-wip-us.apache.org/repos/asf/thrift/blob/577f407d/lib/php/test/Test/Thrift/TestValidators.php
----------------------------------------------------------------------
diff --git a/lib/php/test/Test/Thrift/TestValidators.php b/lib/php/test/Test/Thrift/TestValidators.php
new file mode 100644
index 0000000..41e95fb
--- /dev/null
+++ b/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);
+ }
+}
http://git-wip-us.apache.org/repos/asf/thrift/blob/577f407d/lib/php/test/TestValidators.thrift
----------------------------------------------------------------------
diff --git a/lib/php/test/TestValidators.thrift b/lib/php/test/TestValidators.thrift
new file mode 100644
index 0000000..f994a9e
--- /dev/null
+++ b/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;
+}
+