Skip to content

Commit

Permalink
Add missing test to verify that LookupMapValue sync's the repeated fi…
Browse files Browse the repository at this point in the history
…eld data

before doing the lookup in the map.

PiperOrigin-RevId: 704714248
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Dec 10, 2024
1 parent 7519359 commit 874e03b
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 6 deletions.
39 changes: 39 additions & 0 deletions src/google/protobuf/map_test.inc
Original file line number Diff line number Diff line change
Expand Up @@ -1387,6 +1387,45 @@ TEST_P(MapTestWithParams, SwapMapsFieldsViaReflection) {
reflection_tester.ExpectMapFieldsSetViaReflectionIterator(rhs);
}

class MapTestCodegenOrDynamic : public testing::TestWithParam<bool> {};

INSTANTIATE_TEST_SUITE_P(MapTestCodegenOrDynamicSuite, MapTestCodegenOrDynamic,
testing::Bool());

TEST_P(MapTestCodegenOrDynamic, LookupMapValueSyncsRepeatedFieldRep) {
auto* descriptor = UNITTEST::TestMap::descriptor();
bool use_dynamic = GetParam();

DynamicMessageFactory factory;
std::unique_ptr<Message> dyn_message(
factory.GetPrototype(descriptor)->New());
TestMap gen_message;
Message* msg = use_dynamic ? dyn_message.get() : &gen_message;

const auto* reflection = msg->GetReflection();
const auto* field = msg->GetDescriptor()->FindFieldByName("map_int32_int32");

MapKey map_key;
map_key.SetInt32Value(10);
auto res =
MapReflectionTester::LookupMapValue(*reflection, *msg, *field, map_key);
// Check that we don't have it yet.
ASSERT_FALSE(res.has_value());

{
auto* sub = reflection->AddMessage(msg, field);
sub->GetReflection()->SetInt32(sub, sub->GetDescriptor()->map_key(), 10);
sub->GetReflection()->SetInt32(sub, sub->GetDescriptor()->map_value(), 100);
}


res = MapReflectionTester::LookupMapValue(*reflection, *msg, *field, map_key);
// If this fails, LookupMapValue might have failed to sync from the repeated
// field.
ASSERT_TRUE(res.has_value());
EXPECT_EQ(res->GetInt32Value(), 100);
}

TEST_F(MapImplTest, SpaceUsed) {
constexpr size_t kMinCap = 16 / sizeof(void*);

Expand Down
16 changes: 10 additions & 6 deletions src/google/protobuf/reflection_tester.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
#ifndef GOOGLE_PROTOBUF_REFLECTION_TESTER_H__
#define GOOGLE_PROTOBUF_REFLECTION_TESTER_H__

#include <optional>

#include "google/protobuf/map_field.h"
#include "google/protobuf/message.h"

Expand Down Expand Up @@ -49,13 +51,15 @@ class MapReflectionTester {
MapIterator MapEnd(Message* message, const std::string& field_name);
int MapSize(const Message& message, const std::string& field_name);

static MapValueConstRef LookupMapValue(const Reflection& reflection,
const Message& message,
const FieldDescriptor& descriptor,
const MapKey& map_key) {
static std::optional<MapValueConstRef> LookupMapValue(
const Reflection& reflection, const Message& message,
const FieldDescriptor& descriptor, const MapKey& map_key) {
MapValueConstRef map_val_const;
reflection.LookupMapValue(message, &descriptor, map_key, &map_val_const);
return map_val_const;
if (reflection.LookupMapValue(message, &descriptor, map_key,
&map_val_const)) {
return map_val_const;
}
return std::nullopt;
}

static std::string long_string() {
Expand Down

0 comments on commit 874e03b

Please sign in to comment.