Skip to content

Commit 9cb3b5d

Browse files
jnthntatumcopybara-github
authored andcommitted
Update custom map Find and Has to treat errors as
ErrorValue. Remove special case for empty map/list in wrap field. PiperOrigin-RevId: 966971140
1 parent 42ac7cc commit 9cb3b5d

6 files changed

Lines changed: 314 additions & 27 deletions

File tree

common/value.cc

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <string>
2323
#include <type_traits>
2424
#include <utility>
25+
#include <variant>
2526

2627
#include "google/protobuf/struct.pb.h"
2728
#include "absl/base/attributes.h"
@@ -1520,9 +1521,6 @@ Value WrapFieldImpl(
15201521
message->GetDescriptor()->full_name())));
15211522
}
15221523
if (field->is_map()) {
1523-
if (reflection->FieldSize(*message, field) == 0) {
1524-
return MapValue();
1525-
}
15261524
if constexpr (Unsafe::value) {
15271525
return UnsafeParsedMapFieldValue(message, field);
15281526
} else {
@@ -1531,9 +1529,6 @@ Value WrapFieldImpl(
15311529
}
15321530
}
15331531
if (field->is_repeated()) {
1534-
if (reflection->FieldSize(*message, field) == 0) {
1535-
return ListValue();
1536-
}
15371532
if constexpr (Unsafe::value) {
15381533
return UnsafeParsedRepeatedFieldValue(message, field);
15391534
} else {

common/values/custom_map_value.cc

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414

1515
#include <cstddef>
1616
#include <memory>
17+
#include <optional>
1718
#include <string>
19+
#include <utility>
1820

1921
#include "absl/base/attributes.h"
2022
#include "absl/base/no_destructor.h"
@@ -673,24 +675,34 @@ absl::StatusOr<bool> CustomMapValue::Find(
673675
return false;
674676
}
675677

676-
bool ok;
677678
if (dispatcher_ == nullptr) {
678679
CustomMapValueInterface::Content content =
679680
content_.To<CustomMapValueInterface::Content>();
680681
ABSL_DCHECK(content.interface != nullptr);
681-
CEL_ASSIGN_OR_RETURN(
682-
ok, content.interface->Find(key, descriptor_pool, message_factory,
683-
arena, result));
684-
} else {
685-
CEL_ASSIGN_OR_RETURN(
686-
ok, dispatcher_->find(dispatcher_, content_, key, descriptor_pool,
687-
message_factory, arena, result));
688-
}
689-
if (ok) {
682+
auto status_or_found = content.interface->Find(
683+
key, descriptor_pool, message_factory, arena, result);
684+
if (!status_or_found.ok()) {
685+
*result = ErrorValue(std::move(status_or_found).status());
686+
return false;
687+
}
688+
if (!*status_or_found) {
689+
*result = NullValue();
690+
return false;
691+
}
690692
return true;
691693
}
692-
*result = NullValue{};
693-
return false;
694+
auto status_or_found =
695+
dispatcher_->find(dispatcher_, content_, key, descriptor_pool,
696+
message_factory, arena, result);
697+
if (!status_or_found.ok()) {
698+
*result = ErrorValue(std::move(status_or_found).status());
699+
return false;
700+
}
701+
if (!*status_or_found) {
702+
*result = NullValue();
703+
return false;
704+
}
705+
return true;
694706
}
695707

696708
absl::Status CustomMapValue::Has(
@@ -721,19 +733,26 @@ absl::Status CustomMapValue::Has(
721733
*result = ErrorValue(InvalidMapKeyTypeError(key.kind()));
722734
return absl::OkStatus();
723735
}
724-
bool has;
725736
if (dispatcher_ == nullptr) {
726737
CustomMapValueInterface::Content content =
727738
content_.To<CustomMapValueInterface::Content>();
728739
ABSL_DCHECK(content.interface != nullptr);
729-
CEL_ASSIGN_OR_RETURN(has, content.interface->Has(key, descriptor_pool,
730-
message_factory, arena));
731-
} else {
732-
CEL_ASSIGN_OR_RETURN(
733-
has, dispatcher_->has(dispatcher_, content_, key, descriptor_pool,
734-
message_factory, arena));
740+
auto status_or_has =
741+
content.interface->Has(key, descriptor_pool, message_factory, arena);
742+
if (!status_or_has.ok()) {
743+
*result = ErrorValue(std::move(status_or_has).status());
744+
return absl::OkStatus();
745+
}
746+
*result = BoolValue(*status_or_has);
747+
return absl::OkStatus();
748+
}
749+
auto status_or_has = dispatcher_->has(
750+
dispatcher_, content_, key, descriptor_pool, message_factory, arena);
751+
if (!status_or_has.ok()) {
752+
*result = ErrorValue(std::move(status_or_has).status());
753+
return absl::OkStatus();
735754
}
736-
*result = BoolValue(has);
755+
*result = BoolValue(*status_or_has);
737756
return absl::OkStatus();
738757
}
739758

common/values/custom_map_value.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ class CustomMapValueInterfaceKeysIterator;
5454
class CustomMapValue;
5555
using CustomMapValueContent = CustomValueContent;
5656

57+
// Dispatch table for `CustomMapValue`.
58+
//
59+
// See the documentation for `CustomMapValueInterface` for more details on
60+
// composite functions.
61+
//
62+
// See documentation for `UnsafeCustomMapValue` on how to use this class.
5763
struct CustomMapValueDispatcher {
5864
using GetTypeId =
5965
NativeTypeId (*)(const CustomMapValueDispatcher* absl_nonnull dispatcher,
@@ -247,12 +253,21 @@ class CustomMapValueInterface {
247253

248254
virtual CustomMapValue Clone(google::protobuf::Arena* absl_nonnull arena) const = 0;
249255

256+
// Tests whether the map contains the given key. If it does, the value
257+
// associated with the key is written to `result` and the function returns
258+
// true. Otherwise, the function returns false and `result` is set to
259+
// `NullValue`.
260+
//
261+
// A non-ok status is converted to an ErrorValue (e.g. wrong key type).
250262
virtual absl::StatusOr<bool> Find(
251263
const Value& key,
252264
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,
253265
google::protobuf::MessageFactory* absl_nonnull message_factory,
254266
google::protobuf::Arena* absl_nonnull arena, Value* absl_nonnull result) const = 0;
255267

268+
// Whether the map has the given key.
269+
//
270+
// A non-ok status is converted to an ErrorValue (e.g. wrong key type).
256271
virtual absl::StatusOr<bool> Has(
257272
const Value& key,
258273
const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool,

0 commit comments

Comments
 (0)