Skip to content

Commit

Permalink
Enable new JSON field name conflict handling.
Browse files Browse the repository at this point in the history
This will apply uniformly in both proto2 and proto3, taking into account `json_name` options.  See #10750 for more details.

PiperOrigin-RevId: 496005994
  • Loading branch information
mkruskal-google authored and copybara-github committed Jan 18, 2023
1 parent 324f0b5 commit e1f46f7
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,9 @@ service TestConflictingMethodNames {
}

message TestConflictingFieldNames {
// TODO(b/261750190) Remove these tests once this behavior is removed.
option deprecated_legacy_json_field_conflicts = true;

enum TestEnum {
UNKNOWN = 0;
FOO = 1;
Expand Down
24 changes: 20 additions & 4 deletions python/google/protobuf/descriptor_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,25 @@ class DescriptorPool(object):

if _USE_C_DESCRIPTORS:

def __new__(cls, descriptor_db=None):
# pylint: disable=protected-access
return descriptor._message.DescriptorPool(descriptor_db)
def __new__(
cls,
descriptor_db=None,
use_deprecated_legacy_json_field_conflicts=False,
):
if descriptor.api_implementation.Type() == 'cpp':
# TODO(b/261750190) Remove this once ranklab no longer needs it.
# pylint: disable=protected-access
return descriptor._message.DescriptorPool(
descriptor_db,
use_deprecated_legacy_json_field_conflicts=use_deprecated_legacy_json_field_conflicts,
)
else:
# pylint: disable=protected-access
return descriptor._message.DescriptorPool(descriptor_db)

def __init__(self, descriptor_db=None):
def __init__(
self, descriptor_db=None, use_deprecated_legacy_json_field_conflicts=False
):
"""Initializes a Pool of proto buffs.
The descriptor_db argument to the constructor is provided to allow
Expand All @@ -135,6 +149,8 @@ def __init__(self, descriptor_db=None):
Args:
descriptor_db: A secondary source of file descriptors.
use_deprecated_legacy_json_field_conflicts: Unused, for compatibility with
C++.
"""

self._internal_db = descriptor_database.DescriptorDatabase()
Expand Down
2 changes: 2 additions & 0 deletions python/google/protobuf/internal/descriptor_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1186,6 +1186,7 @@ def testMakeDescriptorWithOptions(self):

def testCamelcaseName(self):
descriptor_proto = descriptor_pb2.DescriptorProto()
descriptor_proto.options.deprecated_legacy_json_field_conflicts = True
descriptor_proto.name = 'Bar'
names = ['foo_foo', 'FooBar', 'fooBaz', 'fooFoo', 'foobar']
camelcase_names = ['fooFoo', 'fooBar', 'fooBaz', 'fooFoo', 'foobar']
Expand All @@ -1200,6 +1201,7 @@ def testCamelcaseName(self):

def testJsonName(self):
descriptor_proto = descriptor_pb2.DescriptorProto()
descriptor_proto.options.deprecated_legacy_json_field_conflicts = True
descriptor_proto.name = 'TestJsonName'
names = ['field_name', 'fieldName', 'FieldName',
'_field_name', 'FIELD_NAME', 'json_name']
Expand Down
25 changes: 17 additions & 8 deletions python/google/protobuf/pyext/descriptor_pool.cc
Original file line number Diff line number Diff line change
Expand Up @@ -155,20 +155,26 @@ static PyDescriptorPool* PyDescriptorPool_NewWithUnderlay(
}

static PyDescriptorPool* PyDescriptorPool_NewWithDatabase(
DescriptorDatabase* database) {
DescriptorDatabase* database,
bool use_deprecated_legacy_json_field_conflicts) {
PyDescriptorPool* cpool = _CreateDescriptorPool();
if (cpool == nullptr) {
return nullptr;
}
DescriptorPool* pool;
if (database != nullptr) {
cpool->error_collector = new BuildFileErrorCollector();
cpool->pool = new DescriptorPool(database, cpool->error_collector);
pool = new DescriptorPool(database, cpool->error_collector);
cpool->is_mutable = false;
cpool->database = database;
} else {
cpool->pool = new DescriptorPool();
pool = new DescriptorPool();
cpool->is_mutable = true;
}
if (use_deprecated_legacy_json_field_conflicts) {
pool->UseDeprecatedLegacyJsonFieldConflicts();
}
cpool->pool = pool;
cpool->is_owned = true;

if (!descriptor_pool_map->insert(std::make_pair(cpool->pool, cpool)).second) {
Expand All @@ -183,18 +189,21 @@ static PyDescriptorPool* PyDescriptorPool_NewWithDatabase(
// The public DescriptorPool constructor.
static PyObject* New(PyTypeObject* type,
PyObject* args, PyObject* kwargs) {
static const char* kwlist[] = {"descriptor_db", nullptr};
static const char* kwlist[] = {
"descriptor_db", "use_deprecated_legacy_json_field_conflicts", nullptr};
PyObject* py_database = nullptr;
if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|O",
const_cast<char**>(kwlist), &py_database)) {
int use_deprecated_legacy_json_field_conflicts = 0;
if (!PyArg_ParseTupleAndKeywords(
args, kwargs, "|O$i", const_cast<char**>(kwlist), &py_database,
&use_deprecated_legacy_json_field_conflicts)) {
return nullptr;
}
DescriptorDatabase* database = nullptr;
if (py_database && py_database != Py_None) {
database = new PyDescriptorDatabase(py_database);
}
return reinterpret_cast<PyObject*>(
PyDescriptorPool_NewWithDatabase(database));
return reinterpret_cast<PyObject*>(PyDescriptorPool_NewWithDatabase(
database, use_deprecated_legacy_json_field_conflicts));
}

static void Dealloc(PyObject* pself) {
Expand Down
12 changes: 2 additions & 10 deletions src/google/protobuf/descriptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5617,16 +5617,8 @@ void DescriptorBuilder::CheckFieldJsonNameUniqueness(
this_type, field.name(), details.orig_name, existing_type,
match.field->name(), name_suffix);

bool involves_default = !details.is_custom || !match.is_custom;
if (syntax == FileDescriptor::SYNTAX_PROTO2 && involves_default) {
// TODO(b/261750676) Upgrade this to an error once downstream proto2 files
// have been fixed.
AddWarning(message_name, field, DescriptorPool::ErrorCollector::NAME,
error_message);
} else {
AddError(message_name, field, DescriptorPool::ErrorCollector::NAME,
error_message);
}
AddError(message_name, field, DescriptorPool::ErrorCollector::NAME,
error_message);
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/google/protobuf/descriptor_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1277,6 +1277,8 @@ class StylizedFieldNamesTest : public testing::Test {
AddExtensionRange(AddMessage(&file, "ExtendableMessage"), 1, 1000);

DescriptorProto* message = AddMessage(&file, "TestMessage");
message->mutable_options()->set_deprecated_legacy_json_field_conflicts(
true);
AddField(message, "foo_foo", 1, FieldDescriptorProto::LABEL_OPTIONAL,
FieldDescriptorProto::TYPE_INT32);
AddField(message, "FooBar", 2, FieldDescriptorProto::LABEL_OPTIONAL,
Expand Down Expand Up @@ -7012,7 +7014,7 @@ TEST_F(ValidationErrorTest, ValidateJsonNameConflictProto3) {
}

TEST_F(ValidationErrorTest, ValidateJsonNameConflictProto2) {
BuildFileWithWarnings(
BuildFileWithErrors(
"name: 'foo.proto' "
"syntax: 'proto2' "
"message_type {"
Expand Down

0 comments on commit e1f46f7

Please sign in to comment.