You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by dr...@apache.org on 2009/02/07 03:36:36 UTC
svn commit: r741824 - in /incubator/thrift/trunk: ./
compiler/cpp/src/generate/ contrib/ contrib/fb303/cpp/ contrib/fb303/if/
contrib/fb303/java/ if/ lib/cpp/ lib/cpp/src/
lib/java/src/org/apache/thrift/reflection/ lib/py/ lib/py/src/reflection/
test/
Author: dreiss
Date: Sat Feb 7 02:36:35 2009
New Revision: 741824
URL: http://svn.apache.org/viewvc?rev=741824&view=rev
Log:
THRIFT-314. Purge reflection.limited
This was a feature designed to allow a Thrift server to report
information about its interface. However, the feature has
significant design problems, and is presence is currently causing
confusion without doing any good. Therefore, it is being removed.
It will always be in source control if and when we are ready to
come back to it.
Removed:
incubator/thrift/trunk/if/
incubator/thrift/trunk/lib/cpp/src/reflection_limited_types.cpp
incubator/thrift/trunk/lib/cpp/src/reflection_limited_types.h
incubator/thrift/trunk/lib/java/src/org/apache/thrift/reflection/
incubator/thrift/trunk/lib/py/src/reflection/
incubator/thrift/trunk/test/ReflectionTest.cpp
Modified:
incubator/thrift/trunk/Makefile.am
incubator/thrift/trunk/compiler/cpp/src/generate/t_cpp_generator.cc
incubator/thrift/trunk/configure.ac
incubator/thrift/trunk/contrib/fb303/cpp/FacebookBase.cpp
incubator/thrift/trunk/contrib/fb303/cpp/FacebookBase.h
incubator/thrift/trunk/contrib/fb303/if/fb303.thrift
incubator/thrift/trunk/contrib/fb303/java/FacebookBase.java
incubator/thrift/trunk/contrib/thrift.spec
incubator/thrift/trunk/lib/cpp/Makefile.am
incubator/thrift/trunk/lib/py/setup.py
incubator/thrift/trunk/test/DebugProtoTest.thrift
incubator/thrift/trunk/test/DenseLinkingTest.thrift
incubator/thrift/trunk/test/Makefile.am
Modified: incubator/thrift/trunk/Makefile.am
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/Makefile.am?rev=741824&r1=741823&r2=741824&view=diff
==============================================================================
--- incubator/thrift/trunk/Makefile.am (original)
+++ incubator/thrift/trunk/Makefile.am Sat Feb 7 02:36:35 2009
@@ -1,6 +1,6 @@
ACLOCAL_AMFLAGS = -I ./aclocal
-SUBDIRS = compiler/cpp lib if test
+SUBDIRS = compiler/cpp lib test
## Don't run make dist from a subversion working copy
## because it will pull in your .svn directories.
EXTRA_DIST = bootstrap.sh cleanup.sh doc test tutorial contrib \
Modified: incubator/thrift/trunk/compiler/cpp/src/generate/t_cpp_generator.cc
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/compiler/cpp/src/generate/t_cpp_generator.cc?rev=741824&r1=741823&r2=741824&view=diff
==============================================================================
--- incubator/thrift/trunk/compiler/cpp/src/generate/t_cpp_generator.cc (original)
+++ incubator/thrift/trunk/compiler/cpp/src/generate/t_cpp_generator.cc Sat Feb 7 02:36:35 2009
@@ -40,9 +40,6 @@
iter = parsed_options.find("include_prefix");
use_include_prefix_ = (iter != parsed_options.end());
- iter = parsed_options.find("reflection_limited");
- gen_reflection_limited_ = (iter != parsed_options.end());
-
out_dir_base_ = "gen-cpp";
}
@@ -94,10 +91,6 @@
void generate_process_function (t_service* tservice, t_function* tfunction);
void generate_function_helpers (t_service* tservice, t_function* tfunction);
- void generate_service_limited_reflector(t_service* tservice);
- bool generate_type_limited_reflection(t_type* ttype, std::string target);
- bool generate_simple_type_limited_reflection(std::ostream& out, t_type* ttype, std::string target);
-
/**
* Serialization constructs
*/
@@ -195,11 +188,6 @@
std::string get_include_prefix(const t_program& program) const;
/**
- * True iff we should generate limited reflectors for services.
- */
- bool gen_reflection_limited_;
-
- /**
* True iff we should generate local reflection metadata for TDenseProtocol.
*/
bool gen_dense_;
@@ -266,7 +254,6 @@
// Include base types
f_types_ <<
"#include <Thrift.h>" << endl <<
- "#include <reflection_limited_types.h>" << endl <<
"#include <protocol/TProtocol.h>" << endl <<
"#include <transport/TTransport.h>" << endl <<
endl;
@@ -1336,10 +1323,6 @@
generate_function_helpers(tservice, *f_iter);
}
-
- if (gen_reflection_limited_) {
- generate_service_limited_reflector(tservice);
- }
}
/**
@@ -1359,21 +1342,6 @@
f_header_ <<
indent() << "virtual ~" << service_name_ << "If() {}" << endl;
- if (gen_reflection_limited_) {
- f_header_ <<
- indent() << "static void getStaticLimitedReflection" <<
- "(apache::thrift::reflection::limited::Service & _return);" << endl;
- // TODO(dreiss): Uncomment and test this if we decide we need
- // a virtual function with this effect.
- //f_header_ <<
- // indent() << "virtual void getVirtualLimitedReflection" <<
- // "(apache::thrift::reflection::limited::Service & _return) ";
- //scope_up(f_header_);
- //f_header_ <<
- // indent() << "getStaticLimitedReflection(_return);" << endl;
- //scope_down(f_header_);
- }
-
vector<t_function*> functions = tservice->get_functions();
vector<t_function*>::iterator f_iter;
for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
@@ -2158,166 +2126,6 @@
}
/**
- * Helper function for generate_service_limited_reflector.
- * Generates a reflection of a single simple type.
- *
- * @param ttype The type to reflect
- * @param target The name of the lvalue to reflect onto
- * @return true iff the type really is simple
- *
- * Note: don't let this function output anything unless it is going to return true.
- */
-bool t_cpp_generator::generate_simple_type_limited_reflection(ostream & out, t_type* ttype, string target) {
- if (ttype->is_base_type()) {
- string type;
- switch (((t_base_type*)ttype)->get_base()) {
- case t_base_type::TYPE_VOID : type = "T_VOID;" ; break;
- case t_base_type::TYPE_STRING : type = "T_STRING;" ; break;
- case t_base_type::TYPE_BOOL : type = "T_BOOL;" ; break;
- case t_base_type::TYPE_BYTE : type = "T_BYTE;" ; break;
- case t_base_type::TYPE_I16 : type = "T_I16;" ; break;
- case t_base_type::TYPE_I32 : type = "T_I32;" ; break;
- case t_base_type::TYPE_I64 : type = "T_I64;" ; break;
- case t_base_type::TYPE_DOUBLE : type = "T_DOUBLE;" ; break;
- default: return false;
- }
- out << indent() << target << ".ttype = " << type << endl;
- return true;
- }
-
- if (ttype->is_enum()) {
- out <<
- indent() << target << ".ttype = T_ENUM;" << endl <<
- indent() << target << ".name = \"" << ttype->get_name() << "\";" << endl;
- }
-
- if (ttype->is_struct()) {
- out <<
- indent() << target << ".ttype = T_STRUCT;" << endl <<
- indent() << target << ".name = \"" << ttype->get_name() << "\";" << endl;
- return true;
- }
-
- return false;
-}
-
-/**
- * Helper function for generate_service_limited_reflector.
- * Generates a reflection of a single type.
- *
- * @param ttype The type to reflect
- * @param target The name of the lvalue to reflect onto
- */
-bool t_cpp_generator::generate_type_limited_reflection(t_type* ttype, string target) {
- bool is_simple = generate_simple_type_limited_reflection(f_service_, ttype, target + ".simple_type");
- if (is_simple) {
- f_service_ <<
- indent() << target << ".is_container = false;" << endl <<
- indent() << target << ".__isset.simple_type = true;" << endl;
- return true;
- }
-
- ostringstream out;
-
- out <<
- indent() << target << ".is_container = true;" << endl <<
- indent() << target << ".__isset.container_type = true;" << endl <<
- indent() << target << ".container_type.ttype = ";
-
- if (ttype->is_list()) out << "T_LIST;" << endl;
- if (ttype->is_set()) out << "T_SET;" << endl;
- if (ttype->is_map()) out << "T_MAP;" << endl;
-
- bool reflected = false;
-
- if (ttype->is_list()) {
- reflected = generate_simple_type_limited_reflection(
- out, ((t_list*)ttype)->get_elem_type(), target + ".container_type.subtype1");
- }
- if (ttype->is_set()) {
- reflected = generate_simple_type_limited_reflection(
- out, ((t_set*)ttype)->get_elem_type(), target + ".container_type.subtype1");
- }
- if (ttype->is_map()) {
- reflected =
- generate_simple_type_limited_reflection(
- out, ((t_map*)ttype)->get_key_type(), target + ".container_type.subtype1")
- &&
- generate_simple_type_limited_reflection(
- out, ((t_map*)ttype)->get_val_type(), target + ".container_type.subtype2");
- out << indent() << target << ".container_type.__isset.subtype2 = true;" << endl;
- }
-
- if (reflected) {
- f_service_ << out.str();
- return true;
- } else {
- f_service_ <<
- indent() << target << ".is_container = false;" << endl <<
- indent() << target << ".__isset.simple_type = true;" << endl;
- f_service_ << indent() << target << ".simple_type.ttype = T_NOT_REFLECTED;" << endl;
- return false;
- }
-}
-
-/**
- * Generates a service reflector definition.
- * This uses thrift::reflection::limited.
- *
- * @param tservice The service to write a reflector for
- */
-void t_cpp_generator::generate_service_limited_reflector(t_service* tservice) {
- // Open function
- f_service_ <<
- indent() << "void " << tservice->get_name() << "If::getStaticLimitedReflection" <<
- "(apache::thrift::reflection::limited::Service & _return) ";
- scope_up(f_service_);
-
- f_service_ << indent() << "using namespace apache::thrift::reflection::limited;" << endl;
-
- f_service_ << indent() << "_return.name = \"" << tservice->get_name() << "\";" << endl;
- f_service_ << indent() << "_return.fully_reflected = true;" << endl;
-
- bool all_reflectable = true;
- bool one_reflectable;
-
- const vector<t_function*> & funcs = tservice->get_functions();
- vector<t_function*>::const_iterator f_iter;
- for (f_iter = funcs.begin(); f_iter != funcs.end(); ++f_iter) {
-
- f_service_ << indent() << "_return.methods.resize(_return.methods.size() + 1);" << endl;
- f_service_ << indent() << "_return.methods.back().name = \"" << (*f_iter)->get_name() << "\";" << endl;
- one_reflectable = generate_type_limited_reflection(
- (*f_iter)->get_returntype(), "_return.methods.back().return_type");
- all_reflectable = all_reflectable && one_reflectable;
-
- t_struct* arglist = (*f_iter)->get_arglist();
- const vector<t_field*> & args = arglist->get_members();
- vector<t_field*>::const_iterator a_iter;
- for (a_iter = args.begin(); a_iter != args.end(); ++a_iter) {
- f_service_ <<
- indent() << "_return.methods.back().arguments.resize("
- "_return.methods.back().arguments.size() + 1);" << endl <<
- indent() << "_return.methods.back().arguments.back().name = \"" <<
- (*a_iter)->get_name() << "\";" << endl <<
- indent() << "_return.methods.back().arguments.back().key = " <<
- (*a_iter)->get_key() << ";" << endl;
- one_reflectable = generate_type_limited_reflection(
- (*a_iter)->get_type(), "_return.methods.back().arguments.back().type");
- all_reflectable = all_reflectable && one_reflectable;
- }
- }
-
- if (!all_reflectable) {
- f_service_ << indent() << "_return.fully_reflected = false;" << endl;
- }
-
- // Close function
- scope_down(f_service_);
- f_service_ << endl;
-}
-
-/**
* Generates a skeleton file of a server
*
* @param tservice The service to generate a server for.
Modified: incubator/thrift/trunk/configure.ac
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/configure.ac?rev=741824&r1=741823&r2=741824&view=diff
==============================================================================
--- incubator/thrift/trunk/configure.ac (original)
+++ incubator/thrift/trunk/configure.ac Sat Feb 7 02:36:35 2009
@@ -187,7 +187,6 @@
lib/perl/test/Makefile
lib/py/Makefile
lib/rb/Makefile
- if/Makefile
test/Makefile
test/py/Makefile
test/rb/Makefile
Modified: incubator/thrift/trunk/contrib/fb303/cpp/FacebookBase.cpp
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/contrib/fb303/cpp/FacebookBase.cpp?rev=741824&r1=741823&r2=741824&view=diff
==============================================================================
--- incubator/thrift/trunk/contrib/fb303/cpp/FacebookBase.cpp (original)
+++ incubator/thrift/trunk/contrib/fb303/cpp/FacebookBase.cpp Sat Feb 7 02:36:35 2009
@@ -9,12 +9,9 @@
using namespace facebook::fb303;
using facebook::thrift::concurrency::Guard;
-FacebookBase::FacebookBase(std::string name, get_static_limref_ptr reflect_lim) :
+FacebookBase::FacebookBase(std::string name) :
name_(name) {
aliveSince_ = (int64_t) time(NULL);
- if (reflect_lim) {
- reflect_lim(reflection_limited_);
- }
}
inline void FacebookBase::getName(std::string& _return) {
Modified: incubator/thrift/trunk/contrib/fb303/cpp/FacebookBase.h
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/contrib/fb303/cpp/FacebookBase.h?rev=741824&r1=741823&r2=741824&view=diff
==============================================================================
--- incubator/thrift/trunk/contrib/fb303/cpp/FacebookBase.h (original)
+++ incubator/thrift/trunk/contrib/fb303/cpp/FacebookBase.h Sat Feb 7 02:36:35 2009
@@ -26,8 +26,6 @@
struct ReadWriteCounterMap : ReadWriteMutex,
std::map<std::string, ReadWriteInt> {};
-typedef void (*get_static_limref_ptr)(facebook::thrift::reflection::limited::Service &);
-
/**
* Base Facebook service implementation in C++.
*
@@ -35,7 +33,7 @@
*/
class FacebookBase : virtual public FacebookServiceIf {
protected:
- FacebookBase(std::string name, get_static_limref_ptr reflect_lim = NULL);
+ FacebookBase(std::string name);
virtual ~FacebookBase() {}
public:
@@ -51,10 +49,6 @@
int64_t aliveSince();
- void getLimitedReflection(facebook::thrift::reflection::limited::Service& _return) {
- _return = reflection_limited_;
- }
-
virtual void reinitialize() {}
virtual void shutdown() {
@@ -81,7 +75,6 @@
private:
std::string name_;
- facebook::thrift::reflection::limited::Service reflection_limited_;
int64_t aliveSince_;
std::map<std::string, std::string> options_;
Modified: incubator/thrift/trunk/contrib/fb303/if/fb303.thrift
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/contrib/fb303/if/fb303.thrift?rev=741824&r1=741823&r2=741824&view=diff
==============================================================================
--- incubator/thrift/trunk/contrib/fb303/if/fb303.thrift (original)
+++ incubator/thrift/trunk/contrib/fb303/if/fb303.thrift Sat Feb 7 02:36:35 2009
@@ -16,8 +16,6 @@
* @author Mark Slee <mc...@facebook.com>
*/
-include "thrift/if/reflection_limited.thrift"
-
namespace java com.facebook.fb303
namespace cpp facebook.fb303
@@ -96,12 +94,6 @@
i64 aliveSince(),
/**
- * Returns a limited description of this service.
- */
- reflection_limited.Service
- getLimitedReflection(),
-
- /**
* Tell the server to reload its configuration, reopen log files, etc
*/
async void reinitialize(),
Modified: incubator/thrift/trunk/contrib/fb303/java/FacebookBase.java
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/contrib/fb303/java/FacebookBase.java?rev=741824&r1=741823&r2=741824&view=diff
==============================================================================
--- incubator/thrift/trunk/contrib/fb303/java/FacebookBase.java (original)
+++ incubator/thrift/trunk/contrib/fb303/java/FacebookBase.java Sat Feb 7 02:36:35 2009
@@ -83,10 +83,6 @@
return "";
}
- public org.apache.thrift.reflection.limited.Service getLimitedReflection() {
- return new org.apache.thrift.reflection.limited.Service();
- }
-
public void reinitialize() {}
public void shutdown() {}
Modified: incubator/thrift/trunk/contrib/thrift.spec
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/contrib/thrift.spec?rev=741824&r1=741823&r2=741824&view=diff
==============================================================================
--- incubator/thrift/trunk/contrib/thrift.spec (original)
+++ incubator/thrift/trunk/contrib/thrift.spec Sat Feb 7 02:36:35 2009
@@ -44,7 +44,6 @@
%files
%defattr(-,root,root)
%{_bindir}/thrift
-%{_datadir}/thrift/if/reflection_limited.thrift
%package lib-cpp
Modified: incubator/thrift/trunk/lib/cpp/Makefile.am
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/cpp/Makefile.am?rev=741824&r1=741823&r2=741824&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/cpp/Makefile.am (original)
+++ incubator/thrift/trunk/lib/cpp/Makefile.am Sat Feb 7 02:36:35 2009
@@ -22,7 +22,6 @@
# Define the source files for the module
libthrift_la_SOURCES = src/Thrift.cpp \
- src/reflection_limited_types.cpp \
src/concurrency/Mutex.cpp \
src/concurrency/Monitor.cpp \
src/concurrency/PosixThreadFactory.cpp \
@@ -64,7 +63,6 @@
$(top_builddir)/config.h \
src/Thrift.h \
src/TReflectionLocal.h \
- src/reflection_limited_types.h \
src/TProcessor.h \
src/TLogging.h
Modified: incubator/thrift/trunk/lib/py/setup.py
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/py/setup.py?rev=741824&r1=741823&r2=741824&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/py/setup.py (original)
+++ incubator/thrift/trunk/lib/py/setup.py Sat Feb 7 02:36:35 2009
@@ -23,8 +23,6 @@
'thrift.protocol',
'thrift.transport',
'thrift.server',
- 'thrift.reflection',
- 'thrift.reflection.limited',
],
package_dir = {'thrift' : 'src'},
ext_modules = [fastbinarymod],
Modified: incubator/thrift/trunk/test/DebugProtoTest.thrift
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/test/DebugProtoTest.thrift?rev=741824&r1=741823&r2=741824&view=diff
==============================================================================
--- incubator/thrift/trunk/test/DebugProtoTest.thrift (original)
+++ incubator/thrift/trunk/test/DebugProtoTest.thrift Sat Feb 7 02:36:35 2009
@@ -82,16 +82,6 @@
i32 Janky(i32 arg)
}
-service PartiallyReflectable {
- map<i32,map<i32,i32>> returnNotReflectable(1: i32 hello),
- void argNotReflectable(1: list<set<i32>> arg),
- void arg2NotReflectable(1: i32 arg1, 2: list<set<i32>> argNotReflectable),
- void withMap(1: map<i32, string> amap),
-
- OneOfEach refl1(1: list<Bonk> arg1),
- OneOfEach refl2(2: list<string> arg1, 1: Bonk arg2);
-}
-
// The only purpose of this thing is to increase the size of the generated code
// so that ZlibTest has more highly compressible data to play with.
struct BlowUp {
Modified: incubator/thrift/trunk/test/DenseLinkingTest.thrift
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/test/DenseLinkingTest.thrift?rev=741824&r1=741823&r2=741824&view=diff
==============================================================================
--- incubator/thrift/trunk/test/DenseLinkingTest.thrift (original)
+++ incubator/thrift/trunk/test/DenseLinkingTest.thrift Sat Feb 7 02:36:35 2009
@@ -71,13 +71,3 @@
service Srv {
i32 Janky(i32 arg)
}
-
-service PartiallyReflectable {
- map<i32,map<i32,i32>> returnNotReflectable(1: i32 hello),
- void argNotReflectable(1: list<set<i32>> arg),
- void arg2NotReflectable(1: i32 arg1, 2: list<set<i32>> argNotReflectable),
- void withMap(1: map<i32, string> amap),
-
- OneOfEachZZ refl1(1: list<BonkZZ> arg1),
- OneOfEachZZ refl2(2: list<string> arg1, 1: BonkZZ arg2);
-}
Modified: incubator/thrift/trunk/test/Makefile.am
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/test/Makefile.am?rev=741824&r1=741823&r2=741824&view=diff
==============================================================================
--- incubator/thrift/trunk/test/Makefile.am (original)
+++ incubator/thrift/trunk/test/Makefile.am Sat Feb 7 02:36:35 2009
@@ -13,7 +13,6 @@
gen-cpp/DebugProtoTest_types.cpp \
gen-cpp/OptionalRequiredTest_types.cpp \
gen-cpp/DebugProtoTest_types.cpp \
- gen-cpp/PartiallyReflectable.cpp \
gen-cpp/Service.cpp \
gen-cpp/StressTest_types.cpp \
gen-cpp/SecondService.cpp \
@@ -37,7 +36,6 @@
DebugProtoTest \
JSONProtoTest \
OptionalRequiredTest \
- ReflectionTest \
UnitTests
TESTS = \
@@ -96,29 +94,21 @@
#
-# ReflectionTest
-#
-ReflectionTest_SOURCES = \
- ReflectionTest.cpp
-
-ReflectionTest_LDADD = libtestgencpp.la
-
-#
# Common thrift code generation rules
#
THRIFT = $(top_builddir)/compiler/cpp/thrift
-gen-cpp/DebugProtoTest_types.cpp gen-cpp/PartiallyReflectable.cpp: DebugProtoTest.thrift
- $(THRIFT) --gen cpp:dense,reflection_limited $<
+gen-cpp/DebugProtoTest_types.cpp: DebugProtoTest.thrift
+ $(THRIFT) --gen cpp:dense $<
gen-cpp/OptionalRequiredTest_types.cpp: OptionalRequiredTest.thrift
$(THRIFT) --gen cpp:dense $<
gen-cpp/Service.cpp gen-cpp/StressTest_types.cpp: StressTest.thrift
- $(THRIFT) --gen cpp:dense,reflection_limited $<
+ $(THRIFT) --gen cpp:dense $<
gen-cpp/SecondService.cpp gen-cpp/ThriftTest_constants.cpp gen-cpp/ThriftTest.cpp gen-cpp/ThriftTest_types.cpp: ThriftTest.thrift
- $(THRIFT) --gen cpp:dense,reflection_limited $<
+ $(THRIFT) --gen cpp:dense $<
INCLUDES = \
-I$(top_srcdir)/lib/cpp/src