From 981bab32b7be796f5d6d6b39ec9d3e7e7aebe909 Mon Sep 17 00:00:00 2001 From: Travis Mehlinger Date: Thu, 22 Jan 2015 17:16:28 -0600 Subject: [PATCH 01/26] Added service multiplexing support. This introduces a `TMultiplexingProcessor` class which extends `TProcessor`. This adds the ability for developers to develop services that implement multiple Thrift interfaces. It is inspired by the `TMultiplexedProcessor` from the Thrift Java library. --- examples/multiplexer/dingdong.thrift | 7 ++ examples/multiplexer/multiplexed_client.py | 23 ++++++ examples/multiplexer/multiplexed_server.py | 41 ++++++++++ examples/multiplexer/pingpong.thrift | 7 ++ tests/multiplexed.thrift | 7 ++ tests/test_multiplexed.py | 73 +++++++++++++++++ thriftpy/thrift.py | 93 +++++++++++++++++----- 7 files changed, 229 insertions(+), 22 deletions(-) create mode 100644 examples/multiplexer/dingdong.thrift create mode 100644 examples/multiplexer/multiplexed_client.py create mode 100644 examples/multiplexer/multiplexed_server.py create mode 100644 examples/multiplexer/pingpong.thrift create mode 100644 tests/multiplexed.thrift create mode 100644 tests/test_multiplexed.py diff --git a/examples/multiplexer/dingdong.thrift b/examples/multiplexer/dingdong.thrift new file mode 100644 index 0000000..6b032d1 --- /dev/null +++ b/examples/multiplexer/dingdong.thrift @@ -0,0 +1,7 @@ +# ding service demo +service DingService { + /* + * Sexy c style comment + */ + string ding(), +} diff --git a/examples/multiplexer/multiplexed_client.py b/examples/multiplexer/multiplexed_client.py new file mode 100644 index 0000000..e4612af --- /dev/null +++ b/examples/multiplexer/multiplexed_client.py @@ -0,0 +1,23 @@ +# -*- coding: utf-8 -*- + +import thriftpy +from thriftpy.rpc import client_context + +dd_thrift = thriftpy.load("dingdong.thrift", module_name="dd_thrift") +pp_thrift = thriftpy.load("pingpong.thrift", module_name="pp_thrift") + + +def main(): + with client_context(dd_thrift.DingService, '127.0.0.1', 9090) as c: + # ring that doorbell + dong = c.ding() + print(dong) + + with client_context(pp_thrift.PingService, '127.0.0.1', 9090) as c: + # play table tennis like a champ + pong = c.ping() + print(pong) + + +if __name__ == '__main__': + main() diff --git a/examples/multiplexer/multiplexed_server.py b/examples/multiplexer/multiplexed_server.py new file mode 100644 index 0000000..90f051b --- /dev/null +++ b/examples/multiplexer/multiplexed_server.py @@ -0,0 +1,41 @@ +# -*- coding: utf-8 -*- + +import thriftpy +from thriftpy.protocol import TBinaryProtocolFactory +from thriftpy.server import TThreadedServer +from thriftpy.thrift import TProcessor, TMultiplexingProcessor +from thriftpy.transport import TBufferedTransportFactory, TServerSocket + + +dd_thrift = thriftpy.load("dingdong.thrift", module_name="dd_thrift") +pp_thrift = thriftpy.load("pingpong.thrift", module_name="pp_thrift") + + +class DingDispatcher(object): + def ding(self): + print("ding dong!") + return 'dong' + + +class PingDispatcher(object): + def ping(self): + print("ping pong!") + return 'pong' + + +def main(): + dd_proc = TProcessor(dd_thrift.DingService, DingDispatcher()) + pp_proc = TProcessor(pp_thrift.PingService, PingDispatcher()) + + mux_proc = TMultiplexingProcessor() + mux_proc.register_processor(dd_proc) + mux_proc.register_processor(pp_proc) + + server = TThreadedServer(mux_proc, TServerSocket(), + iprot_factory=TBinaryProtocolFactory(), + itrans_factory=TBufferedTransportFactory()) + server.serve() + + +if __name__ == '__main__': + main() diff --git a/examples/multiplexer/pingpong.thrift b/examples/multiplexer/pingpong.thrift new file mode 100644 index 0000000..0bb9c85 --- /dev/null +++ b/examples/multiplexer/pingpong.thrift @@ -0,0 +1,7 @@ +# ping service demo +service PingService { + /* + * Sexy c style comment + */ + string ping(), +} diff --git a/tests/multiplexed.thrift b/tests/multiplexed.thrift new file mode 100644 index 0000000..5dfd4e7 --- /dev/null +++ b/tests/multiplexed.thrift @@ -0,0 +1,7 @@ +service ThingOneService { + bool doThingOne(); +} + +service ThingTwoService { + bool doThingTwo(); +} diff --git a/tests/test_multiplexed.py b/tests/test_multiplexed.py new file mode 100644 index 0000000..e8f1e16 --- /dev/null +++ b/tests/test_multiplexed.py @@ -0,0 +1,73 @@ +# -*- coding: utf-8 -*- + +from __future__ import absolute_import + +import os +import multiprocessing +import time + +import pytest + +import thriftpy +from thriftpy.protocol import TBinaryProtocolFactory +from thriftpy.rpc import client_context +from thriftpy.server import TThreadedServer +from thriftpy.thrift import TProcessor, TMultiplexingProcessor +from thriftpy.transport import TBufferedTransportFactory, TServerSocket + + +mux = thriftpy.load(os.path.join(os.path.dirname(__file__), + "multiplexed.thrift")) +sock_path = "./thriftpy_test.sock" + + +class DispatcherOne(object): + def doThingOne(self): + return True + + +class DispatcherTwo(object): + def doThingTwo(self): + return True + + +@pytest.fixture(scope="module") +def server(request): + p1 = TProcessor(mux.ThingOneService, DispatcherOne()) + p2 = TProcessor(mux.ThingTwoService, DispatcherTwo()) + + mux_proc = TMultiplexingProcessor() + mux_proc.register_processor(p1) + mux_proc.register_processor(p2) + + _server = TThreadedServer(mux_proc, TServerSocket(unix_socket=sock_path), + iprot_factory=TBinaryProtocolFactory(), + itrans_factory=TBufferedTransportFactory()) + ps = multiprocessing.Process(target=_server.serve) + ps.start() + time.sleep(0.1) + + def fin(): + if ps.is_alive(): + ps.terminate() + try: + os.remove(sock_path) + except IOError: + pass + request.addfinalizer(fin) + + +def client_one(timeout=3000): + return client_context(mux.ThingOneService, unix_socket=sock_path, + timeout=timeout) + +def client_two(timeout=3000): + return client_context(mux.ThingTwoService, unix_socket=sock_path, + timeout=timeout) + + +def test_multiplexed_server(server): + with client_one() as c: + assert c.doThingOne() is True + with client_two() as c: + assert c.doThingTwo() is True diff --git a/thriftpy/thrift.py b/thriftpy/thrift.py index b216b09..4daeef4 100644 --- a/thriftpy/thrift.py +++ b/thriftpy/thrift.py @@ -189,19 +189,19 @@ def process_in(self, iprot): iprot.skip(TType.STRUCT) iprot.read_message_end() return api, seqid, TApplicationException(TApplicationException.UNKNOWN_METHOD), None # noqa - else: - args = getattr(self._service, api + "_args")() - args.read(iprot) - iprot.read_message_end() - result = getattr(self._service, api + "_result")() - # convert kwargs to args - api_args = [args.thrift_spec[k][1] - for k in sorted(args.thrift_spec)] - call = lambda: getattr(self._handler, api)( - *(args.__dict__[k] for k in api_args) - ) - return api, seqid, result, call + args = getattr(self._service, api + "_args")() + args.read(iprot) + iprot.read_message_end() + result = getattr(self._service, api + "_result")() + + # convert kwargs to args + api_args = [args.thrift_spec[k][1] + for k in sorted(args.thrift_spec)] + call = lambda: getattr(self._handler, api)( + *(args.__dict__[k] for k in api_args) + ) + return api, seqid, result, call def send_exception(self, oprot, api, exc, seqid): oprot.write_message_begin(api, TMessageType.EXCEPTION, seqid) @@ -231,16 +231,65 @@ def process(self, iprot, oprot): api, seqid, result, call = self.process_in(iprot) if isinstance(result, TApplicationException): - self.send_exception(oprot, api, result, seqid) - else: - try: - result.success = call() - except Exception as e: - # raise if api don't have throws - self.handle_exception(e, result) - - if not result.oneway: - self.send_result(oprot, api, result, seqid) + return self.send_exception(oprot, api, result, seqid) + + try: + result.success = call() + except Exception as e: + # raise if api don't have throws + self.handle_exception(e, result) + + if not result.oneway: + self.send_result(oprot, api, result, seqid) + + +class TMultiplexingProcessor(TProcessor): + processors = {} + service_map = {} + + def __init__(self): + pass + + def register_processor(self, processor): + service = processor._service + name = service.__name__ + if name in self.processors: + raise TApplicationException( + type=TApplicationException.INTERNAL_ERROR, + message='processor for `{0}` already registered'.format(name)) + + for srv in service.thrift_services: + if srv in self.service_map: + raise TApplicationException( + type=TApplicationException.INTERNAL_ERROR, + message='cannot multiplex processor for `{0}`; ' + '`{1}` is already a registered method for `{2}`' + .format(name, srv, self.service_map[srv])) + self.service_map[srv] = name + + self.processors[name] = processor + + def process_in(self, iprot): + api, type, seqid = iprot.read_message_begin() + if api not in self.service_map: + iprot.skip(TType.STRUCT) + iprot.read_message_end() + e = TApplicationException(TApplicationException.UNKNOWN_METHOD) + return api, seqid, e, None # noqa + + proc = self.processors[self.service_map[api]] + args = getattr(proc._service, api + "_args")() + args.read(iprot) + iprot.read_message_end() + result = getattr(proc._service, api + "_result")() + + # convert kwargs to args + api_args = [args.thrift_spec[k][1] + for k in sorted(args.thrift_spec)] + call = lambda: getattr(proc._handler, api)( + *(args.__dict__[k] for k in api_args) + ) + return api, seqid, result, call class TException(TPayload, Exception): From 04d4ce95bd4c3a26bdcbd57cc83a4d53dc8fc0cf Mon Sep 17 00:00:00 2001 From: Travis Mehlinger Date: Thu, 22 Jan 2015 17:24:07 -0600 Subject: [PATCH 02/26] fix pep8 issues --- tests/test_multiplexed.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_multiplexed.py b/tests/test_multiplexed.py index e8f1e16..ad53ea2 100644 --- a/tests/test_multiplexed.py +++ b/tests/test_multiplexed.py @@ -61,6 +61,7 @@ def client_one(timeout=3000): return client_context(mux.ThingOneService, unix_socket=sock_path, timeout=timeout) + def client_two(timeout=3000): return client_context(mux.ThingTwoService, unix_socket=sock_path, timeout=timeout) From a6141fadfaa69283a997be3fdc9cf5dfce34dea5 Mon Sep 17 00:00:00 2001 From: hit9 Date: Fri, 23 Jan 2015 14:53:50 +0800 Subject: [PATCH 03/26] New Parser --- thriftpy/parser/__init__.py | 229 +--------------- thriftpy/parser/exc.py | 6 +- thriftpy/parser/lexer.py | 5 - thriftpy/parser/model.py | 205 -------------- thriftpy/parser/parser.py | 517 ++++++++++++++++++++++++++++-------- 5 files changed, 417 insertions(+), 545 deletions(-) delete mode 100644 thriftpy/parser/model.py diff --git a/thriftpy/parser/__init__.py b/thriftpy/parser/__init__.py index 2115cef..4ee1c68 100644 --- a/thriftpy/parser/__init__.py +++ b/thriftpy/parser/__init__.py @@ -8,237 +8,21 @@ """ from __future__ import absolute_import - -import itertools -import os -import os.path import sys -import types - -from .model import IdentifierValue from .parser import parse -from ..thrift import gen_init, TException, TPayload, TType - -_thriftloader = {} - -def load(thrift_file, module_name=None, include_dirs=None): +def load(path, module_name=None, include_dir=None): """Load thrift_file as a module - The module loaded and objects inside may only be pickled if module_name was provided. """ - if module_name and not module_name.endswith("_thrift"): - raise ValueError( - "ThriftPy can only generate module with '_thrift' suffix") - - include_dirs = list(include_dirs or ["."]) - - global _thriftloader - # if module_name provided, it should be unique. we'll ignore the filename - # here for the global thriftloader - _thriftloader_key = module_name or thrift_file - if _thriftloader_key in _thriftloader: - return _thriftloader[_thriftloader_key] - real_module = bool(module_name) + thrift = parse(path, module_name, include_dir=include_dir) - # generate a fake module_name when it's not provided - if not module_name: - basename = os.path.basename(thrift_file) - module_name, _ = os.path.splitext(basename) - - exception = None - for d in include_dirs: - try: - path = os.path.join(d, thrift_file) - with open(path, "r") as fp: - schema = fp.read() - except IOError as e: - exception = e - else: - exception = None - break - - if exception is not None: - raise exception - - result = parse(schema) - - # load thrift schema as module - thrift_schema = types.ModuleType(module_name) - _type = lambda n, o: type(n, (o, ), {"__module__": module_name}) - - def make_thrift_include_name(path): - name = os.path.basename(path) - assert name.endswith(".thrift") - return name[:-len(".thrift")] - - thrift_schema._includes = {} - for path in result["includes"]: - name = make_thrift_include_name(path) - include = thrift_schema._includes[name] = load( - path, include_dirs=include_dirs) - setattr(thrift_schema, name, include) - - thrift_schema._struct_names = list(itertools.chain( - result["structs"].keys(), - result["unions"].keys(), - result["exceptions"].keys())) - - thrift_schema._enums = result["enums"] - thrift_schema._typedefs = result["typedefs"] - - def _ttype(t, module=None): - module = module or thrift_schema - if isinstance(t, str): - if "." in t: - include, field = t.split(".", 1) - return _ttype(field, module=module._includes[include]) - elif t in module._struct_names: - return TType.STRUCT, getattr(module, t) - elif t in module._enums: - return TType.I32, getattr(module, t) - elif t in module._typedefs: - return _ttype(module._typedefs[t], module) - else: - return getattr(TType, t.upper()) - elif t[0] == "list": - return TType.LIST, _ttype(t[1], module) - elif t[0] == "set": - return TType.SET, _ttype(t[1], module) - elif t[0] == "map": - return TType.MAP, (_ttype(t[1], module), _ttype(t[2], module)) - else: - raise Exception("ttype parse error: {0}".format(t)) - - def _lookup_value(const): - if isinstance(const, IdentifierValue): - value = thrift_schema - for ref in const.v.split("."): - value = getattr(value, ref) - return value - - return const - - def _ttype_spec(ttype, name, requirement=None): - ttype = _ttype(ttype) - req = False - if requirement == 'required': - req = True - if isinstance(ttype, int): - return ttype, name, req - else: - return ttype[0], name, ttype[1], req - - # load enums - for name, enum in result["enums"].items(): - attrs = {"__module__": module_name} - v2n, n2v = {}, {} - for key, value in enum.items(): - attrs[key] = value - v2n[value] = key - n2v[key] = value - attrs['_VALUES_TO_NAMES'] = v2n - attrs['_NAMES_TO_VALUES'] = n2v - enum_cls = type(name, (object, ), attrs) - setattr(thrift_schema, name, enum_cls) - - # load consts - for name, value in result["consts"].items(): - setattr(thrift_schema, name, _lookup_value(value)) - - # load structs/unions - for name in itertools.chain(result["structs"].keys(), - result["unions"].keys()): - attrs = {"__module__": module_name} - struct_cls = type(name, (TPayload, ), attrs) - setattr(thrift_schema, name, struct_cls) - - for name in result["exceptions"].keys(): - attrs = {"__module__": module_name} - exc_cls = type(name, (TException, ), attrs) - setattr(thrift_schema, name, exc_cls) - - for name, struct in itertools.chain(result["structs"].items(), - result["unions"].items(), - result["exceptions"].items()): - thrift_spec, default_spec = {}, [] - for m in struct: - thrift_spec[m["id"]] = _ttype_spec(m["type"], m["name"], - m["requirement"]) - default = _lookup_value(m["value"]) if m["value"] else None - default_spec.append((m["name"], default)) - struct_cls = getattr(thrift_schema, name) - gen_init(struct_cls, thrift_spec, default_spec) - - # load services - for name, service in result["services"].items(): - base = _BaseService - if service["extends"]: - module, extends_name = thrift_schema, service["extends"] - if "." in service["extends"]: - module_name, extends_name = service["extends"].split(".", 1) - module = module._includes[module_name] - - base = getattr(module, extends_name) - - assert hasattr(base, "thrift_services"), \ - "%s is not a valid service base" % (service["extends"]) - - service_cls = _type(name, base) - thrift_services = [] - for api_name, api in service["apis"].items(): - thrift_services.append(api_name) - - # generate default spec from thrift spec - _default_spec = lambda s: [(s[k][1], None) for k in sorted(s)] - - # api args payload - args_name = "%s_args" % api_name - args_attrs = {"__module__": module_name} - - args_thrift_spec = {} - for field in api["fields"]: - args_thrift_spec[field["id"]] = _ttype_spec(field["type"], - field["name"]) - args_cls = type(args_name, (TPayload, ), args_attrs) - gen_init(args_cls, args_thrift_spec, - _default_spec(args_thrift_spec)) - setattr(service_cls, args_name, args_cls) - - # api result payload - result_name = "%s_result" % api_name - result_attrs = {"__module__": module_name} - - # if oneway - result_attrs["oneway"] = api["oneway"] - - if api["type"] == "void": - result_thrift_spec = {} - else: - result_thrift_spec = {0: _ttype_spec(api["type"], "success")} - - if api.get("throws"): - for t in api["throws"]: - result_thrift_spec[t["id"]] = _ttype_spec(t["type"], - t["name"]) - - result_cls = type(result_name, (TPayload, ), result_attrs) - gen_init(result_cls, result_thrift_spec, - _default_spec(result_thrift_spec)) - setattr(service_cls, result_name, result_cls) - - service_cls.thrift_services = base.thrift_services + thrift_services - setattr(thrift_schema, name, service_cls) - thrift_schema.__file__ = thrift_file - - _thriftloader[_thriftloader_key] = thrift_schema - # insert into sys.modules if module_name was provided manually if real_module: - sys.modules[module_name] = _thriftloader[_thriftloader_key] - return _thriftloader[_thriftloader_key] + sys.modules[module_name] = thrift + return thrift def _import_module(import_name): @@ -252,7 +36,6 @@ def _import_module(import_name): def load_module(fullname): """Load thrift_file by fullname, fullname should have '_thrift' as suffix. - The loader will replace the '_thrift' with '.thrift' and use it as filename to locate the real thrift file. """ @@ -275,7 +58,3 @@ def load_module(fullname): module = load(thrift_file, module_name=fullname) sys.modules[fullname] = module return sys.modules[fullname] - - -class _BaseService(object): - thrift_services = [] diff --git a/thriftpy/parser/exc.py b/thriftpy/parser/exc.py index b34b3c6..98d697c 100644 --- a/thriftpy/parser/exc.py +++ b/thriftpy/parser/exc.py @@ -3,13 +3,13 @@ from __future__ import absolute_import -class ThriftSyntaxError(SyntaxError): +class ThriftParserError(Exception): pass -class ThriftLexerError(ThriftSyntaxError): +class ThriftLexerError(ThriftParserError): pass -class ThriftGrammerError(ThriftSyntaxError): +class ThriftGrammerError(ThriftParserError): pass diff --git a/thriftpy/parser/lexer.py b/thriftpy/parser/lexer.py index eec70be..8ed36c5 100644 --- a/thriftpy/parser/lexer.py +++ b/thriftpy/parser/lexer.py @@ -2,8 +2,6 @@ from __future__ import absolute_import -from ply import lex - from .exc import ThriftLexerError @@ -148,6 +146,3 @@ def t_IDENTIFIER(t): t.type = t.value.upper() return t return t - - -lexer = lex.lex() diff --git a/thriftpy/parser/model.py b/thriftpy/parser/model.py deleted file mode 100644 index e522c31..0000000 --- a/thriftpy/parser/model.py +++ /dev/null @@ -1,205 +0,0 @@ -# -*- coding: utf-8 -*- - -import sys - -if sys.version_info[0] < 3: - bytes_ = bytes -else: - bytes_ = lambda x: bytes(x, 'utf8') - - -class Thrift(dict): - - def __init__(self, - includes=None, - namespaces=None, - consts=None, - enums=None, - typedefs=None, - structs=None, - unions=None, - exceptions=None, - services=None): - - super(Thrift, self).__init__() - - if includes is None: - includes = [] - if namespaces is None: - namespaces = {} - if consts is None: - consts = {} - if enums is None: - enums = {} - if typedefs is None: - typedefs = {} - if structs is None: - structs = {} - if unions is None: - unions = {} - if exceptions is None: - exceptions = {} - if services is None: - services = {} - - self.includes = self['includes'] = includes - self.namespaces = self['namespaces'] = namespaces - self.consts = self['consts'] = consts - self.enums = self['enums'] = enums - self.typedefs = self['typedefs'] = typedefs - self.structs = self['structs'] = structs - self.unions = self['unions'] = unions - self.exceptions = self['exceptions'] = exceptions - self.services = self['services'] = services - - -class BaseType(str): - pass - - -class BoolType(BaseType): - - cast = bool - - def __new__(self): - return str.__new__(self, 'bool') - - -class ByteType(BaseType): - - cast = int - - def __new__(self): - return str.__new__(self, 'byte') - - -class I16Type(BaseType): - - cast = int - - def __new__(self): - return str.__new__(self, 'i16') - - -class I32Type(BaseType): - - cast = int - - def __new__(self): - return str.__new__(self, 'i32') - - -class I64Type(BaseType): - - cast = int - - def __new__(self): - return str.__new__(self, 'i64') - - -class DoubleType(BaseType): - - cast = float - - def __new__(self): - return str.__new__(self, 'double') - - -class StringType(BaseType): - - cast = str - - def __new__(self): - return str.__new__(self, 'string') - - -class BinaryType(BaseType): - - cast = bytes_ - - def __new__(self): - return str.__new__(self, 'binary') - - -class ContainerType(list): - pass - - -class ListType(ContainerType): # ['list', val_type] - - def cast(self, data): - return list(map(self[1].cast, data)) - - -class MapType(ContainerType): # ['map', k_type, v_type] - - def cast(self, data): - dct = {} - keys = data.keys() - - for key in keys: - dct[self[1].cast(key)] = self[2].cast(data[key]) - return dct - - -class SetType(ContainerType): # ['set', v_type] - - def cast(self, data): - return frozenset(map(self[1].cast, data)) - - -BASE_TYPE_MAPS = { - 'bool': BoolType, - 'byte': ByteType, - 'i16': I16Type, - 'i32': I32Type, - 'i64': I64Type, - 'double': DoubleType, - 'string': StringType, - 'binary': BinaryType -} - - -class IdentifierValue(dict): - def __init__(self, v): - super(IdentifierValue, self).__init__() - self.v = self['v'] = v - - -class Field(dict): - - def __init__(self, id, type, name, value=None, requirement=None): - super(Field, self).__init__() - self.id = self['id'] = id - self.type = self['type'] = type - self.name = self['name'] = name - self.value = self['value'] = value - self.requirement = self['requirement'] = requirement - - -class Function(dict): - - def __init__(self, type, name, fields=None, throws=None, oneway=False): - super(Function, self).__init__() - - if fields is None: - fields = [] - if throws is None: - throws = [] - self.type = self['type'] = type - self.name = self['name'] = name - self.fields = self['fields'] = fields - self.throws = self['throws'] = throws - self.oneway = self['oneway'] = oneway - - -class Service(dict): - - def __init__(self, extends=None, apis=None): - super(Service, self).__init__() - - if apis is None: - apis = {} - - self.extends = self['extends'] = extends - self.apis = self['apis'] = apis diff --git a/thriftpy/parser/parser.py b/thriftpy/parser/parser.py index 88d1a54..02fc137 100644 --- a/thriftpy/parser/parser.py +++ b/thriftpy/parser/parser.py @@ -7,21 +7,12 @@ from __future__ import absolute_import -from ply import yacc - -from .lexer import tokens, lexer # noqa -from .model import ( - BASE_TYPE_MAPS, - Field, - Function, - IdentifierValue, - ListType, - MapType, - Service, - SetType, - Thrift -) -from .exc import ThriftGrammerError +import os +import types +from ply import lex, yacc +from .lexer import * # noqa +from .exc import ThriftParserError, ThriftGrammerError +from ..thrift import gen_init, TType, TPayload, TException def p_error(p): @@ -34,10 +25,15 @@ def p_start(p): def p_header(p): - '''header : header_unit header + '''header : header_unit_ header |''' +def p_header_unit_(p): + '''header_unit_ : header_unit ';' + | header_unit''' + + def p_header_unit(p): '''header_unit : include | namespace''' @@ -45,12 +41,17 @@ def p_header_unit(p): def p_include(p): '''include : INCLUDE LITERAL''' - thrift.includes.append(p[2]) + thrift = thrift_stack[-1] + path = os.path.join(include_dir_, p[2]) + child = parse(path) + setattr(thrift, child.__name__, child) def p_namespace(p): '''namespace : NAMESPACE namespace_scope IDENTIFIER''' - thrift.namespaces[p[3]] = p[2] + # namespace is useless in thriftpy + # if p[2] == 'py' or p[2] == '*': + # setattr(thrift_stack[-1], '__name__', p[3]) def p_namespace_scope(p): @@ -66,24 +67,30 @@ def p_sep(p): def p_definition(p): - '''definition : definition definition_unit + '''definition : definition definition_unit_ |''' +def p_definition_unit_(p): + '''definition_unit_ : definition_unit ';' + | definition_unit''' + + def p_definition_unit(p): '''definition_unit : const - | typedef - | enum - | struct - | union - | exception - | service + | ttype ''' def p_const(p): - '''const : CONST field_type IDENTIFIER '=' const_value ''' - thrift.consts[p[3]] = p[2].cast(p[5]) + '''const : CONST field_type IDENTIFIER '=' const_value''' + + try: + val = _cast(p[2])(p[5]) + except AssertionError: + raise ThriftParserError('Type error for constant %s at line %d' % + (p[3], p.lineno(3))) + setattr(thrift_stack[-1], p[3], val) def p_const_value(p): @@ -91,27 +98,21 @@ def p_const_value(p): | DUBCONSTANT | LITERAL | BOOLCONSTANT - | identifier_value | const_list - | const_map''' - + | const_map + | const_ref''' p[0] = p[1] -def p_identifier_value(p): - '''identifier_value : IDENTIFIER''' - p[0] = IdentifierValue(p[1]) - - def p_const_list(p): - '''const_list : '[' const_value_seq ']' ''' + '''const_list : '[' const_list_seq ']' ''' p[0] = p[2] -def p_const_value_seq(p): - '''const_value_seq : const_value sep const_value_seq - | const_value const_value_seq - |''' +def p_const_list_seq(p): + '''const_list_seq : const_value sep const_list_seq + | const_value const_list_seq + |''' _parse_seq(p) @@ -120,55 +121,65 @@ def p_const_map(p): p[0] = dict(p[2]) -def p_const_map_items(p): +def p_const_map_seq(p): '''const_map_seq : const_map_item sep const_map_seq | const_map_item const_map_seq - | - ''' + |''' _parse_seq(p) def p_const_map_item(p): - '''const_map_item : const_value ':' const_value''' + '''const_map_item : const_value ':' const_value ''' p[0] = [p[1], p[3]] -def p_typedef(p): - '''typedef : TYPEDEF definition_type IDENTIFIER''' - thrift.typedefs[p[3]] = p[2] +def p_const_ref(p): + '''const_ref : IDENTIFIER''' + keys = p[1].split('.') + thrift = thrift_stack[-1] + if len(keys) == 1 and hasattr(thrift, keys[0]): + p[0] = getattr(thrift, keys[0]) + return -def p_enum(p): - '''enum : ENUM IDENTIFIER '{' enum_seq '}' ''' + if len(keys) == 2 and hasattr(thrift, keys[0]): + enum = getattr(thrift, keys[0]) + if hasattr(enum, keys[1]): + p[0] = getattr(enum, keys[1]) + return + raise ThriftParserError('No enum value or constant found named %r' % p[1]) - if not p[4]: - thrift.enums[p[2]] = {} - else: - init_val = p[4][0][1] - vals = [-1 if init_val is None else init_val] - for item in p[4]: - if item[1] is None: - val = vals[-1] + 1 - item[1] = val - vals.append(val) - vals.append(item[1]) +def p_ttype(p): + '''ttype : typedef + | enum + | struct + | union + | exception + | service''' + - dct = dict(p[4]) - thrift.enums[p[2]] = dct +def p_typedef(p): + '''typedef : TYPEDEF definition_type IDENTIFIER''' + setattr(thrift_stack[-1], p[3], p[2]) + + +def p_enum(p): # noqa + '''enum : ENUM IDENTIFIER '{' enum_seq '}' ''' + setattr(thrift_stack[-1], p[2], _make_enum(p[2], p[4])) def p_enum_seq(p): '''enum_seq : enum_item sep enum_seq | enum_item enum_seq - | - ''' + |''' _parse_seq(p) def p_enum_item(p): '''enum_item : IDENTIFIER '=' INTCONSTANT - | IDENTIFIER''' + | IDENTIFIER + |''' if len(p) == 4: p[0] = [p[1], p[3]] elif len(p) == 2: @@ -177,55 +188,71 @@ def p_enum_item(p): def p_struct(p): '''struct : STRUCT IDENTIFIER '{' field_seq '}' ''' - thrift.structs[p[2]] = p[4] + setattr(thrift_stack[-1], p[2], _make_struct(p[2], p[4])) def p_union(p): '''union : UNION IDENTIFIER '{' field_seq '}' ''' - thrift.unions[p[2]] = p[4] + setattr(thrift_stack[-1], p[2], _make_struct(p[2], p[4])) def p_exception(p): '''exception : EXCEPTION IDENTIFIER '{' field_seq '}' ''' - thrift.exceptions[p[2]] = p[4] + setattr(thrift_stack[-1], p[2], _make_struct(p[2], p[4], + base_cls=TException)) def p_service(p): '''service : SERVICE IDENTIFIER '{' function_seq '}' | SERVICE IDENTIFIER EXTENDS IDENTIFIER '{' function_seq '}' ''' - apis = {} - extends = p[4] if len(p) == 8 else None - functions = p[len(p) - 2] + thrift = thrift_stack[-1] - for function in functions: - apis[function.name] = function + if len(p) == 8: + father = thrift + + for name in p[4].split('.'): + child = getattr(father, name, None) + if child is None: + raise ThriftParserError('Can\'t find service %r for ' + 'service %r to extend' % + (p[4], p[2])) + if not hasattr(child, 'thrift_services'): + raise ThriftParserError('Can\'t extends %r, not a service' + % p[4]) + + extends = child + else: + extends = None - thrift.services[p[2]] = Service(extends=extends, apis=apis) + setattr(thrift, p[2], _make_service(p[2], p[len(p) - 2], extends)) def p_function(p): '''function : ONEWAY function_type IDENTIFIER '(' field_seq ')' throws | ONEWAY function_type IDENTIFIER '(' field_seq ')' | function_type IDENTIFIER '(' field_seq ')' throws - | function_type IDENTIFIER '(' field_seq ')' - ''' + | function_type IDENTIFIER '(' field_seq ')' ''' - if len(p) == 8: - p[0] = Function(p[2], p[3], fields=p[5], throws=p[7], oneway=True) - elif len(p) == 7 and p[1] == 'oneway': - p[0] = Function(p[2], p[3], fields=p[5], throws=None, oneway=True) - elif len(p) == 7 and p[1] != 'oneway': - p[0] = Function(p[1], p[2], fields=p[4], throws=p[6], oneway=False) - elif len(p) == 6: - p[0] = Function(p[1], p[2], fields=p[4], throws=None, oneway=False) + if p[1] == 'oneway': + oneway = True + base = 1 + else: + oneway = False + base = 0 + + if p[len(p) - 1] == ')': + throws = [] + else: + throws = p[len(p) - 1] + + p[0] = [oneway, p[base + 1], p[base + 2], p[base + 4], throws] def p_function_seq(p): '''function_seq : function sep function_seq | function function_seq - | - ''' + |''' _parse_seq(p) @@ -237,22 +264,34 @@ def p_throws(p): def p_function_type(p): '''function_type : field_type | VOID''' - p[0] = p[1] + if p[1] == 'void': + p[0] = TType.VOID + else: + p[0] = p[1] def p_field_seq(p): '''field_seq : field sep field_seq | field field_seq - | - ''' + |''' _parse_seq(p) def p_field(p): '''field : field_id field_req field_type IDENTIFIER | field_id field_req field_type IDENTIFIER '=' const_value''' - v = p[6] if len(p) == 7 else None - p[0] = Field(p[1], p[3], p[4], value=v, requirement=p[2]) + + if len(p) == 7: + try: + val = _cast(p[3])(p[6]) + except AssertionError: + raise ThriftParserError( + 'Type error for field %s ' + 'at line %d' % (p[4], p.lineno(4))) + else: + val = None + + p[0] = [p[1], p[2], p[3], p[4], val] def p_field_id(p): @@ -265,17 +304,31 @@ def p_field_req(p): | OPTIONAL |''' if len(p) == 2: - p[0] = p[1] + p[0] = p[1] == 'required' + elif len(p) == 1: + p[0] = True # default: required=True def p_field_type(p): - '''field_type : IDENTIFIER - | base_type - | container_type''' + '''field_type : ref_type + | definition_type''' p[0] = p[1] -def p_base_type(p): +def p_ref_type(p): + '''ref_type : IDENTIFIER''' + ttype = getattr(thrift_stack[-1], p[1], None) + + if ttype is None: + raise ThriftParserError('No type found: %r, at line %d' % + (p[1], p.lineno(1))) + if hasattr(ttype, '_ttype'): + p[0] = getattr(ttype, '_ttype'), ttype + else: + p[0] = ttype + + +def p_base_type(p): # noqa '''base_type : BOOL | BYTE | I16 @@ -284,8 +337,22 @@ def p_base_type(p): | DOUBLE | STRING | BINARY''' - - p[0] = BASE_TYPE_MAPS[p[1]]() + if p[1] == 'bool': + p[0] = TType.BOOL + if p[1] == 'byte': + p[0] = TType.BYTE + if p[1] == 'i16': + p[0] = TType.I16 + if p[1] == 'i32': + p[0] = TType.I32 + if p[1] == 'i64': + p[0] = TType.I64 + if p[1] == 'double': + p[0] = TType.DOUBLE + if p[1] == 'string': + p[0] = TType.STRING + if p[1] == 'binary': + p[0] = TType.BINARY def p_container_type(p): @@ -297,17 +364,17 @@ def p_container_type(p): def p_map_type(p): '''map_type : MAP '<' field_type ',' field_type '>' ''' - p[0] = MapType(['map', p[3], p[5]]) + p[0] = [TType.MAP, [p[3], p[5]]] def p_list_type(p): '''list_type : LIST '<' field_type '>' ''' - p[0] = ListType(['list', p[3]]) + p[0] = [TType.LIST, p[3]] def p_set_type(p): '''set_type : SET '<' field_type '>' ''' - p[0] = SetType(['set', p[3]]) + p[0] = [TType.SET, p[3]] def p_definition_type(p): @@ -316,17 +383,38 @@ def p_definition_type(p): p[0] = p[1] -parser = yacc.yacc(debug=False, write_tables=0) +thrift_stack = [] +include_dir_ = '.' + + +def parse(path, module_name=None, include_dir=None, lexer=None, parser=None): + if lexer is None: + lexer = lex.lex() + if parser is None: + parser = yacc.yacc(debug=False, write_tables=0) + + global include_dir_ + + if include_dir is not None: + include_dir_ = include_dir + + if not path.endswith('.thrift'): + raise ThriftParserError('Path should end with .thrift') -thrift = None + with open(path) as fh: + data = fh.read() + if module_name is None: + module_name = os.path.basename(path)[:-7] + '_thrift' -def parse(data): - global thrift - thrift = Thrift() + if not module_name.endswith('_thrift'): + raise ThriftParserError('ThriftPy can only generate module with ' + '\'_thrift\' suffix') + thrift = types.ModuleType(module_name) + thrift_stack.append(thrift) lexer.lineno = 1 parser.parse(data) - return thrift + return thrift_stack.pop() def _parse_seq(p): @@ -336,3 +424,218 @@ def _parse_seq(p): p[0] = [p[1]] + p[2] elif len(p) == 1: p[0] = [] + + +def _cast(t): # noqa + if t == TType.BOOL: + return _cast_bool + if t == TType.BYTE: + return _cast_byte + if t == TType.I16: + return _cast_i16 + if t == TType.I32: + return _cast_i32 + if t == TType.I64: + return _cast_i64 + if t == TType.DOUBLE: + return _cast_double + if t == TType.STRING: + return _cast_string + if t == TType.BINARY: + return _cast_binary + if t[0] == TType.LIST: + return _cast_list(t) + if t[0] == TType.SET: + return _cast_set(t) + if t[0] == TType.MAP: + return _cast_map(t) + if t[0] == TType.I32: + return _cast_enum(t) + if t[0] == TType.STRUCT: + return _cast_struct(t) + + +def _cast_bool(v): + assert isinstance(v, bool) + return v + + +def _cast_byte(v): + assert isinstance(v, str) + return v + + +def _cast_i16(v): + assert isinstance(v, int) + return v + + +def _cast_i32(v): + assert isinstance(v, int) + return v + + +def _cast_i64(v): + assert isinstance(v, int) + return v + + +def _cast_double(v): + assert isinstance(v, float) + return v + + +def _cast_string(v): + assert isinstance(v, str) + return v + + +def _cast_binary(v): + assert isinstance(v, str) + return v + + +def _cast_list(t): + assert t[0] == TType.LIST + + def __cast_list(v): + assert isinstance(v, list) + map(_cast(t[1]), v) + return v + return __cast_list + + +def _cast_set(t): + assert t[0] == TType.SET + + def __cast_set(v): + assert isinstance(v, (list, set)) + map(_cast(t[1]), v) + if not isinstance(v, set): + return set(v) + return v + return __cast_set + + +def _cast_map(t): + assert t[0] == TType.MAP + + def __cast_map(v): + assert isinstance(v, dict) + for key in v: + v[_cast(t[1][0])(key)] = \ + _cast(t[1][1])(v[key]) + return v + return __cast_map + + +def _cast_enum(t): + assert t[0] == TType.I32 + + def __cast_enum(v): + assert isinstance(v, int) + if v in getattr(t[1], '_named_values'): + return v + raise ThriftParserError('Couldn\'t find a named value in enum ' + '%s for value %d' % (t[1].__name__, v)) + return __cast_enum + + +def _cast_struct(t): # struct/exception/union + assert t[0] == TType.STRUCT + + def __cast_struct(v): + if isinstance(v, t[1]): + return v # already cast + + assert isinstance(v, dict) + tspec = getattr(t[1], '_tspec') + + for key in tspec: # requirement check + if tspec[key][0] and key not in v: + raise ThriftParserError('Field %r was required to create ' + 'constant for type %r' % + (key, t[1].__name__)) + + for key in v: # cast values + if key not in tspec: + raise ThriftParserError('No field named %r was ' + 'found in struct of type %r' % + (key, t[1].__name__)) + v[key] = _cast(tspec[key][1])(v[key]) + return t[1](**v) + return __cast_struct + + +def _make_enum(name, kvs): + attrs = {'__module__': thrift_stack[-1].__name__, '_ttype': TType.I32} + cls = type(name, (object, ), attrs) + named_values = set() + for key, val in kvs: + if val is not None: + named_values.add(val) + setattr(cls, '_named_values', named_values) + + if kvs: + val = kvs[0][1] + if val is None: + val = -1 + for item in kvs: + if item[1] is None: + item[1] = val + 1 + val = item[1] + for key, val in kvs: + setattr(cls, key, val) + return cls + + +def _make_struct(name, fields, ttype=TType.STRUCT, base_cls=TPayload): + attrs = {'__module__': thrift_stack[-1].__name__, '_ttype': ttype} + cls = type(name, (base_cls, ), attrs) + thrift_spec = {} + default_spec = [] + _tspec = {} + + for field in fields: + ttype = field[2] + if isinstance(ttype, int): # not a list + thrift_spec[field[0]] = ttype, field[3], field[1] + else: + thrift_spec[field[0]] = ttype[0], field[3], ttype[1], field[1] + default_spec.append((field[3], field[4])) + _tspec[field[3]] = [field[1], ttype] + setattr(cls, 'thrift_spec', thrift_spec) + setattr(cls, 'default_spec', default_spec) + setattr(cls, '_tspec', _tspec) + gen_init(cls, thrift_spec, default_spec) + return cls + + +def _make_service(name, funcs, extends): + if extends is None: + extends = object + + attrs = {'__module__': thrift_stack[-1].__name__} + cls = type(name, (extends, ), attrs) + thrift_services = [] + + for func in funcs: + func_name = func[2] + # args payload cls + args_name = '%s_args' % func_name + args_fields = func[3] + args_cls = _make_struct(args_name, args_fields) + setattr(cls, args_name, args_cls) + # result payload cls + result_name = '%s_result' % func_name + result_type = func[1] + result_throws = func[4] + result_oneway = func[0] + result_cls = _make_struct(result_name, result_throws) + setattr(result_cls, 'oneway', result_oneway) + result_cls.thrift_spec[0] = (result_type, 'success') + result_cls.default_spec.insert(0, ('success', None)) + setattr(cls, result_name, result_cls) + thrift_services.append(func_name) + setattr(cls, 'thrift_services', thrift_services) + return cls From 568b991d21345297de050e3494106255b5e41e80 Mon Sep 17 00:00:00 2001 From: hit9 Date: Fri, 23 Jan 2015 15:19:56 +0800 Subject: [PATCH 04/26] Referenced enum value should be named --- tests/addressbook.thrift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/addressbook.thrift b/tests/addressbook.thrift index f1a18bd..1d7ad04 100644 --- a/tests/addressbook.thrift +++ b/tests/addressbook.thrift @@ -5,7 +5,7 @@ const i16 DEFAULT_LIST_SIZE = 10 typedef i32 timestamp enum PhoneType { - MOBILE, + MOBILE = 0, HOME, WORK, } From 3f72b003a76ce2fae08fd92074e2a4ac7cb44486 Mon Sep 17 00:00:00 2001 From: hit9 Date: Fri, 23 Jan 2015 15:20:56 +0800 Subject: [PATCH 05/26] Fix module_name and child/father searching on ref --- thriftpy/parser/parser.py | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/thriftpy/parser/parser.py b/thriftpy/parser/parser.py index 02fc137..89baf76 100644 --- a/thriftpy/parser/parser.py +++ b/thriftpy/parser/parser.py @@ -217,9 +217,11 @@ def p_service(p): raise ThriftParserError('Can\'t find service %r for ' 'service %r to extend' % (p[4], p[2])) - if not hasattr(child, 'thrift_services'): - raise ThriftParserError('Can\'t extends %r, not a service' - % p[4]) + father = child + + if not hasattr(child, 'thrift_services'): + raise ThriftParserError('Can\'t extends %r, not a service' + % p[4]) extends = child else: @@ -317,15 +319,20 @@ def p_field_type(p): def p_ref_type(p): '''ref_type : IDENTIFIER''' - ttype = getattr(thrift_stack[-1], p[1], None) + father = thrift_stack[-1] + + for name in p[1].split('.'): + child = getattr(father, name, None) + if child is None: + raise ThriftParserError('No type found: %r, at line %d' % + (p[1], p.lineno(1))) - if ttype is None: - raise ThriftParserError('No type found: %r, at line %d' % - (p[1], p.lineno(1))) - if hasattr(ttype, '_ttype'): - p[0] = getattr(ttype, '_ttype'), ttype + father = child + + if hasattr(child, '_ttype'): + p[0] = getattr(child, '_ttype'), child else: - p[0] = ttype + p[0] = child def p_base_type(p): # noqa @@ -404,12 +411,13 @@ def parse(path, module_name=None, include_dir=None, lexer=None, parser=None): with open(path) as fh: data = fh.read() - if module_name is None: - module_name = os.path.basename(path)[:-7] + '_thrift' - - if not module_name.endswith('_thrift'): + if module_name is not None and not module_name.endswith('_thrift'): raise ThriftParserError('ThriftPy can only generate module with ' '\'_thrift\' suffix') + + if module_name is None: + module_name = os.path.basename(path)[:-7] + thrift = types.ModuleType(module_name) thrift_stack.append(thrift) lexer.lineno = 1 From 208917ad4ab5dfcd3c65fc4dda53dbc07610868e Mon Sep 17 00:00:00 2001 From: hit9 Date: Fri, 23 Jan 2015 15:59:30 +0800 Subject: [PATCH 06/26] Add thrift_loader cache --- thriftpy/parser/__init__.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/thriftpy/parser/__init__.py b/thriftpy/parser/__init__.py index 4ee1c68..38620d1 100644 --- a/thriftpy/parser/__init__.py +++ b/thriftpy/parser/__init__.py @@ -12,16 +12,25 @@ from .parser import parse +_thriftloader = {} + + def load(path, module_name=None, include_dir=None): """Load thrift_file as a module The module loaded and objects inside may only be pickled if module_name was provided. """ + global _thriftloader + _thriftloader_key = module_name or path + if _thriftloader_key in _thriftloader: + return _thriftloader[_thriftloader_key] + real_module = bool(module_name) thrift = parse(path, module_name, include_dir=include_dir) if real_module: sys.modules[module_name] = thrift + _thriftloader[_thriftloader_key] = thrift return thrift From 3e45b9736f2a0edba79ea71fa790aee17941ef83 Mon Sep 17 00:00:00 2001 From: hit9 Date: Fri, 23 Jan 2015 15:59:40 +0800 Subject: [PATCH 07/26] Use tuple as container types --- thriftpy/parser/parser.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/thriftpy/parser/parser.py b/thriftpy/parser/parser.py index 89baf76..49d6e48 100644 --- a/thriftpy/parser/parser.py +++ b/thriftpy/parser/parser.py @@ -371,17 +371,17 @@ def p_container_type(p): def p_map_type(p): '''map_type : MAP '<' field_type ',' field_type '>' ''' - p[0] = [TType.MAP, [p[3], p[5]]] + p[0] = TType.MAP, (p[3], p[5]) def p_list_type(p): '''list_type : LIST '<' field_type '>' ''' - p[0] = [TType.LIST, p[3]] + p[0] = TType.LIST, p[3] def p_set_type(p): '''set_type : SET '<' field_type '>' ''' - p[0] = [TType.SET, p[3]] + p[0] = TType.SET, p[3] def p_definition_type(p): @@ -611,7 +611,7 @@ def _make_struct(name, fields, ttype=TType.STRUCT, base_cls=TPayload): else: thrift_spec[field[0]] = ttype[0], field[3], ttype[1], field[1] default_spec.append((field[3], field[4])) - _tspec[field[3]] = [field[1], ttype] + _tspec[field[3]] = field[1], ttype setattr(cls, 'thrift_spec', thrift_spec) setattr(cls, 'default_spec', default_spec) setattr(cls, '_tspec', _tspec) From 42dbd02b803a67c1b5914ced11a320fa628ab16e Mon Sep 17 00:00:00 2001 From: hit9 Date: Fri, 23 Jan 2015 16:11:25 +0800 Subject: [PATCH 08/26] Fix result spec --- thriftpy/parser/parser.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/thriftpy/parser/parser.py b/thriftpy/parser/parser.py index 49d6e48..4612126 100644 --- a/thriftpy/parser/parser.py +++ b/thriftpy/parser/parser.py @@ -606,10 +606,7 @@ def _make_struct(name, fields, ttype=TType.STRUCT, base_cls=TPayload): for field in fields: ttype = field[2] - if isinstance(ttype, int): # not a list - thrift_spec[field[0]] = ttype, field[3], field[1] - else: - thrift_spec[field[0]] = ttype[0], field[3], ttype[1], field[1] + thrift_spec[field[0]] = _ttype_spec(ttype, field[3], field[1]) default_spec.append((field[3], field[4])) _tspec[field[3]] = field[1], ttype setattr(cls, 'thrift_spec', thrift_spec) @@ -641,9 +638,16 @@ def _make_service(name, funcs, extends): result_oneway = func[0] result_cls = _make_struct(result_name, result_throws) setattr(result_cls, 'oneway', result_oneway) - result_cls.thrift_spec[0] = (result_type, 'success') + result_cls.thrift_spec[0] = _ttype_spec(result_type, 'success') result_cls.default_spec.insert(0, ('success', None)) setattr(cls, result_name, result_cls) thrift_services.append(func_name) setattr(cls, 'thrift_services', thrift_services) return cls + + +def _ttype_spec(ttype, name, required=True): + if isinstance(ttype, int): + return ttype, name, required + else: + return ttype[0], name, ttype[1], required From 2fc6d714b83207490aedb9ea56f2590271e08947 Mon Sep 17 00:00:00 2001 From: hit9 Date: Fri, 23 Jan 2015 16:15:40 +0800 Subject: [PATCH 09/26] Default required is False --- thriftpy/parser/parser.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/thriftpy/parser/parser.py b/thriftpy/parser/parser.py index 4612126..21d54db 100644 --- a/thriftpy/parser/parser.py +++ b/thriftpy/parser/parser.py @@ -308,7 +308,7 @@ def p_field_req(p): if len(p) == 2: p[0] = p[1] == 'required' elif len(p) == 1: - p[0] = True # default: required=True + p[0] = False # default: required=False def p_field_type(p): @@ -646,7 +646,7 @@ def _make_service(name, funcs, extends): return cls -def _ttype_spec(ttype, name, required=True): +def _ttype_spec(ttype, name, required=False): if isinstance(ttype, int): return ttype, name, required else: From 728487d41c95d3ed25d517141fce403a08447013 Mon Sep 17 00:00:00 2001 From: hit9 Date: Fri, 23 Jan 2015 16:32:54 +0800 Subject: [PATCH 10/26] Move thrift parsing cache to parser; and add option `enable_cache` --- thriftpy/parser/__init__.py | 9 --------- thriftpy/parser/parser.py | 18 ++++++++++++++++-- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/thriftpy/parser/__init__.py b/thriftpy/parser/__init__.py index 38620d1..4ee1c68 100644 --- a/thriftpy/parser/__init__.py +++ b/thriftpy/parser/__init__.py @@ -12,25 +12,16 @@ from .parser import parse -_thriftloader = {} - - def load(path, module_name=None, include_dir=None): """Load thrift_file as a module The module loaded and objects inside may only be pickled if module_name was provided. """ - global _thriftloader - _thriftloader_key = module_name or path - if _thriftloader_key in _thriftloader: - return _thriftloader[_thriftloader_key] - real_module = bool(module_name) thrift = parse(path, module_name, include_dir=include_dir) if real_module: sys.modules[module_name] = thrift - _thriftloader[_thriftloader_key] = thrift return thrift diff --git a/thriftpy/parser/parser.py b/thriftpy/parser/parser.py index 21d54db..ee9aad9 100644 --- a/thriftpy/parser/parser.py +++ b/thriftpy/parser/parser.py @@ -392,9 +392,19 @@ def p_definition_type(p): thrift_stack = [] include_dir_ = '.' +thrift_cache = {} -def parse(path, module_name=None, include_dir=None, lexer=None, parser=None): +def parse(path, module_name=None, include_dir=None, + lexer=None, parser=None, enable_cache=True): + + global thrift_cache + + cache_key = module_name or os.path.normpath(path) + + if enable_cache and cache_key in thrift_cache: + return thrift_cache[cache_key] + if lexer is None: lexer = lex.lex() if parser is None: @@ -422,7 +432,11 @@ def parse(path, module_name=None, include_dir=None, lexer=None, parser=None): thrift_stack.append(thrift) lexer.lineno = 1 parser.parse(data) - return thrift_stack.pop() + thrift_stack.pop() + + if enable_cache: + thrift_cache[cache_key] = thrift + return thrift def _parse_seq(p): From 0632630dc680ca243f8bb3010adb5f0c1a2e4e29 Mon Sep 17 00:00:00 2001 From: hit9 Date: Fri, 23 Jan 2015 17:37:26 +0800 Subject: [PATCH 11/26] Fix result struct ttype VOID --- thriftpy/parser/parser.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/thriftpy/parser/parser.py b/thriftpy/parser/parser.py index ee9aad9..60aa006 100644 --- a/thriftpy/parser/parser.py +++ b/thriftpy/parser/parser.py @@ -611,7 +611,8 @@ def _make_enum(name, kvs): return cls -def _make_struct(name, fields, ttype=TType.STRUCT, base_cls=TPayload): +def _make_struct(name, fields, ttype=TType.STRUCT, base_cls=TPayload, + _gen_init=True): attrs = {'__module__': thrift_stack[-1].__name__, '_ttype': ttype} cls = type(name, (base_cls, ), attrs) thrift_spec = {} @@ -626,7 +627,8 @@ def _make_struct(name, fields, ttype=TType.STRUCT, base_cls=TPayload): setattr(cls, 'thrift_spec', thrift_spec) setattr(cls, 'default_spec', default_spec) setattr(cls, '_tspec', _tspec) - gen_init(cls, thrift_spec, default_spec) + if _gen_init: + gen_init(cls, thrift_spec, default_spec) return cls @@ -650,10 +652,13 @@ def _make_service(name, funcs, extends): result_type = func[1] result_throws = func[4] result_oneway = func[0] - result_cls = _make_struct(result_name, result_throws) + result_cls = _make_struct(result_name, result_throws, + _gen_init=False) setattr(result_cls, 'oneway', result_oneway) - result_cls.thrift_spec[0] = _ttype_spec(result_type, 'success') - result_cls.default_spec.insert(0, ('success', None)) + if result_type != TType.VOID: + result_cls.thrift_spec[0] = _ttype_spec(result_type, 'success') + result_cls.default_spec.insert(0, ('success', None)) + gen_init(result_cls, result_cls.thrift_spec, result_cls.default_spec) setattr(cls, result_name, result_cls) thrift_services.append(func_name) setattr(cls, 'thrift_services', thrift_services) From 5d964c76353fbd7bcc1ed04bd601ac14d45bde8a Mon Sep 17 00:00:00 2001 From: hit9 Date: Fri, 23 Jan 2015 17:44:49 +0800 Subject: [PATCH 12/26] Remove old parser tests --- tests/parser-cases/json/comments.json | 11 -- tests/parser-cases/json/consts.json | 19 --- tests/parser-cases/json/enums.json | 27 ---- tests/parser-cases/json/escape.json | 13 -- tests/parser-cases/json/exceptions.json | 28 ---- tests/parser-cases/json/includes.json | 29 ---- tests/parser-cases/json/namespaces.json | 11 -- tests/parser-cases/json/services.json | 57 ------- tests/parser-cases/json/structs.json | 43 ----- tests/parser-cases/json/tutorial.json | 150 ----------------- tests/parser-cases/json/typedefs.json | 18 --- tests/parser-cases/json/unions.json | 28 ---- tests/parser-cases/thrift/bad_grammer.thrift | 3 - tests/parser-cases/thrift/bad_lexer.thrift | 6 - .../thrift/bad_lexer_escaping.thrift | 1 - tests/parser-cases/thrift/comments.thrift | 12 -- tests/parser-cases/thrift/consts.thrift | 7 - tests/parser-cases/thrift/enums.thrift | 17 -- tests/parser-cases/thrift/escape.thrift | 1 - tests/parser-cases/thrift/exceptions.thrift | 4 - tests/parser-cases/thrift/includes.thrift | 9 -- tests/parser-cases/thrift/namespaces.thrift | 2 - tests/parser-cases/thrift/services.thrift | 8 - tests/parser-cases/thrift/structs.thrift | 11 -- tests/parser-cases/thrift/tutorial.thrift | 152 ------------------ tests/parser-cases/thrift/typedefs.thrift | 6 - tests/parser-cases/thrift/unions.thrift | 4 - tests/test_parser.py | 65 -------- 28 files changed, 742 deletions(-) delete mode 100644 tests/parser-cases/json/comments.json delete mode 100644 tests/parser-cases/json/consts.json delete mode 100644 tests/parser-cases/json/enums.json delete mode 100644 tests/parser-cases/json/escape.json delete mode 100644 tests/parser-cases/json/exceptions.json delete mode 100644 tests/parser-cases/json/includes.json delete mode 100644 tests/parser-cases/json/namespaces.json delete mode 100644 tests/parser-cases/json/services.json delete mode 100644 tests/parser-cases/json/structs.json delete mode 100644 tests/parser-cases/json/tutorial.json delete mode 100644 tests/parser-cases/json/typedefs.json delete mode 100644 tests/parser-cases/json/unions.json delete mode 100644 tests/parser-cases/thrift/bad_grammer.thrift delete mode 100644 tests/parser-cases/thrift/bad_lexer.thrift delete mode 100644 tests/parser-cases/thrift/bad_lexer_escaping.thrift delete mode 100644 tests/parser-cases/thrift/comments.thrift delete mode 100644 tests/parser-cases/thrift/consts.thrift delete mode 100644 tests/parser-cases/thrift/enums.thrift delete mode 100644 tests/parser-cases/thrift/escape.thrift delete mode 100644 tests/parser-cases/thrift/exceptions.thrift delete mode 100644 tests/parser-cases/thrift/includes.thrift delete mode 100644 tests/parser-cases/thrift/namespaces.thrift delete mode 100644 tests/parser-cases/thrift/services.thrift delete mode 100644 tests/parser-cases/thrift/structs.thrift delete mode 100644 tests/parser-cases/thrift/tutorial.thrift delete mode 100644 tests/parser-cases/thrift/typedefs.thrift delete mode 100644 tests/parser-cases/thrift/unions.thrift delete mode 100644 tests/test_parser.py diff --git a/tests/parser-cases/json/comments.json b/tests/parser-cases/json/comments.json deleted file mode 100644 index bbb68a3..0000000 --- a/tests/parser-cases/json/comments.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "includes": [], - "namespaces": {}, - "enums": {}, - "structs": {}, - "unions": {}, - "exceptions": {}, - "typedefs": {}, - "consts": {}, - "services": {} -} diff --git a/tests/parser-cases/json/consts.json b/tests/parser-cases/json/consts.json deleted file mode 100644 index b17fc74..0000000 --- a/tests/parser-cases/json/consts.json +++ /dev/null @@ -1,19 +0,0 @@ -{ - "consts": { - "num1": 1, - "num2": 2000, - "num3": 30000000000, - "str": "hello", - "lst": [1, 2, 3], - "mp": {"key": "val"}, - "st": [1, 2, 3] - }, - "includes": [], - "namespaces": {}, - "enums": {}, - "structs": {}, - "unions": {}, - "exceptions": {}, - "typedefs": {}, - "services": {} -} diff --git a/tests/parser-cases/json/enums.json b/tests/parser-cases/json/enums.json deleted file mode 100644 index b541516..0000000 --- a/tests/parser-cases/json/enums.json +++ /dev/null @@ -1,27 +0,0 @@ -{ - "typedefs": {}, - "includes": [], - "namespaces": {}, - "structs": {}, - "unions": {}, - "consts": {}, - "services": {}, - "exceptions": {}, - "enums": { - "Enum1": { - "A": 1, - "B": 2, - "C": 3 - }, - "Enum2": { - "A": 1, - "B": 2, - "C": 3 - }, - "Enum3": { - "A": 0, - "B": 1, - "C": 2 - } - } -} diff --git a/tests/parser-cases/json/escape.json b/tests/parser-cases/json/escape.json deleted file mode 100644 index dc42d13..0000000 --- a/tests/parser-cases/json/escape.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "consts": { - "name": "\t\n\r\\" - }, - "includes": [], - "namespaces": {}, - "enums": {}, - "structs": {}, - "unions": {}, - "exceptions": {}, - "typedefs": {}, - "services": {} -} diff --git a/tests/parser-cases/json/exceptions.json b/tests/parser-cases/json/exceptions.json deleted file mode 100644 index 3738dfa..0000000 --- a/tests/parser-cases/json/exceptions.json +++ /dev/null @@ -1,28 +0,0 @@ -{ - "includes": [], - "namespaces": {}, - "typedefs": {}, - "enums": {}, - "unions": {}, - "consts": {}, - "services": {}, - "exceptions": { - "SyntaxError": [ - { - "id": 1, - "name": "lineNumber", - "type": "i32", - "value": null, - "requirement": null - }, - { - "id": 2, - "name": "errorMessage", - "type": "string", - "value": null, - "requirement": null - } - ] - }, - "structs": {} -} diff --git a/tests/parser-cases/json/includes.json b/tests/parser-cases/json/includes.json deleted file mode 100644 index 6210992..0000000 --- a/tests/parser-cases/json/includes.json +++ /dev/null @@ -1,29 +0,0 @@ -{ - "includes": ["structs.thrift", "enums.thrift", "typedefs.thrift"], - "namespaces": {}, - "enums": {}, - "structs": { - "Both": [ - {"requirement": null, - "type": "structs.Person", - "id": 1, - "value": null, - "name": "person"}, - {"requirement": null, - "type": "enums.Enum1", - "id": 2, - "value": null, - "name": "enum1"}, - {"requirement": null, - "type": "typedefs.bytes", - "id": 3, - "value": null, - "name": "bytes" - } - ]}, - "unions": {}, - "exceptions": {}, - "typedefs": {}, - "consts": {}, - "services": {} -} diff --git a/tests/parser-cases/json/namespaces.json b/tests/parser-cases/json/namespaces.json deleted file mode 100644 index 75af1ac..0000000 --- a/tests/parser-cases/json/namespaces.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "namespaces": {"SAMPLE": "php", "sample": "py"}, - "includes": [], - "enums": {}, - "structs": {}, - "unions": {}, - "exceptions": {}, - "typedefs": {}, - "consts": {}, - "services": {} -} diff --git a/tests/parser-cases/json/services.json b/tests/parser-cases/json/services.json deleted file mode 100644 index 019f404..0000000 --- a/tests/parser-cases/json/services.json +++ /dev/null @@ -1,57 +0,0 @@ -{ - "includes": [], - "namespaces": {}, - "typedefs": {}, - "enums": {}, - "unions": {}, - "consts": {}, - "exceptions": { - "NetError": [] - }, - "structs": {}, - "services": { - "SMS": { - "extends": "BasicService", - "apis": { - "get_message_by_id": { - "name": "get_message_by_id", - "type": "string", - "oneway": false, - "throws": [], - "fields": [ - { - "id": 1, - "name": "id", - "type": "i32", - "value": null, - "requirement": "required" - } - ] - }, - "send": { - "name": "send", - "type": "void", - "oneway": true, - "throws": [ - { - "type": "NetError", - "id": 1, - "value": null, - "requirement": null, - "name": "net_error" - } - ], - "fields": [ - { - "id": 1, - "name": "to", - "type": "string", - "value": null, - "requirement": "required" - } - ] - } - } - } - } -} diff --git a/tests/parser-cases/json/structs.json b/tests/parser-cases/json/structs.json deleted file mode 100644 index b115e81..0000000 --- a/tests/parser-cases/json/structs.json +++ /dev/null @@ -1,43 +0,0 @@ -{ - "includes": [], - "namespaces": {}, - "typedefs": {}, - "enums": { - "Country": { - "CA": 3, - "UK": 2, - "US": 1 - } - }, - "unions": {}, - "consts": {}, - "services": {}, - "exceptions": {}, - "structs": { - "Person": [ - { - "id": 1, - "name": "name", - "type": "string", - "value": null, - "requirement": null - }, - { - "id": 2, - "name": "age", - "type": "i32", - "value": null, - "requirement": null - }, - { - "id": 3, - "name": "country", - "type": "Country", - "value": { - "v": "Country.US" - }, - "requirement": null - } - ] - } -} diff --git a/tests/parser-cases/json/tutorial.json b/tests/parser-cases/json/tutorial.json deleted file mode 100644 index 18e99ab..0000000 --- a/tests/parser-cases/json/tutorial.json +++ /dev/null @@ -1,150 +0,0 @@ -{ - "namespaces": { - "tutorial": "perl" - }, - "includes": [ - "shared.thrift" - ], - "services": { - "Calculator": { - "extends": "shared.SharedService", - "apis": { - "add": { - "fields": [ - { - "requirement": null, - "type": "i32", - "id": 1, - "value": null, - "name": "num1" - }, - { - "requirement": null, - "type": "i32", - "id": 2, - "value": null, - "name": "num2" - } - ], - "oneway": false, - "type": "i32", - "name": "add", - "throws": [] - }, - "ping": { - "fields": [], - "oneway": false, - "type": "void", - "name": "ping", - "throws": [] - }, - "calculate": { - "fields": [ - { - "requirement": null, - "type": "i32", - "id": 1, - "value": null, - "name": "logid" - }, - { - "requirement": null, - "type": "Work", - "id": 2, - "value": null, - "name": "w" - } - ], - "oneway": false, - "type": "i32", - "name": "calculate", - "throws": [ - { - "requirement": null, - "type": "InvalidOperation", - "id": 1, - "value": null, - "name": "ouch" - } - ] - }, - "zip": { - "fields": [], - "oneway": true, - "type": "void", - "name": "zip", - "throws": [] - } - } - } - }, - "unions": {}, - "typedefs": { - "MyInteger": "i32" - }, - "consts": { - "MAPCONSTANT": { - "goodnight": "moon", - "hello": "world" - }, - "INT32CONSTANT": 9853 - }, - "exceptions": { - "InvalidOperation": [ - { - "requirement": null, - "type": "i32", - "id": 1, - "value": null, - "name": "what" - }, - { - "requirement": null, - "type": "string", - "id": 2, - "value": null, - "name": "why" - } - ] - }, - "enums": { - "Operation": { - "MULTIPLY": 3, - "ADD": 1, - "SUBTRACT": 2, - "DIVIDE": 4 - } - }, - "structs": { - "Work": [ - { - "requirement": null, - "type": "i32", - "id": 1, - "value": 0, - "name": "num1" - }, - { - "requirement": null, - "type": "i32", - "id": 2, - "value": null, - "name": "num2" - }, - { - "requirement": null, - "type": "Operation", - "id": 3, - "value": null, - "name": "op" - }, - { - "requirement": "optional", - "type": "string", - "id": 4, - "value": null, - "name": "comment" - } - ] - } -} diff --git a/tests/parser-cases/json/typedefs.json b/tests/parser-cases/json/typedefs.json deleted file mode 100644 index 710cccb..0000000 --- a/tests/parser-cases/json/typedefs.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "typedefs": { - "myInt": "i32", - "str": "string", - "miniInt": "byte", - "byteMap": ["map", "bytes", "int"], - "byteSet": ["set", "miniInt"], - "bytes": ["list", "miniInt"] - }, - "includes": [], - "namespaces": {}, - "enums": {}, - "structs": {}, - "unions": {}, - "consts": {}, - "services": {}, - "exceptions": {} -} diff --git a/tests/parser-cases/json/unions.json b/tests/parser-cases/json/unions.json deleted file mode 100644 index d3a6766..0000000 --- a/tests/parser-cases/json/unions.json +++ /dev/null @@ -1,28 +0,0 @@ -{ - "typedefs": {}, - "includes": [], - "namespaces": {}, - "structs": {}, - "consts": {}, - "services": {}, - "exceptions": {}, - "enums": {}, - "unions": { - "Sample": [ - { - "id": 1, - "name": "size", - "type": "i32", - "value": null, - "requirement": null - }, - { - "id": 2, - "name": "name", - "type": "string", - "value": null, - "requirement": null - } - ] - } -} diff --git a/tests/parser-cases/thrift/bad_grammer.thrift b/tests/parser-cases/thrift/bad_grammer.thrift deleted file mode 100644 index ef1e5f7..0000000 --- a/tests/parser-cases/thrift/bad_grammer.thrift +++ /dev/null @@ -1,3 +0,0 @@ -struct Bad Person { // bad! - 1: string name -} diff --git a/tests/parser-cases/thrift/bad_lexer.thrift b/tests/parser-cases/thrift/bad_lexer.thrift deleted file mode 100644 index 36cb8a7..0000000 --- a/tests/parser-cases/thrift/bad_lexer.thrift +++ /dev/null @@ -1,6 +0,0 @@ -typedef i32 MyInt - -struct Person { - 1: i32 age - 2: string name -} ! /* lexer error, unlknow token */ diff --git a/tests/parser-cases/thrift/bad_lexer_escaping.thrift b/tests/parser-cases/thrift/bad_lexer_escaping.thrift deleted file mode 100644 index 4a30cbc..0000000 --- a/tests/parser-cases/thrift/bad_lexer_escaping.thrift +++ /dev/null @@ -1 +0,0 @@ -const string name = "\b" diff --git a/tests/parser-cases/thrift/comments.thrift b/tests/parser-cases/thrift/comments.thrift deleted file mode 100644 index 6b13178..0000000 --- a/tests/parser-cases/thrift/comments.thrift +++ /dev/null @@ -1,12 +0,0 @@ -/* inline comment */ - -# unit style comment - -/* - * multiple - * lines - * comment - */ - -/** silly comment - */ diff --git a/tests/parser-cases/thrift/consts.thrift b/tests/parser-cases/thrift/consts.thrift deleted file mode 100644 index 1df26ee..0000000 --- a/tests/parser-cases/thrift/consts.thrift +++ /dev/null @@ -1,7 +0,0 @@ -const i16 num1 = 1 -const i32 num2 = 2000 -const i64 num3 = 30000000000 -const string str = "hello" -const list lst = [1, 2, 3] -const set st = [1, 2, 3] -const map mp = {"key": "val"} diff --git a/tests/parser-cases/thrift/enums.thrift b/tests/parser-cases/thrift/enums.thrift deleted file mode 100644 index 6aaae5b..0000000 --- a/tests/parser-cases/thrift/enums.thrift +++ /dev/null @@ -1,17 +0,0 @@ -enum Enum1 { - A = 1 - B = 2 - C = 3 -} - -enum Enum2 { - A = 1, - B = 2, - C = 3 -} - -enum Enum3 { - A, - B, - C = 2 -} diff --git a/tests/parser-cases/thrift/escape.thrift b/tests/parser-cases/thrift/escape.thrift deleted file mode 100644 index c3af023..0000000 --- a/tests/parser-cases/thrift/escape.thrift +++ /dev/null @@ -1 +0,0 @@ -const string name = "\t\n\r\\" diff --git a/tests/parser-cases/thrift/exceptions.thrift b/tests/parser-cases/thrift/exceptions.thrift deleted file mode 100644 index 815404f..0000000 --- a/tests/parser-cases/thrift/exceptions.thrift +++ /dev/null @@ -1,4 +0,0 @@ -exception SyntaxError { - 1: i32 lineNumber - 2: string errorMessage -} diff --git a/tests/parser-cases/thrift/includes.thrift b/tests/parser-cases/thrift/includes.thrift deleted file mode 100644 index 6d9571c..0000000 --- a/tests/parser-cases/thrift/includes.thrift +++ /dev/null @@ -1,9 +0,0 @@ -include "structs.thrift" -include "enums.thrift" -include "typedefs.thrift" - -struct Both { - 1: structs.Person person; - 2: enums.Enum1 enum1; - 3: typedefs.bytes bytes; -} diff --git a/tests/parser-cases/thrift/namespaces.thrift b/tests/parser-cases/thrift/namespaces.thrift deleted file mode 100644 index eea9ad0..0000000 --- a/tests/parser-cases/thrift/namespaces.thrift +++ /dev/null @@ -1,2 +0,0 @@ -namespace php SAMPLE -namespace py sample diff --git a/tests/parser-cases/thrift/services.thrift b/tests/parser-cases/thrift/services.thrift deleted file mode 100644 index 62d763f..0000000 --- a/tests/parser-cases/thrift/services.thrift +++ /dev/null @@ -1,8 +0,0 @@ -exception NetError {} - -service SMS extends BasicService { - string get_message_by_id(1: required i32 id), - oneway void send(1: required string to) throws ( - 1: NetError net_error - ) -} diff --git a/tests/parser-cases/thrift/structs.thrift b/tests/parser-cases/thrift/structs.thrift deleted file mode 100644 index 7aa3acb..0000000 --- a/tests/parser-cases/thrift/structs.thrift +++ /dev/null @@ -1,11 +0,0 @@ -enum Country { - US = 1, - UK = 2, - CA = 3, -} - -struct Person { - 1: string name, - 2: i32 age - 3: Country country = Country.US -} diff --git a/tests/parser-cases/thrift/tutorial.thrift b/tests/parser-cases/thrift/tutorial.thrift deleted file mode 100644 index 3150151..0000000 --- a/tests/parser-cases/thrift/tutorial.thrift +++ /dev/null @@ -1,152 +0,0 @@ -/* - * 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. - */ - -# Thrift Tutorial -# Mark Slee (mcslee@facebook.com) -# -# This file aims to teach you how to use Thrift, in a .thrift file. Neato. The -# first thing to notice is that .thrift files support standard shell comments. -# This lets you make your thrift file executable and include your Thrift build -# step on the top line. And you can place comments like this anywhere you like. -# -# Before running this file, you will need to have installed the thrift compiler -# into /usr/local/bin. - -/** - * The first thing to know about are types. The available types in Thrift are: - * - * bool Boolean, one byte - * byte Signed byte - * i16 Signed 16-bit integer - * i32 Signed 32-bit integer - * i64 Signed 64-bit integer - * double 64-bit floating point value - * string String - * binary Blob (byte array) - * map Map from one type to another - * list Ordered list of one type - * set Set of unique elements of one type - * - * Did you also notice that Thrift supports C style comments? - */ - -// Just in case you were wondering... yes. We support simple C comments too. - -/** - * Thrift files can reference other Thrift files to include common struct - * and service definitions. These are found using the current path, or by - * searching relative to any paths specified with the -I compiler flag. - * - * Included objects are accessed using the name of the .thrift file as a - * prefix. i.e. shared.SharedObject - */ -include "shared.thrift" - -/** - * Thrift files can namespace, package, or prefix their output in various - * target languages. - */ -namespace cpp tutorial -namespace d tutorial -namespace java tutorial -namespace php tutorial -namespace perl tutorial - -/** - * Thrift lets you do typedefs to get pretty names for your types. Standard - * C style here. - */ -typedef i32 MyInteger - -/** - * Thrift also lets you define constants for use across languages. Complex - * types and structs are specified using JSON notation. - */ -const i32 INT32CONSTANT = 9853 -const map MAPCONSTANT = {'hello':'world', 'goodnight':'moon'} - -/** - * You can define enums, which are just 32 bit integers. Values are optional - * and start at 1 if not supplied, C style again. - */ -enum Operation { - ADD = 1, - SUBTRACT = 2, - MULTIPLY = 3, - DIVIDE = 4 -} - -/** - * Structs are the basic complex data structures. They are comprised of fields - * which each have an integer identifier, a type, a symbolic name, and an - * optional default value. - * - * Fields can be declared "optional", which ensures they will not be included - * in the serialized output if they aren't set. Note that this requires some - * manual management in some languages. - */ -struct Work { - 1: i32 num1 = 0, - 2: i32 num2, - 3: Operation op, - 4: optional string comment, -} - -/** - * Structs can also be exceptions, if they are nasty. - */ -exception InvalidOperation { - 1: i32 what, - 2: string why -} - -/** - * Ahh, now onto the cool part, defining a service. Services just need a name - * and can optionally inherit from another service using the extends keyword. - */ -service Calculator extends shared.SharedService { - - /** - * A method definition looks like C code. It has a return type, arguments, - * and optionally a list of exceptions that it may throw. Note that argument - * lists and exception lists are specified using the exact same syntax as - * field lists in struct or exception definitions. - */ - - void ping(), - - i32 add(1:i32 num1, 2:i32 num2), - - i32 calculate(1:i32 logid, 2:Work w) throws (1:InvalidOperation ouch), - - /** - * This method has a oneway modifier. That means the client only makes - * a request and does not listen for any response at all. Oneway methods - * must be void. - */ - oneway void zip() - -} - -/** - * That just about covers the basics. Take a look in the test/ folder for more - * detailed examples. After you run this file, your generated code shows up - * in folders with names gen-. The generated code isn't too scary - * to look at. It even has pretty indentation. - */ diff --git a/tests/parser-cases/thrift/typedefs.thrift b/tests/parser-cases/thrift/typedefs.thrift deleted file mode 100644 index 2a7b237..0000000 --- a/tests/parser-cases/thrift/typedefs.thrift +++ /dev/null @@ -1,6 +0,0 @@ -typedef i32 myInt -typedef string str -typedef byte miniInt -typedef list bytes -typedef set byteSet -typedef map byteMap diff --git a/tests/parser-cases/thrift/unions.thrift b/tests/parser-cases/thrift/unions.thrift deleted file mode 100644 index 00b48d7..0000000 --- a/tests/parser-cases/thrift/unions.thrift +++ /dev/null @@ -1,4 +0,0 @@ -union Sample { - 1: i32 size, - 2: string name, -} diff --git a/tests/test_parser.py b/tests/test_parser.py deleted file mode 100644 index c28b081..0000000 --- a/tests/test_parser.py +++ /dev/null @@ -1,65 +0,0 @@ -# -*- coding: utf-8 -*- - -from __future__ import absolute_import - -import json -import os - -from thriftpy.parser import parse -from thriftpy.parser.exc import ThriftLexerError, ThriftGrammerError - - -def _json(name): - path = os.path.join('parser-cases', 'json', name + '.json') - return json.load(open(path)) - - -def _to_json(value): - if isinstance(value, (set, frozenset)): - return list(sorted(value)) - - assert False - - -def _thrift(name): - path = os.path.join('parser-cases', 'thrift', name + '.thrift') - return json.loads(json.dumps(parse(open(path).read()), default=_to_json)) - - -class TestParser(object): - - def case(name): - def _case(self): - assert _thrift(name) == _json(name) - return _case - - test_includes = case('includes') - test_namespaces = case('namespaces') - test_comments = case('comments') - test_consts = case('consts') - test_typedefs = case('typedefs') - test_enums = case('enums') - test_unions = case('unions') - test_structs = case('structs') - test_exceptions = case('exceptions') - test_serices = case('services') - test_tutorial = case('tutorial') - test_escape = case('escape') - - def test_lexer_exc(self): - try: - _thrift('bad_lexer') - except SyntaxError as e: - assert isinstance(e, ThriftLexerError) - - def test_lexer_bad_escaping_exc(self): - try: - _thrift('bad_lexer_escaping') - except SyntaxError as e: - assert isinstance(e, ThriftLexerError) - - def test_bad_grammer(self): - try: - _thrift('bad_grammer') - except SyntaxError as e: - assert isinstance(e, ThriftGrammerError) From 01f39d0df02a7e759f253baf3f66529ec58501f0 Mon Sep 17 00:00:00 2001 From: hit9 Date: Fri, 23 Jan 2015 17:46:48 +0800 Subject: [PATCH 13/26] Use splitext --- thriftpy/parser/parser.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/thriftpy/parser/parser.py b/thriftpy/parser/parser.py index 60aa006..feff7a7 100644 --- a/thriftpy/parser/parser.py +++ b/thriftpy/parser/parser.py @@ -426,7 +426,8 @@ def parse(path, module_name=None, include_dir=None, '\'_thrift\' suffix') if module_name is None: - module_name = os.path.basename(path)[:-7] + basename = os.path.basename(path) + module_name = os.path.splitext(basename)[0] thrift = types.ModuleType(module_name) thrift_stack.append(thrift) From 8cc624befe47505d99b4d1f1744f8ebc293b6c26 Mon Sep 17 00:00:00 2001 From: hit9 Date: Fri, 23 Jan 2015 18:50:57 +0800 Subject: [PATCH 14/26] Extends services from father --- thriftpy/parser/parser.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/thriftpy/parser/parser.py b/thriftpy/parser/parser.py index feff7a7..a235f5e 100644 --- a/thriftpy/parser/parser.py +++ b/thriftpy/parser/parser.py @@ -662,6 +662,8 @@ def _make_service(name, funcs, extends): gen_init(result_cls, result_cls.thrift_spec, result_cls.default_spec) setattr(cls, result_name, result_cls) thrift_services.append(func_name) + if extends is not None and hasattr(extends, 'thrift_services'): + thrift_services.extend(extends.thrift_services) setattr(cls, 'thrift_services', thrift_services) return cls From 8e0d6210d285e5c84e27c87697fd587bac3a3f78 Mon Sep 17 00:00:00 2001 From: hit9 Date: Fri, 23 Jan 2015 19:20:34 +0800 Subject: [PATCH 15/26] Add more tests cases for PR#80 --- tests/parser-cases/comments.thrift | 9 ++ tests/parser-cases/constants.thrift | 32 ++++++ tests/parser-cases/include.thrift | 3 + tests/parser-cases/included.thrift | 1 + tests/parser-cases/shared.thrift | 39 +++++++ tests/parser-cases/tutorial.thrift | 153 ++++++++++++++++++++++++++++ tests/test_parser.py | 42 ++++++++ 7 files changed, 279 insertions(+) create mode 100644 tests/parser-cases/comments.thrift create mode 100644 tests/parser-cases/constants.thrift create mode 100644 tests/parser-cases/include.thrift create mode 100644 tests/parser-cases/included.thrift create mode 100644 tests/parser-cases/shared.thrift create mode 100644 tests/parser-cases/tutorial.thrift create mode 100644 tests/test_parser.py diff --git a/tests/parser-cases/comments.thrift b/tests/parser-cases/comments.thrift new file mode 100644 index 0000000..9d02e18 --- /dev/null +++ b/tests/parser-cases/comments.thrift @@ -0,0 +1,9 @@ +/* + * C/CPP like comments + */ + +/** + * Silly comments + */ + +# Python like comments diff --git a/tests/parser-cases/constants.thrift b/tests/parser-cases/constants.thrift new file mode 100644 index 0000000..fb8333b --- /dev/null +++ b/tests/parser-cases/constants.thrift @@ -0,0 +1,32 @@ +const i16 int16 = 3 +const i32 int32 = 800 +const i64 int64 = 123456789 +const string tstr = "hello world" +const double tdouble = 1.3 +typedef i32 Integer32 +const Integer32 integer32 = 900 +const list tlist = [1, 2, 3] +const set tset = [1, 2, 3] +const map tmap1 = {"key": "val"} +const map tmap2 = {"key": 32} + +# https://github.com/eleme/thriftpy/pull/69 +enum Country { + US = 1, + UK = 2, + CA = 3, + CN = 4 +} + +const Country my_country = Country.CN; + +struct Person { + 1: string name, + 2: Country country = Country.US +} + +const Person tom = {"name": "tom"} + +# https://github.com/eleme/thriftpy/issues/75 +const map country_map = { + Country.US: "US", Country.UK: "UK", Country.CA: "CA", Country.CN: "CN"} diff --git a/tests/parser-cases/include.thrift b/tests/parser-cases/include.thrift new file mode 100644 index 0000000..14678cf --- /dev/null +++ b/tests/parser-cases/include.thrift @@ -0,0 +1,3 @@ +include "included.thrift" + +const included.Timestamp datetime = 1422009523 diff --git a/tests/parser-cases/included.thrift b/tests/parser-cases/included.thrift new file mode 100644 index 0000000..7a66178 --- /dev/null +++ b/tests/parser-cases/included.thrift @@ -0,0 +1 @@ +typedef i64 Timestamp diff --git a/tests/parser-cases/shared.thrift b/tests/parser-cases/shared.thrift new file mode 100644 index 0000000..60e8e7a --- /dev/null +++ b/tests/parser-cases/shared.thrift @@ -0,0 +1,39 @@ +/* + * 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. + */ + +/** + * This Thrift file can be included by other Thrift files that want to share + * these definitions. + */ + +namespace cpp shared +namespace d share // "shared" would collide with the eponymous D keyword. +namespace java shared +namespace perl shared +namespace php shared +namespace haxe shared + +struct SharedStruct { + 1: i32 key + 2: string value +} + +service SharedService { + SharedStruct getStruct(1: i32 key) +} diff --git a/tests/parser-cases/tutorial.thrift b/tests/parser-cases/tutorial.thrift new file mode 100644 index 0000000..c8710d4 --- /dev/null +++ b/tests/parser-cases/tutorial.thrift @@ -0,0 +1,153 @@ +/* + * 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. + */ + +# Thrift Tutorial +# Mark Slee (mcslee@facebook.com) +# +# This file aims to teach you how to use Thrift, in a .thrift file. Neato. The +# first thing to notice is that .thrift files support standard shell comments. +# This lets you make your thrift file executable and include your Thrift build +# step on the top line. And you can place comments like this anywhere you like. +# +# Before running this file, you will need to have installed the thrift compiler +# into /usr/local/bin. + +/** + * The first thing to know about are types. The available types in Thrift are: + * + * bool Boolean, one byte + * byte Signed byte + * i16 Signed 16-bit integer + * i32 Signed 32-bit integer + * i64 Signed 64-bit integer + * double 64-bit floating point value + * string String + * binary Blob (byte array) + * map Map from one type to another + * list Ordered list of one type + * set Set of unique elements of one type + * + * Did you also notice that Thrift supports C style comments? + */ + +// Just in case you were wondering... yes. We support simple C comments too. + +/** + * Thrift files can reference other Thrift files to include common struct + * and service definitions. These are found using the current path, or by + * searching relative to any paths specified with the -I compiler flag. + * + * Included objects are accessed using the name of the .thrift file as a + * prefix. i.e. shared.SharedObject + */ +include "shared.thrift" + +/** + * Thrift files can namespace, package, or prefix their output in various + * target languages. + */ +namespace cpp tutorial +namespace d tutorial +namespace java tutorial +namespace php tutorial +namespace perl tutorial +namespace haxe tutorial + +/** + * Thrift lets you do typedefs to get pretty names for your types. Standard + * C style here. + */ +typedef i32 MyInteger + +/** + * Thrift also lets you define constants for use across languages. Complex + * types and structs are specified using JSON notation. + */ +const i32 INT32CONSTANT = 9853 +const map MAPCONSTANT = {'hello':'world', 'goodnight':'moon'} + +/** + * You can define enums, which are just 32 bit integers. Values are optional + * and start at 1 if not supplied, C style again. + */ +enum Operation { + ADD = 1, + SUBTRACT = 2, + MULTIPLY = 3, + DIVIDE = 4 +} + +/** + * Structs are the basic complex data structures. They are comprised of fields + * which each have an integer identifier, a type, a symbolic name, and an + * optional default value. + * + * Fields can be declared "optional", which ensures they will not be included + * in the serialized output if they aren't set. Note that this requires some + * manual management in some languages. + */ +struct Work { + 1: i32 num1 = 0, + 2: i32 num2, + 3: Operation op, + 4: optional string comment, +} + +/** + * Structs can also be exceptions, if they are nasty. + */ +exception InvalidOperation { + 1: i32 what, + 2: string why +} + +/** + * Ahh, now onto the cool part, defining a service. Services just need a name + * and can optionally inherit from another service using the extends keyword. + */ +service Calculator extends shared.SharedService { + + /** + * A method definition looks like C code. It has a return type, arguments, + * and optionally a list of exceptions that it may throw. Note that argument + * lists and exception lists are specified using the exact same syntax as + * field lists in struct or exception definitions. + */ + + void ping(), + + i32 add(1:i32 num1, 2:i32 num2), + + i32 calculate(1:i32 logid, 2:Work w) throws (1:InvalidOperation ouch), + + /** + * This method has a oneway modifier. That means the client only makes + * a request and does not listen for any response at all. Oneway methods + * must be void. + */ + oneway void zip() + +} + +/** + * That just about covers the basics. Take a look in the test/ folder for more + * detailed examples. After you run this file, your generated code shows up + * in folders with names gen-. The generated code isn't too scary + * to look at. It even has pretty indentation. + */ diff --git a/tests/test_parser.py b/tests/test_parser.py new file mode 100644 index 0000000..ce9b936 --- /dev/null +++ b/tests/test_parser.py @@ -0,0 +1,42 @@ +# coding=utf8 + +from thriftpy.parser import load + + +def test_comments(): + load('parser-cases/comments.thrift') + + +def test_constants(): + thrift = load('parser-cases/constants.thrift') + assert thrift.int16 == 3 + assert thrift.int32 == 800 + assert thrift.int64 == 123456789 + assert thrift.tstr == 'hello world' + assert thrift.integer32 == 900 + assert thrift.tdouble == 1.3 + assert thrift.tlist == [1, 2, 3] + assert thrift.tset == set([1, 2, 3]) + assert thrift.tmap1 == {'key': 'val'} + assert thrift.tmap2 == {'key': 32} + assert thrift.my_country == 4 + assert thrift.tom == thrift.Person(name='tom') + assert thrift.country_map == {1: 'US', 2: 'UK', 3: 'CA', 4: 'CN'} + + +def test_include(): + thrift = load('parser-cases/include.thrift', include_dir='./parser-cases') + assert thrift.datetime == 1422009523 + + +def test_tutorial(): + thrift = load('parser-cases/tutorial.thrift', include_dir='./parser-cases') + assert thrift.INT32CONSTANT == 9853 + assert thrift.MAPCONSTANT == {'hello': 'world', 'goodnight': 'moon'} + assert thrift.Operation.ADD == 1 and thrift.Operation.SUBTRACT == 2 \ + and thrift.Operation.MULTIPLY == 3 and thrift.Operation.DIVIDE == 4 + work = thrift.Work() + assert work.num1 == 0 and work.num2 is None and work.op is None \ + and work.comment is None + assert set(thrift.Calculator.thrift_services) == set([ + 'ping', 'add', 'calculate', 'zip', 'getStruct']) From b51a5fa99f8a29560d1c9919b20b131bcfa0e429 Mon Sep 17 00:00:00 2001 From: hit9 Date: Sat, 24 Jan 2015 15:46:19 +0800 Subject: [PATCH 16/26] Referenced constant should be a named enum value or another constant --- thriftpy/parser/parser.py | 57 +++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/thriftpy/parser/parser.py b/thriftpy/parser/parser.py index a235f5e..6e29d42 100644 --- a/thriftpy/parser/parser.py +++ b/thriftpy/parser/parser.py @@ -135,19 +135,23 @@ def p_const_map_item(p): def p_const_ref(p): '''const_ref : IDENTIFIER''' - keys = p[1].split('.') - thrift = thrift_stack[-1] + child = thrift_stack[-1] - if len(keys) == 1 and hasattr(thrift, keys[0]): - p[0] = getattr(thrift, keys[0]) - return + for name in p[1].split('.'): + father = child + child = getattr(child, name, None) + if child is None: + raise ThriftParserError('Cann\'t find name %r at line %d' + % (p[1], p.lineno(1))) - if len(keys) == 2 and hasattr(thrift, keys[0]): - enum = getattr(thrift, keys[0]) - if hasattr(enum, keys[1]): - p[0] = getattr(enum, keys[1]) - return - raise ThriftParserError('No enum value or constant found named %r' % p[1]) + if _get_ttype(father) == TType.I32 and child in father._named_values: + # father is enum and child is its named value + p[0] = child + elif _get_ttype(child) is None: + # child is a constant + p[0] = child + else: + raise ThriftParserError('No named enum value or constant found named %r' % p[1]) def p_ttype(p): @@ -209,21 +213,18 @@ def p_service(p): thrift = thrift_stack[-1] if len(p) == 8: - father = thrift - + extends = thrift for name in p[4].split('.'): - child = getattr(father, name, None) - if child is None: + extends = getattr(extends, name, None) + if extends is None: raise ThriftParserError('Can\'t find service %r for ' 'service %r to extend' % (p[4], p[2])) - father = child - if not hasattr(child, 'thrift_services'): + if not hasattr(extends, 'thrift_services'): raise ThriftParserError('Can\'t extends %r, not a service' % p[4]) - extends = child else: extends = None @@ -319,20 +320,18 @@ def p_field_type(p): def p_ref_type(p): '''ref_type : IDENTIFIER''' - father = thrift_stack[-1] + ref_type = thrift_stack[-1] for name in p[1].split('.'): - child = getattr(father, name, None) - if child is None: + ref_type = getattr(ref_type, name, None) + if ref_type is None: raise ThriftParserError('No type found: %r, at line %d' % (p[1], p.lineno(1))) - father = child - - if hasattr(child, '_ttype'): - p[0] = getattr(child, '_ttype'), child + if hasattr(ref_type, '_ttype'): + p[0] = getattr(ref_type, '_ttype'), ref_type else: - p[0] = child + p[0] = ref_type def p_base_type(p): # noqa @@ -673,3 +672,9 @@ def _ttype_spec(ttype, name, required=False): return ttype, name, required else: return ttype[0], name, ttype[1], required + + +def _get_ttype(inst, default_ttype=None): + if hasattr(inst, '__dict__') and '_ttype' in inst.__dict__: + return inst.__dict__['_ttype'] + return default_ttype From 4cfd5af4146b270c141c5143ddcc6d2288faa113 Mon Sep 17 00:00:00 2001 From: hit9 Date: Sat, 24 Jan 2015 15:46:52 +0800 Subject: [PATCH 17/26] Add more test cases --- tests/parser-cases/e_type_error_0.thrift | 1 + tests/parser-cases/e_type_error_1.thrift | 1 + tests/parser-cases/e_type_error_2.thrift | 6 +++++ tests/parser-cases/type_ref.thrift | 6 +++++ tests/parser-cases/type_ref_shared.thrift | 16 ++++++++++++ tests/parser-cases/value_ref.thrift | 6 +++++ tests/parser-cases/value_ref_shared.thrift | 2 ++ tests/test_parser.py | 29 ++++++++++++++++++++++ 8 files changed, 67 insertions(+) create mode 100644 tests/parser-cases/e_type_error_0.thrift create mode 100644 tests/parser-cases/e_type_error_1.thrift create mode 100644 tests/parser-cases/e_type_error_2.thrift create mode 100644 tests/parser-cases/type_ref.thrift create mode 100644 tests/parser-cases/type_ref_shared.thrift create mode 100644 tests/parser-cases/value_ref.thrift create mode 100644 tests/parser-cases/value_ref_shared.thrift diff --git a/tests/parser-cases/e_type_error_0.thrift b/tests/parser-cases/e_type_error_0.thrift new file mode 100644 index 0000000..011a4e1 --- /dev/null +++ b/tests/parser-cases/e_type_error_0.thrift @@ -0,0 +1 @@ +const i32 int32 = 1.7; diff --git a/tests/parser-cases/e_type_error_1.thrift b/tests/parser-cases/e_type_error_1.thrift new file mode 100644 index 0000000..1e0e127 --- /dev/null +++ b/tests/parser-cases/e_type_error_1.thrift @@ -0,0 +1 @@ +const map> dct = {'key': {'key1': '1'}} diff --git a/tests/parser-cases/e_type_error_2.thrift b/tests/parser-cases/e_type_error_2.thrift new file mode 100644 index 0000000..620ab49 --- /dev/null +++ b/tests/parser-cases/e_type_error_2.thrift @@ -0,0 +1,6 @@ +struct Person { + 1: string name, + 2: i32 age, +} + +const Person jack = {'name': 'jack', 'age': 1.8} diff --git a/tests/parser-cases/type_ref.thrift b/tests/parser-cases/type_ref.thrift new file mode 100644 index 0000000..962ab29 --- /dev/null +++ b/tests/parser-cases/type_ref.thrift @@ -0,0 +1,6 @@ +include "type_ref_shared.thrift"; + +const type_ref_shared.Writer jerry = { + 'name': 'jerry', 'age': 26, + 'country': type_ref_shared.Country.US} +const type_ref_shared.Book book = {'writer': jerry, 'name': 'Hello World'} diff --git a/tests/parser-cases/type_ref_shared.thrift b/tests/parser-cases/type_ref_shared.thrift new file mode 100644 index 0000000..4894684 --- /dev/null +++ b/tests/parser-cases/type_ref_shared.thrift @@ -0,0 +1,16 @@ +enum Country { + UK = 0, + US = 1, + CN = 2 +} + +struct Writer { + 1: string name, + 2: i32 age, + 3: Country country, +} + +struct Book { + 1: string name, + 2: Writer writer, +} diff --git a/tests/parser-cases/value_ref.thrift b/tests/parser-cases/value_ref.thrift new file mode 100644 index 0000000..4f1b981 --- /dev/null +++ b/tests/parser-cases/value_ref.thrift @@ -0,0 +1,6 @@ +include "value_ref_shared.thrift"; + +const i32 abc = 899 + +const map> container = value_ref_shared.container; +const list lst = [value_ref_shared.int32, abc, 123]; diff --git a/tests/parser-cases/value_ref_shared.thrift b/tests/parser-cases/value_ref_shared.thrift new file mode 100644 index 0000000..f467197 --- /dev/null +++ b/tests/parser-cases/value_ref_shared.thrift @@ -0,0 +1,2 @@ +const i32 int32 = 39; +const map > container = {'key': [1, 2, 3]}; diff --git a/tests/test_parser.py b/tests/test_parser.py index ce9b936..d96fd1e 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -1,6 +1,7 @@ # coding=utf8 from thriftpy.parser import load +from thriftpy.parser.exc import ThriftParserError def test_comments(): @@ -40,3 +41,31 @@ def test_tutorial(): and work.comment is None assert set(thrift.Calculator.thrift_services) == set([ 'ping', 'add', 'calculate', 'zip', 'getStruct']) + + +def test_e_type_error(): + try: + load('parser-cases/e_type_error_0.thrift') + except ThriftParserError as e: + assert 'Type error' in str(e) + try: + load('parser-cases/e_type_error_1.thrift') + except ThriftParserError as e: + assert 'Type error' in str(e) + + try: + load('parser-cases/e_type_error_2.thrift') + except ThriftParserError as e: + assert 'Type error' in str(e) + + +def test_value_ref(): + thrift = load('parser-cases/value_ref.thrift') + assert thrift.container == {'key': [1, 2, 3]} + assert thrift.lst == [39, 899, 123] + + +def test_type_ref(): + thrift = load('parser-cases/type_ref.thrift') + assert thrift.jerry == thrift.type_ref_shared.Writer( + name='jerry', age=26, country=thrift.type_ref_shared.Country.US) From 8af6f9f776c6190a43d9392cd0905293f3513a7a Mon Sep 17 00:00:00 2001 From: hit9 Date: Sat, 24 Jan 2015 16:03:35 +0800 Subject: [PATCH 18/26] Referenced enum values must be named --- thriftpy/parser/parser.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/thriftpy/parser/parser.py b/thriftpy/parser/parser.py index 6e29d42..fabb8d9 100644 --- a/thriftpy/parser/parser.py +++ b/thriftpy/parser/parser.py @@ -144,9 +144,12 @@ def p_const_ref(p): raise ThriftParserError('Cann\'t find name %r at line %d' % (p[1], p.lineno(1))) - if _get_ttype(father) == TType.I32 and child in father._named_values: + if _get_ttype(father) == TType.I32: # father is enum and child is its named value - p[0] = child + if child in father._named_values: + p[0] = child + else: + raise ThriftParserError('No named enum value found named %r' % p[1]) elif _get_ttype(child) is None: # child is a constant p[0] = child From 4cfb80d339bc211df63bc724873b2e50a2718b93 Mon Sep 17 00:00:00 2001 From: hit9 Date: Sat, 24 Jan 2015 16:26:03 +0800 Subject: [PATCH 19/26] Add tests for enums/structs --- tests/parser-cases/e_value_ref_0.thrift | 1 + tests/parser-cases/e_value_ref_1.thrift | 7 +++ tests/parser-cases/e_value_ref_2.thrift | 5 ++ tests/parser-cases/enums.thrift | 23 +++++++++ tests/parser-cases/structs.thrift | 16 +++++++ tests/test_parser.py | 63 +++++++++++++++++++++++++ 6 files changed, 115 insertions(+) create mode 100644 tests/parser-cases/e_value_ref_0.thrift create mode 100644 tests/parser-cases/e_value_ref_1.thrift create mode 100644 tests/parser-cases/e_value_ref_2.thrift create mode 100644 tests/parser-cases/enums.thrift create mode 100644 tests/parser-cases/structs.thrift diff --git a/tests/parser-cases/e_value_ref_0.thrift b/tests/parser-cases/e_value_ref_0.thrift new file mode 100644 index 0000000..e7e35c7 --- /dev/null +++ b/tests/parser-cases/e_value_ref_0.thrift @@ -0,0 +1 @@ +const i32 dct = ref; diff --git a/tests/parser-cases/e_value_ref_1.thrift b/tests/parser-cases/e_value_ref_1.thrift new file mode 100644 index 0000000..77e2fb7 --- /dev/null +++ b/tests/parser-cases/e_value_ref_1.thrift @@ -0,0 +1,7 @@ +enum Lang { + CPP = 0, + Python, + Ruby = 2, +} + +const i32 my_lang = Lang.Python; diff --git a/tests/parser-cases/e_value_ref_2.thrift b/tests/parser-cases/e_value_ref_2.thrift new file mode 100644 index 0000000..2c02755 --- /dev/null +++ b/tests/parser-cases/e_value_ref_2.thrift @@ -0,0 +1,5 @@ +struct Cookbook { + 1: string name +} + +const Cookbook cookbook = Cookbook diff --git a/tests/parser-cases/enums.thrift b/tests/parser-cases/enums.thrift new file mode 100644 index 0000000..5d5a803 --- /dev/null +++ b/tests/parser-cases/enums.thrift @@ -0,0 +1,23 @@ +enum Lang { + C, + Go, + Java, + Javascript, + PHP, + Python, + Ruby, +} + + +enum Country { + US = 1, + UK, + CN, +} + + +enum OS { + OSX, + Win = 3, + Linux +} diff --git a/tests/parser-cases/structs.thrift b/tests/parser-cases/structs.thrift new file mode 100644 index 0000000..6c92a97 --- /dev/null +++ b/tests/parser-cases/structs.thrift @@ -0,0 +1,16 @@ +struct Person { + 1: string name, + 2: string address +} + +struct Email { + 1: string subject = 'Subject', + 2: string content, + 3: Person sender, + 4: required Person recver, +} + +const Email email = {'subject': 'Hello', 'content': 'Long time no see', + 'sender': {'name': 'jack', 'address': 'jack@gmail.com'}, + 'recver': {'name': 'chao', 'address': 'chao@gmail.com'} +} diff --git a/tests/test_parser.py b/tests/test_parser.py index d96fd1e..2694176 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -1,5 +1,6 @@ # coding=utf8 +from thriftpy.thrift import TType from thriftpy.parser import load from thriftpy.parser.exc import ThriftParserError @@ -69,3 +70,65 @@ def test_type_ref(): thrift = load('parser-cases/type_ref.thrift') assert thrift.jerry == thrift.type_ref_shared.Writer( name='jerry', age=26, country=thrift.type_ref_shared.Country.US) + assert thrift.book == thrift.type_ref_shared.Book(name='Hello World', + writer=thrift.jerry) + + +def test_e_value_ref(): + try: + load('parser-cases/e_value_ref_0.thrift') + except ThriftParserError as e: + pass + try: + t = load('parser-cases/e_value_ref_1.thrift') + except ThriftParserError as e: + assert str(e) == 'No named enum value found named \'Lang.Python\'' + + try: + load('parser-cases/e_value_ref_2.thrift') + except ThriftParserError as e: + assert str(e) == 'No named enum value or constant found named \'Cookbook\'' + + +def test_enum(): + thrift = load('parser-cases/enums.thrift') + assert thrift.Lang.C == 0 + assert thrift.Lang.Go == 1 + assert thrift.Lang.Java == 2 + assert thrift.Lang.Javascript == 3 + assert thrift.Lang.PHP == 4 + assert thrift.Lang.Python == 5 + assert thrift.Lang.Ruby == 6 + assert thrift.Country.US == 1 + assert thrift.Country.UK == 2 + assert thrift.Country.CN == 3 + assert thrift.Country._named_values == set([1]) + assert thrift.OS.OSX == 0 + assert thrift.OS.Win == 3 + assert thrift.OS.Linux == 4 + assert thrift.OS._named_values == set([3]) + + +def test_struct(): + thrift = load('parser-cases/structs.thrift') + assert thrift.Person.thrift_spec == { + 1: (TType.STRING, 'name', False), + 2: (TType.STRING, 'address', False) + } + assert thrift.Person.default_spec == [ + ('name', None), ('address', None) + ] + assert thrift.Email.thrift_spec == { + 1: (TType.STRING, 'subject', False), + 2: (TType.STRING, 'content', False), + 3: (TType.STRUCT, 'sender', thrift.Person, False), + 4: (TType.STRUCT, 'recver', thrift.Person, True), + } + assert thrift.Email.default_spec == [ + ('subject', 'Subject'), ('content', None), + ('sender', None), ('recver', None) + ] + assert thrift.email == thrift.Email(subject='Hello', content='Long time no see', + sender=thrift.Person(name='jack', address='jack@gmail.com'), + recver=thrift.Person(name='chao', address='chao@gmail.com') + ) From 022ccdf72daa19810ae3cdb304fc65ffeacc86a0 Mon Sep 17 00:00:00 2001 From: hit9 Date: Sat, 24 Jan 2015 16:57:40 +0800 Subject: [PATCH 20/26] Tests case for structs/services and flakes8 fixes --- tests/parser-cases/e_structs_0.thrift | 13 +++++ tests/parser-cases/e_structs_1.thrift | 7 +++ tests/parser-cases/service.thrift | 21 ++++++++ tests/parser-cases/service_extends.thrift | 6 +++ tests/test_parser.py | 58 +++++++++++++++++++++-- thriftpy/parser/parser.py | 6 ++- 6 files changed, 105 insertions(+), 6 deletions(-) create mode 100644 tests/parser-cases/e_structs_0.thrift create mode 100644 tests/parser-cases/e_structs_1.thrift create mode 100644 tests/parser-cases/service.thrift create mode 100644 tests/parser-cases/service_extends.thrift diff --git a/tests/parser-cases/e_structs_0.thrift b/tests/parser-cases/e_structs_0.thrift new file mode 100644 index 0000000..d5a1579 --- /dev/null +++ b/tests/parser-cases/e_structs_0.thrift @@ -0,0 +1,13 @@ +struct User { + 1: required string name, + 2: optional string avatar +} + +struct Post { + 1: required string title, + 2: required string content, + 3: required User user, +} + +const Post post = {'title': 'hello world', 'content': 'hello', + 'user': {}} diff --git a/tests/parser-cases/e_structs_1.thrift b/tests/parser-cases/e_structs_1.thrift new file mode 100644 index 0000000..819f5d8 --- /dev/null +++ b/tests/parser-cases/e_structs_1.thrift @@ -0,0 +1,7 @@ +struct User { + 1: string name, + 2: i32 age, + 3: string email +} + +const User user = {'avatar': '/avatar.jpg'} diff --git a/tests/parser-cases/service.thrift b/tests/parser-cases/service.thrift new file mode 100644 index 0000000..af4e66b --- /dev/null +++ b/tests/parser-cases/service.thrift @@ -0,0 +1,21 @@ +struct User { + 1: string name, + 2: string address, +} + +struct Email { + 1: string subject, + 2: string content, +} + +exception NetworkError { + 1: string message, + 2: i32 http_code +} + +service EmailService { + void ping () + throws (1: NetworkError network_error) + bool send(1: User recver, 2: User sender, 3: Email email) + throws (1: NetworkError network_error) +} diff --git a/tests/parser-cases/service_extends.thrift b/tests/parser-cases/service_extends.thrift new file mode 100644 index 0000000..39e9e5a --- /dev/null +++ b/tests/parser-cases/service_extends.thrift @@ -0,0 +1,6 @@ +include "shared.thrift" + + +service PingService extends shared.SharedService { + string ping() +} diff --git a/tests/test_parser.py b/tests/test_parser.py index 2694176..9043472 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -87,10 +87,11 @@ def test_e_value_ref(): try: load('parser-cases/e_value_ref_2.thrift') except ThriftParserError as e: - assert str(e) == 'No named enum value or constant found named \'Cookbook\'' + assert str(e) == \ + 'No named enum value or constant found named \'Cookbook\'' -def test_enum(): +def test_enums(): thrift = load('parser-cases/enums.thrift') assert thrift.Lang.C == 0 assert thrift.Lang.Go == 1 @@ -109,7 +110,7 @@ def test_enum(): assert thrift.OS._named_values == set([3]) -def test_struct(): +def test_structs(): thrift = load('parser-cases/structs.thrift') assert thrift.Person.thrift_spec == { 1: (TType.STRING, 'name', False), @@ -128,7 +129,56 @@ def test_struct(): ('subject', 'Subject'), ('content', None), ('sender', None), ('recver', None) ] - assert thrift.email == thrift.Email(subject='Hello', content='Long time no see', + assert thrift.email == thrift.Email( + subject='Hello', + content='Long time no see', sender=thrift.Person(name='jack', address='jack@gmail.com'), recver=thrift.Person(name='chao', address='chao@gmail.com') ) + + +def test_e_structs(): + try: + load('parser-cases/e_structs_0.thrift') + except ThriftParserError as e: + assert str(e) == \ + 'Field \'name\' was required to create constant for type \'User\'' + + try: + load('parser-cases/e_structs_1.thrift') + except ThriftParserError as e: + assert str(e) == \ + 'No field named \'avatar\' was found in struct of type \'User\'' + + +def test_service(): + thrift = load('parser-cases/service.thrift') + assert thrift.EmailService.thrift_services == ['ping', 'send'] + assert thrift.EmailService.ping_args.thrift_spec == {} + assert thrift.EmailService.ping_args.default_spec == [] + assert thrift.EmailService.ping_result.thrift_spec == { + 1: (TType.STRUCT, 'network_error', thrift.NetworkError, False) + } + assert thrift.EmailService.ping_result.default_spec == [ + ('network_error', None) + ] + assert thrift.EmailService.send_args.thrift_spec == { + 1: (TType.STRUCT, 'recver', thrift.User, False), + 2: (TType.STRUCT, 'sender', thrift.User, False), + 3: (TType.STRUCT, 'email', thrift.Email, False), + } + assert thrift.EmailService.send_args.default_spec == [ + ('recver', None), ('sender', None), ('email', None) + ] + assert thrift.EmailService.send_result.thrift_spec == { + 0: (TType.BOOL, 'success', False), + 1: (TType.STRUCT, 'network_error', thrift.NetworkError, False) + } + assert thrift.EmailService.send_result.default_spec == [ + ('success', None), ('network_error', None) + ] + + +def test_service_extends(): + thrift = load('parser-cases/service_extends.thrift') + assert thrift.PingService.thrift_services == ['ping', 'getStruct'] diff --git a/thriftpy/parser/parser.py b/thriftpy/parser/parser.py index fabb8d9..593ac8d 100644 --- a/thriftpy/parser/parser.py +++ b/thriftpy/parser/parser.py @@ -149,12 +149,14 @@ def p_const_ref(p): if child in father._named_values: p[0] = child else: - raise ThriftParserError('No named enum value found named %r' % p[1]) + raise ThriftParserError('No named enum value found named %r' + % p[1]) elif _get_ttype(child) is None: # child is a constant p[0] = child else: - raise ThriftParserError('No named enum value or constant found named %r' % p[1]) + raise ThriftParserError('No named enum value or constant found ' + 'named %r' % p[1]) def p_ttype(p): From 39a62af0751ac2ef6d818d6876899ee76b1502b5 Mon Sep 17 00:00:00 2001 From: hit9 Date: Sat, 24 Jan 2015 17:01:53 +0800 Subject: [PATCH 21/26] Add test-case for e-service-extends --- tests/parser-cases/e_service_extends_0.thrift | 5 +++++ tests/test_parser.py | 7 +++++++ 2 files changed, 12 insertions(+) create mode 100644 tests/parser-cases/e_service_extends_0.thrift diff --git a/tests/parser-cases/e_service_extends_0.thrift b/tests/parser-cases/e_service_extends_0.thrift new file mode 100644 index 0000000..f1c59ea --- /dev/null +++ b/tests/parser-cases/e_service_extends_0.thrift @@ -0,0 +1,5 @@ +include "shared.thrift" + +service PingService extends shared.NotExistService { + +} diff --git a/tests/test_parser.py b/tests/test_parser.py index 9043472..1f1a208 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -182,3 +182,10 @@ def test_service(): def test_service_extends(): thrift = load('parser-cases/service_extends.thrift') assert thrift.PingService.thrift_services == ['ping', 'getStruct'] + + +def test_e_service_extends(): + try: + load('parser-cases/e_service_extends_0.thrift') + except ThriftParserError as e: + assert 'Can\'t find service' in str(e) From 039cf262e42d4dde0292ad8fb3324412116603b1 Mon Sep 17 00:00:00 2001 From: hit9 Date: Sat, 24 Jan 2015 17:05:09 +0800 Subject: [PATCH 22/26] Flake8 fixed --- tests/test_parser.py | 2 +- thriftpy/parser/__init__.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_parser.py b/tests/test_parser.py index 1f1a208..d53c144 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -80,7 +80,7 @@ def test_e_value_ref(): except ThriftParserError as e: pass try: - t = load('parser-cases/e_value_ref_1.thrift') + load('parser-cases/e_value_ref_1.thrift') except ThriftParserError as e: assert str(e) == 'No named enum value found named \'Lang.Python\'' diff --git a/thriftpy/parser/__init__.py b/thriftpy/parser/__init__.py index 4ee1c68..ea556bf 100644 --- a/thriftpy/parser/__init__.py +++ b/thriftpy/parser/__init__.py @@ -8,6 +8,7 @@ """ from __future__ import absolute_import +import os import sys from .parser import parse From f9f921636b2c8a2e0b6cbe3257918e48dc854449 Mon Sep 17 00:00:00 2001 From: hit9 Date: Sat, 24 Jan 2015 17:21:52 +0800 Subject: [PATCH 23/26] Add names-to-values and values-to-names for enums --- thriftpy/parser/parser.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/thriftpy/parser/parser.py b/thriftpy/parser/parser.py index 593ac8d..1826bec 100644 --- a/thriftpy/parser/parser.py +++ b/thriftpy/parser/parser.py @@ -603,6 +603,9 @@ def _make_enum(name, kvs): named_values.add(val) setattr(cls, '_named_values', named_values) + _values_to_names = {} + _names_to_values = {} + if kvs: val = kvs[0][1] if val is None: @@ -613,6 +616,10 @@ def _make_enum(name, kvs): val = item[1] for key, val in kvs: setattr(cls, key, val) + _values_to_names[val] = key + _names_to_values[key] = val + setattr(cls, '_VALUES_TO_NAMES', _values_to_names) + setattr(cls, '_NAMES_TO_VALUES', _names_to_values) return cls From 6dc0a26105bb70812457b9dad4b6c7c6f774f743 Mon Sep 17 00:00:00 2001 From: hit9 Date: Sat, 24 Jan 2015 18:43:07 +0800 Subject: [PATCH 24/26] Cyclig including (dead including) checking --- thriftpy/parser/parser.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/thriftpy/parser/parser.py b/thriftpy/parser/parser.py index 1826bec..630785c 100644 --- a/thriftpy/parser/parser.py +++ b/thriftpy/parser/parser.py @@ -402,6 +402,11 @@ def p_definition_type(p): def parse(path, module_name=None, include_dir=None, lexer=None, parser=None, enable_cache=True): + # dead include checking on current stack + for thrift in thrift_stack: + if os.path.samefile(path, thrift.__thrift_file__): + raise ThriftParserError('Dead including on %s' % path) + global thrift_cache cache_key = module_name or os.path.normpath(path) @@ -434,6 +439,7 @@ def parse(path, module_name=None, include_dir=None, module_name = os.path.splitext(basename)[0] thrift = types.ModuleType(module_name) + setattr(thrift, '__thrift_file__', path) thrift_stack.append(thrift) lexer.lineno = 1 parser.parse(data) From a77d6fe03bcbd847516e71dbac0d04b173cea58d Mon Sep 17 00:00:00 2001 From: hit9 Date: Sat, 24 Jan 2015 18:47:40 +0800 Subject: [PATCH 25/26] Add test case for dead include --- tests/parser-cases/e_dead_include_0.thrift | 1 + tests/parser-cases/e_dead_include_1.thrift | 1 + tests/parser-cases/e_dead_include_2.thrift | 1 + tests/parser-cases/e_dead_include_3.thrift | 1 + tests/test_parser.py | 7 +++++++ 5 files changed, 11 insertions(+) create mode 100644 tests/parser-cases/e_dead_include_0.thrift create mode 100644 tests/parser-cases/e_dead_include_1.thrift create mode 100644 tests/parser-cases/e_dead_include_2.thrift create mode 100644 tests/parser-cases/e_dead_include_3.thrift diff --git a/tests/parser-cases/e_dead_include_0.thrift b/tests/parser-cases/e_dead_include_0.thrift new file mode 100644 index 0000000..dfe45dd --- /dev/null +++ b/tests/parser-cases/e_dead_include_0.thrift @@ -0,0 +1 @@ +include "e_dead_include_1.thrift" diff --git a/tests/parser-cases/e_dead_include_1.thrift b/tests/parser-cases/e_dead_include_1.thrift new file mode 100644 index 0000000..2e358cc --- /dev/null +++ b/tests/parser-cases/e_dead_include_1.thrift @@ -0,0 +1 @@ +include "e_dead_include_2.thrift" diff --git a/tests/parser-cases/e_dead_include_2.thrift b/tests/parser-cases/e_dead_include_2.thrift new file mode 100644 index 0000000..85e51ff --- /dev/null +++ b/tests/parser-cases/e_dead_include_2.thrift @@ -0,0 +1 @@ +include "e_dead_include_3.thrift" diff --git a/tests/parser-cases/e_dead_include_3.thrift b/tests/parser-cases/e_dead_include_3.thrift new file mode 100644 index 0000000..dfe45dd --- /dev/null +++ b/tests/parser-cases/e_dead_include_3.thrift @@ -0,0 +1 @@ +include "e_dead_include_1.thrift" diff --git a/tests/test_parser.py b/tests/test_parser.py index d53c144..193fadd 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -189,3 +189,10 @@ def test_e_service_extends(): load('parser-cases/e_service_extends_0.thrift') except ThriftParserError as e: assert 'Can\'t find service' in str(e) + + +def test_e_dead_include(): + try: + load('parser-cases/e_dead_include_0.thrift') + except ThriftParserError as e: + assert 'Dead including' in str(e) From 01a20109f6cbdd74618a118a22713916c45cd0b7 Mon Sep 17 00:00:00 2001 From: Travis Mehlinger Date: Mon, 9 Feb 2015 10:55:08 -0600 Subject: [PATCH 26/26] better assert uniqueness by catching processor module name --- thriftpy/thrift.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/thriftpy/thrift.py b/thriftpy/thrift.py index 4daeef4..da46427 100644 --- a/thriftpy/thrift.py +++ b/thriftpy/thrift.py @@ -10,6 +10,7 @@ from __future__ import absolute_import import functools +import inspect from ._compat import init_func_generator, with_metaclass @@ -252,7 +253,8 @@ def __init__(self): def register_processor(self, processor): service = processor._service - name = service.__name__ + module = inspect.getmodule(processor) + name = '{0}:{1}'.format(module.__name__, service.__name__) if name in self.processors: raise TApplicationException( type=TApplicationException.INTERNAL_ERROR,