From f7b22c66ff7ef9ea9f4688812cbbab2fd48cf098 Mon Sep 17 00:00:00 2001 From: Tommi Date: Thu, 15 Feb 2024 09:18:34 +0100 Subject: [PATCH] Add Candidate::type_name() Candidate::type() is currently how the name of the type is fetched, but that getter returns a non-standard type name. Instead, I'm adding a new getter, type_name(), will follow up with updating dependent code that needs the string, to use type_name (and adapt to potential dependency on "local" or "stun") and then switch type() to be enum based. Also adding a test file for Candidate with a couple of basic tests to start with. Bug: webrtc:15846 Change-Id: I9b78b2405a9f962a3c07eaa8e72a79854c6f5ceb Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/339660 Reviewed-by: Harald Alvestrand Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#41740} --- api/BUILD.gn | 4 +++ api/candidate.cc | 12 +++++++++ api/candidate.h | 4 +++ api/candidate_unittest.cc | 54 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+) create mode 100644 api/candidate_unittest.cc diff --git a/api/BUILD.gn b/api/BUILD.gn index e8d03f0e3e..0c982a2ac6 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -1430,6 +1430,7 @@ if (rtc_include_tests) { sources = [ "array_view_unittest.cc", + "candidate_unittest.cc", "field_trials_unittest.cc", "function_view_unittest.cc", "rtc_error_unittest.cc", @@ -1445,6 +1446,7 @@ if (rtc_include_tests) { deps = [ ":array_view", + ":candidate", ":create_time_controller", ":field_trials", ":field_trials_view", @@ -1458,12 +1460,14 @@ if (rtc_include_tests) { ":scoped_refptr", ":sequence_checker", ":time_controller", + "../p2p:rtc_p2p", "../rtc_base:buffer", "../rtc_base:checks", "../rtc_base:gunit_helpers", "../rtc_base:platform_thread", "../rtc_base:rtc_event", "../rtc_base:rtc_task_queue", + "../rtc_base:ssl", "../rtc_base:task_queue_for_test", "../rtc_base/containers:flat_set", "../rtc_base/task_utils:repeating_task", diff --git a/api/candidate.cc b/api/candidate.cc index 4311556233..8969b06583 100644 --- a/api/candidate.cc +++ b/api/candidate.cc @@ -27,6 +27,7 @@ Candidate::Candidate() : id_(rtc::CreateRandomString(8)), component_(0), priority_(0), + type_(LOCAL_PORT_TYPE), network_type_(rtc::ADAPTER_TYPE_UNKNOWN), underlying_type_for_vpn_(rtc::ADAPTER_TYPE_UNKNOWN), generation_(0), @@ -76,6 +77,17 @@ bool Candidate::is_relay() const { return type_ == RELAY_PORT_TYPE; } +absl::string_view Candidate::type_name() const { + // The LOCAL_PORT_TYPE and STUN_PORT_TYPE constants are not the standard type + // names, so check for those specifically. For other types, `type_` will have + // the correct name. + if (is_local()) + return "host"; + if (is_stun()) + return "srflx"; + return type_; +} + bool Candidate::IsEquivalent(const Candidate& c) const { // We ignore the network name, since that is just debug information, and // the priority and the network cost, since they should be the same if the diff --git a/api/candidate.h b/api/candidate.h index 12b1512e72..101a3bd0a4 100644 --- a/api/candidate.h +++ b/api/candidate.h @@ -93,6 +93,10 @@ class RTC_EXPORT Candidate { const std::string& type() const { return type_; } + // Returns the name of the candidate type as specified in + // https://datatracker.ietf.org/doc/html/rfc5245#section-15.1 + absl::string_view type_name() const; + // Setting the type requires a constant string (e.g. // cricket::LOCAL_PORT_TYPE). The type should really be an enum rather than a // string, but until we make that change the lifetime attribute helps us lock diff --git a/api/candidate_unittest.cc b/api/candidate_unittest.cc new file mode 100644 index 0000000000..fa512d6220 --- /dev/null +++ b/api/candidate_unittest.cc @@ -0,0 +1,54 @@ +/* + * Copyright 2024 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "api/candidate.h" + +#include + +#include "p2p/base/p2p_constants.h" +#include "rtc_base/gunit.h" + +namespace cricket { + +TEST(CandidateTest, Id) { + Candidate c; + EXPECT_EQ(c.id().size(), 8u); + std::string new_id = "12345678"; + c.set_id(new_id); + EXPECT_EQ(new_id, c.id()); +} + +TEST(CandidateTest, Component) { + Candidate c; + EXPECT_EQ(c.component(), 0); + c.set_component(ICE_CANDIDATE_COMPONENT_DEFAULT); + EXPECT_EQ(c.component(), ICE_CANDIDATE_COMPONENT_DEFAULT); +} + +TEST(CandidateTest, TypeName) { + Candidate c; + // The `type_name()` property defaults to "host". + EXPECT_EQ(c.type_name(), "host"); + EXPECT_EQ(c.type(), LOCAL_PORT_TYPE); + + c.set_type(STUN_PORT_TYPE); + EXPECT_EQ(c.type_name(), "srflx"); + EXPECT_EQ(c.type(), STUN_PORT_TYPE); + + c.set_type(PRFLX_PORT_TYPE); + EXPECT_EQ(c.type_name(), "prflx"); + EXPECT_EQ(c.type(), PRFLX_PORT_TYPE); + + c.set_type(RELAY_PORT_TYPE); + EXPECT_EQ(c.type_name(), "relay"); + EXPECT_EQ(c.type(), RELAY_PORT_TYPE); +} + +} // namespace cricket