Merge pull request #488 from haberman/python-findmethod

Fixed Python test by implementing FindMethodByName

python/descriptor.h

@@ -60,6 +60,7 @@ PyObject* PyUpb_OneofDescriptor_Get(const upb_OneofDef* oneof);
 PyObject* PyUpb_EnumValueDescriptor_Get(const upb_EnumValueDef* enumval);
 PyObject* PyUpb_Descriptor_GetOrCreateWrapper(const upb_MessageDef* msg);
 PyObject* PyUpb_ServiceDescriptor_Get(const upb_ServiceDef* s);
+PyObject* PyUpb_MethodDescriptor_Get(const upb_MethodDef* s);
 
 // Returns the underlying |def| for a given wrapper object. The caller must
 // have already verified that the given Python object is of the expected type.

python/descriptor_pool.c

@@ -484,13 +484,43 @@ static PyObject* PyUpb_DescriptorPool_FindServiceByName(PyObject* _self,
   if (!name) return NULL;
 
   const upb_ServiceDef* s = upb_DefPool_FindServiceByName(self->symtab, name);
+  if (s == NULL && self->db) {
+    if (!PyUpb_DescriptorPool_TryLoadSymbol(self, arg)) return NULL;
+    s = upb_DefPool_FindServiceByName(self->symtab, name);
+  }
   if (s == NULL) {
-    return PyErr_Format(PyExc_KeyError, "Couldn't find enum %.200s", name);
+    return PyErr_Format(PyExc_KeyError, "Couldn't find service %.200s", name);
   }
 
   return PyUpb_ServiceDescriptor_Get(s);
 }
 
+static PyObject* PyUpb_DescriptorPool_FindMethodByName(PyObject* _self,
+                                                       PyObject* arg) {
+  PyUpb_DescriptorPool* self = (PyUpb_DescriptorPool*)_self;
+
+  const char* name = PyUpb_GetStrData(arg);
+  if (!name) return NULL;
+  size_t parent_size;
+  const char* child = PyUpb_DescriptorPool_SplitSymbolName(name, &parent_size);
+
+  if (!child) goto err;
+  const upb_ServiceDef* parent =
+      upb_DefPool_FindServiceByNameWithSize(self->symtab, name, parent_size);
+  if (parent == NULL && self->db) {
+    if (!PyUpb_DescriptorPool_TryLoadSymbol(self, arg)) return NULL;
+    parent = upb_DefPool_FindServiceByNameWithSize(self->symtab, name,
+                                                    parent_size);
+  }
+  if (!parent) goto err;
+  const upb_MethodDef* m = upb_ServiceDef_FindMethodByName(parent, child);
+  if (!m) goto err;
+  return PyUpb_MethodDescriptor_Get(m);
+
+err:
+  return PyErr_Format(PyExc_KeyError, "Couldn't find method %.200s", name);
+}
+
 static PyObject* PyUpb_DescriptorPool_FindFileContainingSymbol(PyObject* _self,
                                                                PyObject* arg) {
   PyUpb_DescriptorPool* self = (PyUpb_DescriptorPool*)_self;
@@ -498,10 +528,11 @@ static PyObject* PyUpb_DescriptorPool_FindFileContainingSymbol(PyObject* _self,
   const char* name = PyUpb_GetStrData(arg);
   if (!name) return NULL;
 
-  const upb_FileDef* f = upb_DefPool_FindFileByNameforsym(self->symtab, name);
+  const upb_FileDef* f =
+      upb_DefPool_FindFileContainingSymbol(self->symtab, name);
   if (f == NULL && self->db) {
     if (!PyUpb_DescriptorPool_TryLoadSymbol(self, arg)) return NULL;
-    f = upb_DefPool_FindFileByNameforsym(self->symtab, name);
+    f = upb_DefPool_FindFileContainingSymbol(self->symtab, name);
   }
   if (f == NULL) {
     return PyErr_Format(PyExc_KeyError, "Couldn't find symbol %.200s", name);
@@ -562,8 +593,8 @@ static PyMethodDef PyUpb_DescriptorPool_Methods[] = {
      "Searches for oneof descriptor by full name."},
     {"FindServiceByName", PyUpb_DescriptorPool_FindServiceByName, METH_O,
      "Searches for service descriptor by full name."},
-    //{ "Find, PyUpb_DescriptorPool_Find, METH_O,
-    //  "Searches for method descriptor by full name." },
+    {"FindMethodByName", PyUpb_DescriptorPool_FindMethodByName, METH_O,
+     "Searches for method descriptor by full name."},
     {"FindFileContainingSymbol", PyUpb_DescriptorPool_FindFileContainingSymbol,
      METH_O, "Gets the FileDescriptor containing the specified symbol."},
     {"FindExtensionByNumber", PyUpb_DescriptorPool_FindExtensionByNumber,

python/pb_unit_tests/descriptor_pool_test_wrapper.py

@@ -33,7 +33,6 @@ descriptor_pool_test.AddDescriptorTest.testFile.__unittest_expecting_failure__ =
 descriptor_pool_test.AddDescriptorTest.testMessage.__unittest_expecting_failure__ = True
 descriptor_pool_test.AddDescriptorTest.testService.__unittest_expecting_failure__ = True
 descriptor_pool_test.CreateDescriptorPoolTest.testFindFieldByName.__unittest_expecting_failure__ = True
-descriptor_pool_test.CreateDescriptorPoolTest.testFindService.__unittest_expecting_failure__ = True
 descriptor_pool_test.CreateDescriptorPoolTest.testFindTypeErrors.__unittest_expecting_failure__ = True
 descriptor_pool_test.SecondaryDescriptorFromDescriptorDB.testErrorCollector.__unittest_expecting_failure__ = True
 

upb/def.c

@@ -1159,8 +1159,13 @@ const upb_ServiceDef* upb_DefPool_FindServiceByName(const upb_DefPool* s,
   return symtab_lookup(s, name, UPB_DEFTYPE_SERVICE);
 }
 
-const upb_FileDef* upb_DefPool_FindFileByNameforsym(const upb_DefPool* s,
-                                                    const char* name) {
+const upb_ServiceDef* upb_DefPool_FindServiceByNameWithSize(
+    const upb_DefPool* s, const char* name, size_t size) {
+  return symtab_lookup2(s, name, size, UPB_DEFTYPE_SERVICE);
+}
+
+const upb_FileDef* upb_DefPool_FindFileContainingSymbol(const upb_DefPool* s,
+                                                        const char* name) {
   upb_value v;
   // TODO(haberman): non-extension fields and oneofs.
   if (upb_strtable_lookup(&s->syms, name, &v)) {

upb/def.h

@@ -358,8 +358,10 @@ const upb_FileDef* upb_DefPool_FindFileByName(const upb_DefPool* s,
                                               const char* name);
 const upb_ServiceDef* upb_DefPool_FindServiceByName(const upb_DefPool* s,
                                                     const char* name);
-const upb_FileDef* upb_DefPool_FindFileByNameforsym(const upb_DefPool* s,
-                                                    const char* name);
+const upb_ServiceDef* upb_DefPool_FindServiceByNameWithSize(
+    const upb_DefPool* s, const char* name, size_t size);
+const upb_FileDef* upb_DefPool_FindFileContainingSymbol(const upb_DefPool* s,
+                                                        const char* name);
 const upb_FileDef* upb_DefPool_FindFileByNameWithSize(const upb_DefPool* s,
                                                       const char* name,
                                                       size_t len);