From 911d55ff634e39fc14a9395e0ffe1d86d76e43a7 Mon Sep 17 00:00:00 2001 From: Francis Tsui Date: Mon, 3 Jun 2024 12:26:58 -0700 Subject: [PATCH] Change SendAttachements to accept an AttachmentContainer. PiperOrigin-RevId: 639874033 --- sharing/attachment.h | 4 --- sharing/fake_nearby_sharing_service.cc | 4 +-- sharing/fake_nearby_sharing_service.h | 4 +-- sharing/file_attachment.cc | 5 --- sharing/file_attachment.h | 3 -- sharing/nearby_sharing_service.h | 5 ++- sharing/nearby_sharing_service_impl.cc | 32 +++++++---------- sharing/nearby_sharing_service_impl.h | 3 +- sharing/nearby_sharing_service_impl_test.cc | 40 ++++++++++----------- sharing/text_attachment.cc | 5 --- sharing/text_attachment.h | 3 -- sharing/wifi_credentials_attachment.cc | 6 ---- sharing/wifi_credentials_attachment.h | 3 -- 13 files changed, 39 insertions(+), 78 deletions(-) diff --git a/sharing/attachment.h b/sharing/attachment.h index afec2eec..ec2e2a49 100644 --- a/sharing/attachment.h +++ b/sharing/attachment.h @@ -23,8 +23,6 @@ namespace nearby { namespace sharing { -class AttachmentContainer; - // A single attachment to be sent by / received from a ShareTarget, can be // either a file or text. class Attachment { @@ -65,8 +63,6 @@ class Attachment { int32_t batch_id() const { return batch_id_; } SourceType source_type() const { return source_type_; } - // Move the attachment to a container - virtual void MoveToContainer(AttachmentContainer& container) = 0; virtual absl::string_view GetDescription() const = 0; virtual ShareType GetShareType() const = 0; diff --git a/sharing/fake_nearby_sharing_service.cc b/sharing/fake_nearby_sharing_service.cc index 234ccbbf..6b005407 100644 --- a/sharing/fake_nearby_sharing_service.cc +++ b/sharing/fake_nearby_sharing_service.cc @@ -18,12 +18,10 @@ #include #include #include -#include #include "absl/strings/string_view.h" #include "internal/base/observer_list.h" #include "sharing/advertisement.h" -#include "sharing/attachment.h" #include "sharing/attachment_container.h" #include "sharing/internal/api/sharing_rpc_notifier.h" #include "sharing/local_device_data/nearby_share_local_device_data_manager.h" @@ -137,7 +135,7 @@ bool FakeNearbySharingService::IsScanning() const { return false; } // Sends |attachments| to the remote |share_target|. void FakeNearbySharingService::SendAttachments( int64_t share_target_id, - std::vector> attachments, + std::unique_ptr attachment_container, std::function status_codes_callback) { status_codes_callback(StatusCodes::kOk); } diff --git a/sharing/fake_nearby_sharing_service.h b/sharing/fake_nearby_sharing_service.h index 32764edc..237cddeb 100644 --- a/sharing/fake_nearby_sharing_service.h +++ b/sharing/fake_nearby_sharing_service.h @@ -19,11 +19,9 @@ #include #include #include -#include #include "absl/strings/string_view.h" #include "internal/base/observer_list.h" -#include "sharing/attachment.h" #include "sharing/attachment_container.h" #include "sharing/internal/api/sharing_rpc_notifier.h" #include "sharing/local_device_data/nearby_share_local_device_data_manager.h" @@ -99,7 +97,7 @@ class FakeNearbySharingService : public NearbySharingService { // Sends |attachments| to the remote |share_target|. void SendAttachments( int64_t share_target_id, - std::vector> attachments, + std::unique_ptr attachment_container, std::function status_codes_callback) override; // Accepts incoming share from the remote |share_target|. diff --git a/sharing/file_attachment.cc b/sharing/file_attachment.cc index 9284e2e1..7ec995f7 100644 --- a/sharing/file_attachment.cc +++ b/sharing/file_attachment.cc @@ -23,7 +23,6 @@ #include "absl/strings/match.h" #include "absl/strings/string_view.h" #include "sharing/attachment.h" -#include "sharing/attachment_container.h" #include "sharing/common/compatible_u8_string.h" #include "sharing/common/nearby_share_enums.h" #include "sharing/internal/base/mime.h" @@ -78,10 +77,6 @@ FileAttachment::FileAttachment(int64_t id, int64_t size, std::string file_name, type_(type), parent_folder_(std::move(parent_folder)) {} -void FileAttachment::MoveToContainer(AttachmentContainer& container) { - container.AddFileAttachment(std::move(*this)); -} - absl::string_view FileAttachment::GetDescription() const { return file_name_; } ShareType FileAttachment::GetShareType() const { diff --git a/sharing/file_attachment.h b/sharing/file_attachment.h index 5754b2d7..9bc1e28e 100644 --- a/sharing/file_attachment.h +++ b/sharing/file_attachment.h @@ -31,8 +31,6 @@ namespace sharing { // A single attachment to be sent by / received from a |ShareTarget|, can be // either a file or text. -class AttachmentContainer; - class FileAttachment : public Attachment { public: using Type = nearby::sharing::service::proto::FileMetadata::Type; @@ -59,7 +57,6 @@ class FileAttachment : public Attachment { } // Attachment: - void MoveToContainer(AttachmentContainer& container) override; absl::string_view GetDescription() const override; ShareType GetShareType() const override; diff --git a/sharing/nearby_sharing_service.h b/sharing/nearby_sharing_service.h index 98050a91..1867c410 100644 --- a/sharing/nearby_sharing_service.h +++ b/sharing/nearby_sharing_service.h @@ -19,12 +19,11 @@ #include #include #include -#include #include "absl/strings/string_view.h" #include "internal/network/url.h" #include "sharing/advertisement.h" -#include "sharing/attachment.h" +#include "sharing/attachment_container.h" #include "sharing/internal/api/sharing_rpc_notifier.h" #include "sharing/local_device_data/nearby_share_local_device_data_manager.h" #include "sharing/nearby_sharing_settings.h" @@ -203,7 +202,7 @@ class NearbySharingService { // Sends |attachments| to the remote |share_target|. virtual void SendAttachments( int64_t share_target_id, - std::vector> attachments, + std::unique_ptr attachment_container, std::function status_codes_callback) = 0; // Accepts incoming share from the remote |share_target|. diff --git a/sharing/nearby_sharing_service_impl.cc b/sharing/nearby_sharing_service_impl.cc index 164047c8..7ca31a9a 100644 --- a/sharing/nearby_sharing_service_impl.cc +++ b/sharing/nearby_sharing_service_impl.cc @@ -665,11 +665,12 @@ std::string NearbySharingServiceImpl::GetQrCodeUrl() const { void NearbySharingServiceImpl::SendAttachments( int64_t share_target_id, - std::vector> attachments, + std::unique_ptr attachment_container, std::function status_codes_callback) { RunOnNearbySharingServiceThread( "api_send_attachments", - [this, share_target_id, attachments = std::move(attachments), + [this, share_target_id, + attachment_container = std::move(attachment_container), status_codes_callback = std::move(status_codes_callback)]() mutable { if (!is_scanning_) { NL_LOG(WARNING) << __func__ @@ -684,11 +685,19 @@ void NearbySharingServiceImpl::SendAttachments( // |is_scanning_| and |is_transferring_| are mutually exclusive. NL_DCHECK(!is_transferring_); - if (attachments.empty()) { + if (!attachment_container || !attachment_container->HasAttachments()) { NL_LOG(WARNING) << __func__ << ": No attachments to send."; std::move(status_codes_callback)(StatusCodes::kInvalidArgument); return; } + for (const FileAttachment& attachment : + attachment_container->GetFileAttachments()) { + if (!attachment.file_path()) { + NL_LOG(WARNING) << __func__ << ": Got file attachment without path"; + std::move(status_codes_callback)(StatusCodes::kInvalidArgument); + return; + } + } // Outgoing connections always announces with contacts visibility. std::optional> endpoint_info = CreateEndpointInfo(DeviceVisibility::DEVICE_VISIBILITY_ALL_CONTACTS, @@ -711,22 +720,7 @@ void NearbySharingServiceImpl::SendAttachments( } ShareTarget share_target = info->share_target(); - AttachmentContainer& container = share_target.attachment_container; - for (std::unique_ptr& attachment : attachments) { - attachment->MoveToContainer(container); - } - if (!container.HasAttachments()) { - std::move(status_codes_callback)(StatusCodes::kInvalidArgument); - return; - } - for (const FileAttachment& attachment : - container.GetFileAttachments()) { - if (!attachment.file_path()) { - NL_LOG(WARNING) << __func__ << ": Got file attachment without path"; - std::move(status_codes_callback)(StatusCodes::kInvalidArgument); - return; - } - } + share_target.attachment_container = std::move(*attachment_container); app_info_->SetActiveFlag(); // Set session ID. diff --git a/sharing/nearby_sharing_service_impl.h b/sharing/nearby_sharing_service_impl.h index 193d88a9..3151178f 100644 --- a/sharing/nearby_sharing_service_impl.h +++ b/sharing/nearby_sharing_service_impl.h @@ -44,6 +44,7 @@ #include "sharing/advertisement.h" #include "sharing/analytics/analytics_recorder.h" #include "sharing/attachment.h" +#include "sharing/attachment_container.h" #include "sharing/attachment_info.h" #include "sharing/certificates/nearby_share_certificate_manager.h" #include "sharing/certificates/nearby_share_decrypted_public_certificate.h" @@ -155,7 +156,7 @@ class NearbySharingServiceImpl std::string GetQrCodeUrl() const override; void SendAttachments( int64_t share_target_id, - std::vector> attachments, + std::unique_ptr attachment_container, std::function status_codes_callback) override; void Accept(int64_t share_target_id, std::function diff --git a/sharing/nearby_sharing_service_impl_test.cc b/sharing/nearby_sharing_service_impl_test.cc index 5c3395da..b2d93e94 100644 --- a/sharing/nearby_sharing_service_impl_test.cc +++ b/sharing/nearby_sharing_service_impl_test.cc @@ -46,7 +46,7 @@ #include "internal/test/fake_device_info.h" #include "internal/test/fake_task_runner.h" #include "sharing/advertisement.h" -#include "sharing/attachment.h" +#include "sharing/attachment_container.h" #include "sharing/certificates/fake_nearby_share_certificate_manager.h" #include "sharing/certificates/nearby_share_certificate_manager_impl.h" #include "sharing/certificates/nearby_share_decrypted_public_certificate.h" @@ -333,35 +333,35 @@ std::unique_ptr GetCancelFrame() { return std::unique_ptr(frame); } -std::vector> CreateTextAttachments( +std::unique_ptr CreateTextAttachments( std::vector texts) { - std::vector> attachments; + auto attachment_container = std::make_unique(); for (auto& text : texts) { - attachments.push_back(std::make_unique( - service::proto::TextMetadata::TEXT, std::move(text), - /*text_title=*/std::nullopt, - /*mime_type=*/std::nullopt)); + attachment_container->AddTextAttachment( + TextAttachment(service::proto::TextMetadata::TEXT, std::move(text), + /*text_title=*/std::nullopt, + /*mime_type=*/std::nullopt)); } - return attachments; + return attachment_container; } -std::vector> CreateFileAttachments( +std::unique_ptr CreateFileAttachments( std::vector file_paths) { - std::vector> attachments; + auto attachment_container = std::make_unique(); for (auto& file_path : file_paths) { - attachments.push_back( - std::make_unique(std::move(file_path))); + attachment_container->AddFileAttachment( + FileAttachment(std::move(file_path))); } - return attachments; + return attachment_container; } -std::vector> CreateWifiCredentialAttachments( +std::unique_ptr CreateWifiCredentialAttachments( std::string ssid, std::string password) { - std::vector> attachments; - attachments.push_back(std::make_unique( + auto attachment_container = std::make_unique(); + attachment_container->AddWifiCredentialsAttachment(WifiCredentialsAttachment( std::move(ssid), service::proto::WifiCredentialsMetadata::WPA_PSK, std::move(password))); - return attachments; + return attachment_container; } class NearbySharingServiceImplTest : public testing::Test { @@ -562,12 +562,12 @@ class NearbySharingServiceImplTest : public testing::Test { NearbySharingService::StatusCodes SendAttachments( const ShareTarget& share_target, - std::vector> attachments) { + std::unique_ptr attachment_container) { NearbySharingService::StatusCodes result = NearbySharingService::StatusCodes::kError; absl::Notification notification; service_->SendAttachments( - share_target.id, std::move(attachments), + share_target.id, std::move(attachment_container), [&](NearbySharingService::StatusCodes status_codes) { result = status_codes; notification.Notify(); @@ -3431,7 +3431,7 @@ TEST_F(NearbySharingServiceImplTest, SendAttachmentsWithoutAttachments) { ShareTarget target = DiscoverShareTarget(transfer_callback, discovery_callback); - EXPECT_EQ(SendAttachments(target, /*attachments=*/{}), + EXPECT_EQ(SendAttachments(target, /*attachment_container=*/nullptr), NearbySharingServiceImpl::StatusCodes::kInvalidArgument); UnregisterSendSurface(&transfer_callback, &discovery_callback); diff --git a/sharing/text_attachment.cc b/sharing/text_attachment.cc index 91be1258..4ba395a2 100644 --- a/sharing/text_attachment.cc +++ b/sharing/text_attachment.cc @@ -27,7 +27,6 @@ #include "absl/strings/string_view.h" #include "internal/network/url.h" #include "sharing/attachment.h" -#include "sharing/attachment_container.h" #include "sharing/common/nearby_share_enums.h" #include "sharing/proto/wire_format.pb.h" @@ -146,10 +145,6 @@ TextAttachment::TextAttachment(int64_t id, Type type, std::string text_body, text_body_(std::move(text_body)), mime_type_(std::move(mime_type)) {} -void TextAttachment::MoveToContainer(AttachmentContainer& container) { - container.AddTextAttachment(std::move(*this)); -} - absl::string_view TextAttachment::GetDescription() const { return text_title_; } ShareType TextAttachment::GetShareType() const { diff --git a/sharing/text_attachment.h b/sharing/text_attachment.h index a93eae01..833f52a0 100644 --- a/sharing/text_attachment.h +++ b/sharing/text_attachment.h @@ -28,8 +28,6 @@ namespace nearby { namespace sharing { // Represents a text attachment. -class AttachmentContainer; - class TextAttachment : public Attachment { public: using Type = nearby::sharing::service::proto::TextMetadata::Type; @@ -55,7 +53,6 @@ class TextAttachment : public Attachment { Type type() const { return type_; } // Attachment: - void MoveToContainer(AttachmentContainer& container) override; absl::string_view GetDescription() const override; ShareType GetShareType() const override; diff --git a/sharing/wifi_credentials_attachment.cc b/sharing/wifi_credentials_attachment.cc index 4b718743..d5ab5d93 100644 --- a/sharing/wifi_credentials_attachment.cc +++ b/sharing/wifi_credentials_attachment.cc @@ -20,7 +20,6 @@ #include "absl/strings/string_view.h" #include "sharing/attachment.h" -#include "sharing/attachment_container.h" #include "sharing/common/nearby_share_enums.h" namespace nearby { @@ -47,11 +46,6 @@ WifiCredentialsAttachment::WifiCredentialsAttachment( password_(std::move(password)), is_hidden_(is_hidden) {} -void WifiCredentialsAttachment::MoveToContainer( - AttachmentContainer& container) { - container.AddWifiCredentialsAttachment(std::move(*this)); -} - absl::string_view WifiCredentialsAttachment::GetDescription() const { return ssid_; } diff --git a/sharing/wifi_credentials_attachment.h b/sharing/wifi_credentials_attachment.h index 5def3cfe..38d4fffe 100644 --- a/sharing/wifi_credentials_attachment.h +++ b/sharing/wifi_credentials_attachment.h @@ -27,8 +27,6 @@ namespace nearby { namespace sharing { // Represents a WiFi credentials attachment. -class AttachmentContainer; - class WifiCredentialsAttachment : public Attachment { public: using SecurityType = @@ -56,7 +54,6 @@ class WifiCredentialsAttachment : public Attachment { bool is_hidden() const { return is_hidden_; } // Attachment: - void MoveToContainer(AttachmentContainer& container) override; absl::string_view GetDescription() const override; ShareType GetShareType() const override;