Cleanup apply_endpoints_dedup flag.

PiperOrigin-RevId: 705512858
This commit is contained in:
Francis Tsui
2024-12-12 08:28:01 -08:00
committed by Copybara-Service
parent eae28d3a45
commit 1e1eab7e9f
3 changed files with 21 additions and 147 deletions
@@ -73,9 +73,6 @@ constexpr auto kShowAutoUpdateSetting =
// When true, use gRpc client to access backend.
constexpr auto kUseGrpcClient =
flags::Flag<bool>(kConfigPackage, "45630055", false);
// When true, dedup discovered endpoints.
constexpr auto kApplyEndpointsDedup =
flags::Flag<bool>(kConfigPackage, "45656298", false);
// When true, call the 3P Nearby Identity API instead of the 1P private API
constexpr auto kCallNearbyIdentityApi =
flags::Flag<bool>(kConfigPackage, "45667328", false);
@@ -128,7 +125,6 @@ inline absl::btree_map<int, const flags::Flag<bool>&> GetBoolFlags() {
{45411353, kSenderSkipsConfirmation},
{45409033, kShowAutoUpdateSetting},
{45630055, kUseGrpcClient},
{45656298, kApplyEndpointsDedup},
{45667328, kCallNearbyIdentityApi},
{45664277, kDedupInUnregisterShareTarget},
{45657036, kDeleteUnexpectedReceivedFileFix},
+15 -52
View File
@@ -1667,19 +1667,6 @@ void NearbySharingServiceImpl::HandleEndpointDiscovered(
// Check outgoingShareSessionMap first and pass the same shareTarget if we
// found one.
// Looking for the ShareTarget based on endpoint id.
if (!NearbyFlags::GetInstance().GetBoolFlag(
config_package_nearby::nearby_sharing_feature::
kApplyEndpointsDedup)) {
if (outgoing_share_target_map_.find(endpoint_id) !=
outgoing_share_target_map_.end()) {
LOG(INFO) << __func__ << ": Ignoring endpoint_id: " << endpoint_id
<< " because it is already in the "
<< "outgoingShareTargetMap.";
FinishEndpointDiscoveryEvent();
return;
}
}
// Once we get the advertisement, the first thing to do is decrypt the
// certificate.
NearbyShareEncryptedMetadataKey encrypted_metadata_key(
@@ -1723,16 +1710,10 @@ void NearbySharingServiceImpl::HandleEndpointLost(
discovered_advertisements_to_retry_map_.erase(endpoint_id);
discovered_advertisements_retried_set_.erase(endpoint_id);
if (NearbyFlags::GetInstance().GetBoolFlag(
config_package_nearby::nearby_sharing_feature::
kApplyEndpointsDedup)) {
MoveToDiscoveryCache(endpoint_id,
NearbyFlags::GetInstance().GetInt64Flag(
config_package_nearby::nearby_sharing_feature::
kDiscoveryCacheLostExpiryMs));
} else {
RemoveOutgoingShareTargetAndReportLost(endpoint_id);
}
MoveToDiscoveryCache(endpoint_id,
NearbyFlags::GetInstance().GetInt64Flag(
config_package_nearby::nearby_sharing_feature::
kDiscoveryCacheLostExpiryMs));
FinishEndpointDiscoveryEvent();
}
@@ -1753,19 +1734,6 @@ void NearbySharingServiceImpl::OnOutgoingDecryptedCertificate(
absl::string_view endpoint_id, absl::Span<const uint8_t> endpoint_info,
const Advertisement& advertisement,
std::optional<NearbyShareDecryptedPublicCertificate> certificate) {
bool apply_endpoints_dedup = NearbyFlags::GetInstance().GetBoolFlag(
config_package_nearby::nearby_sharing_feature::kApplyEndpointsDedup);
if (!apply_endpoints_dedup) {
// Check again for this endpoint id, to avoid race conditions.
if (outgoing_share_target_map_.find(endpoint_id) !=
outgoing_share_target_map_.end()) {
LOG(INFO) << __func__ << ": Ignoring endpoint_id: " << endpoint_id
<< " because it is already in the "
<< "outgoingShareTargetMap.";
FinishEndpointDiscoveryEvent();
return;
}
}
// The certificate provides the device name, in order to create a ShareTarget
// to represent this remote device.
@@ -1793,19 +1761,17 @@ void NearbySharingServiceImpl::OnOutgoingDecryptedCertificate(
FinishEndpointDiscoveryEvent();
return;
}
if (apply_endpoints_dedup) {
if (FindDuplicateInOutgoingShareTargets(endpoint_id, *share_target)) {
DeduplicateInOutgoingShareTarget(*share_target, endpoint_id,
std::move(certificate));
FinishEndpointDiscoveryEvent();
return;
}
if (FindDuplicateInDiscoveryCache(endpoint_id, *share_target)) {
DeDuplicateInDiscoveryCache(*share_target, endpoint_id,
std::move(certificate));
FinishEndpointDiscoveryEvent();
return;
}
if (FindDuplicateInOutgoingShareTargets(endpoint_id, *share_target)) {
DeduplicateInOutgoingShareTarget(*share_target, endpoint_id,
std::move(certificate));
FinishEndpointDiscoveryEvent();
return;
}
if (FindDuplicateInDiscoveryCache(endpoint_id, *share_target)) {
DeDuplicateInDiscoveryCache(*share_target, endpoint_id,
std::move(certificate));
FinishEndpointDiscoveryEvent();
return;
}
VLOG(1) << __func__ << ": Adding (endpoint_id=" << endpoint_id
@@ -3518,9 +3484,6 @@ void NearbySharingServiceImpl::UnregisterShareTarget(int64_t share_target_id) {
auto it = outgoing_share_session_map_.find(share_target_id);
if (it != outgoing_share_session_map_.end()) {
if (NearbyFlags::GetInstance().GetBoolFlag(
config_package_nearby::nearby_sharing_feature::
kApplyEndpointsDedup) &&
NearbyFlags::GetInstance().GetBoolFlag(
config_package_nearby::nearby_sharing_feature::
kDedupInUnregisterShareTarget)) {
LOG(INFO) << __func__ << ": [Dedupped] Move the endpoint "
+6 -91
View File
@@ -3972,19 +3972,19 @@ TEST_F(NearbySharingServiceImplTest, OrderedEndpointDiscoveryEvents) {
// Order of events:
// - Nearby Connections discovers endpoint 1
// - Nearby Connections loses endpoint 1
// - Nearby Share processes these two events in order.
// - Nearby Share processes discovered event.
// - endpoint 1 moved to discovery cache.
// - Nearby Connections discovers endpoint 2
// - Nearby Connections discovers endpoint 3
// - Nearby Connections loses endpoint 3
// - Nearby Connections loses endpoint 2
// - Nearby Share processes these four events in order.
// - Nearby Share processes these 2 discovered events in order.
// - endpoints 2 and 3 moved to discovery cache.
{
absl::Notification notification;
FindEndpoint(/*endpoint_id=*/"1");
LoseEndpoint(/*endpoint_id=*/"1");
InSequence s;
EXPECT_CALL(discovery_callback, OnShareTargetDiscovered);
EXPECT_CALL(discovery_callback, OnShareTargetLost)
EXPECT_CALL(discovery_callback, OnShareTargetDiscovered)
.WillOnce([&](ShareTarget share_target) { notification.Notify(); });
// Needed for discovery processing.
@@ -4004,16 +4004,8 @@ TEST_F(NearbySharingServiceImplTest, OrderedEndpointDiscoveryEvents) {
EXPECT_EQ(share_target.device_id, "2");
});
EXPECT_CALL(discovery_callback, OnShareTargetDiscovered)
.WillOnce([](ShareTarget share_target) {
EXPECT_EQ(share_target.device_id, "3");
});
EXPECT_CALL(discovery_callback, OnShareTargetLost)
.WillOnce([](ShareTarget share_target) {
EXPECT_EQ(share_target.device_id, "3");
});
EXPECT_CALL(discovery_callback, OnShareTargetLost)
.WillOnce([&](ShareTarget share_target) {
EXPECT_EQ(share_target.device_id, "2");
EXPECT_EQ(share_target.device_id, "3");
notification.Notify();
});
@@ -4064,9 +4056,6 @@ TEST_F(NearbySharingServiceImplTest,
}
TEST_F(NearbySharingServiceImplTest, DedupSameEndpointId) {
NearbyFlags::GetInstance().OverrideBoolFlagValue(
config_package_nearby::nearby_sharing_feature::kApplyEndpointsDedup,
true);
// Start discovery.
SetConnectionType(ConnectionType::kWifi);
MockTransferUpdateCallback transfer_callback;
@@ -4113,9 +4102,6 @@ TEST_F(NearbySharingServiceImplTest, DedupSameEndpointId) {
TEST_F(NearbySharingServiceImplTest,
OnLostDedupSameEndpointIdBeforeExpiryNoOnShareTargetLost) {
NearbyFlags::GetInstance().OverrideBoolFlagValue(
config_package_nearby::nearby_sharing_feature::kApplyEndpointsDedup,
true);
// Start discovery.
SetConnectionType(ConnectionType::kWifi);
MockTransferUpdateCallback transfer_callback;
@@ -4172,9 +4158,6 @@ TEST_F(NearbySharingServiceImplTest,
}
TEST_F(NearbySharingServiceImplTest, OnLostDedupSameEndpointIdAfterExpiry) {
NearbyFlags::GetInstance().OverrideBoolFlagValue(
config_package_nearby::nearby_sharing_feature::kApplyEndpointsDedup,
true);
// Start discovery.
SetConnectionType(ConnectionType::kWifi);
MockTransferUpdateCallback transfer_callback;
@@ -4236,77 +4219,9 @@ TEST_F(NearbySharingServiceImplTest, OnLostDedupSameEndpointIdAfterExpiry) {
service_.reset();
}
TEST_F(NearbySharingServiceImplTest,
RetryDiscoveredEndpointsDownloadCertsAndRetryDecryption) {
NearbyFlags::GetInstance().OverrideBoolFlagValue(
config_package_nearby::nearby_sharing_feature::kApplyEndpointsDedup,
false);
// Start discovery.
SetConnectionType(ConnectionType::kWifi);
MockTransferUpdateCallback transfer_callback;
MockShareTargetDiscoveredCallback discovery_callback;
RegisterSendSurface(&transfer_callback, &discovery_callback,
SendSurfaceState::kForeground);
EXPECT_EQ(certificate_manager()->num_download_public_certificates_calls(),
1u);
EXPECT_TRUE(fake_nearby_connections_manager_->IsDiscovering());
// Order of events:
// - Discover endpoint 1 --> decrypts public certificate
// - Discover endpoint 2 --> cannot decrypt public certificate
// - Discover endpoint 3 --> decrypts public certificate
// - Discover endpoint 4 --> cannot decrypt public certificate
// - Lose endpoint 3
// - Fire certificate download timer --> certificates downloaded
// - (Re)discover endpoints 2 and 4
{
absl::Notification notification;
FindInvalidEndpoint(/*endpoint_id=*/"1");
FindInvalidEndpoint(/*endpoint_id=*/"2");
FindInvalidEndpoint(/*endpoint_id=*/"3");
FindInvalidEndpoint(/*endpoint_id=*/"4");
LoseEndpoint(/*endpoint_id=*/"3");
::testing::InSequence s;
EXPECT_CALL(discovery_callback, OnShareTargetDiscovered).Times(2);
EXPECT_CALL(discovery_callback, OnShareTargetLost)
.WillOnce([&](ShareTarget share_target) { notification.Notify(); });
ProcessLatestPublicCertificateDecryption(/*expected_num_calls=*/1,
/*success=*/true);
ProcessLatestPublicCertificateDecryption(/*expected_num_calls=*/2,
/*success=*/false);
ProcessLatestPublicCertificateDecryption(/*expected_num_calls=*/3,
/*success=*/true);
ProcessLatestPublicCertificateDecryption(/*expected_num_calls=*/4,
/*success=*/false);
EXPECT_TRUE(notification.WaitForNotificationWithTimeout(kWaitTimeout));
}
FastForward(kCertificateDownloadDuringDiscoveryPeriod);
EXPECT_EQ(certificate_manager()->num_download_public_certificates_calls(),
2u);
certificate_manager()->NotifyPublicCertificatesDownloaded();
FlushTesting();
{
absl::Notification notification;
::testing::InSequence s;
EXPECT_CALL(discovery_callback, OnShareTargetDiscovered);
EXPECT_CALL(discovery_callback, OnShareTargetDiscovered)
.WillOnce([&](ShareTarget share_target) { notification.Notify(); });
ProcessLatestPublicCertificateDecryption(/*expected_num_calls=*/5,
/*success=*/true);
ProcessLatestPublicCertificateDecryption(/*expected_num_calls=*/6,
/*success=*/true);
EXPECT_TRUE(notification.WaitForNotificationWithTimeout(kWaitTimeout));
}
EXPECT_CALL(discovery_callback, OnShareTargetLost).Times(3);
Shutdown();
service_.reset();
}
// This test verifies the de-dup logic. Since certificates are the same, all the
// share targets are duplicates.
TEST_F(NearbySharingServiceImplTest, EndpointDedupBasedOnDeviceId) {
NearbyFlags::GetInstance().OverrideBoolFlagValue(
config_package_nearby::nearby_sharing_feature::kApplyEndpointsDedup,
true);
// Make kDiscoveryCacheLostExpiryMs larger than
// kCertificateDownloadDuringDiscoveryPeriod (10s).
NearbyFlags::GetInstance().OverrideInt64FlagValue(