Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b8012f78 authored by Luke Huang's avatar Luke Huang
Browse files

Fix default value problem of DnsResolver Experiments#getFlag() 

The default value of Experiments::getFlag() doesn't actually work
because Experiments::updateInternal() wil also initial the property as
0. Use INT_MIN as the default value to represent the flag can't be
found.

Bug: 155042187
Test: atest
Merged-In: Icc863b9a9d7b4cefca4dce23933c705a51299095
Change-Id: I125a6eeecee4995640d276c06b7d9f48108e33f1
(cherry picked from commit 2c1e86ed)
parent 328324ef

Experiments.cpp

+8 −3
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


#include "Experiments.h"



#include <android-base/format.h>

#include <netdutils/DumpWriter.h>

#include <string>
@@ -46,21 +47,25 @@ void Experiments::dump(DumpWriter& dw) const { 
    dw.println("Experiments list: ");

    for (const auto& [key, value] : mFlagsMapInt) {

        ScopedIndent indentStats(dw);

        dw.println("%.*s: %d", static_cast<int>(key.length()), key.data(), value);

        if (value == Experiments::kFlagIntDefault) {

            dw.println(fmt::format("{}: UNSET", key));

        } else {

            dw.println(fmt::format("{}: {}", key, value));

        }

    }

}



void Experiments::updateInternal() {

    std::lock_guard guard(mMutex);

    for (const auto& key : kExperimentFlagKeyList) {

        mFlagsMapInt[key] = mGetExperimentFlagIntFunction(key, 0);

        mFlagsMapInt[key] = mGetExperimentFlagIntFunction(key, Experiments::kFlagIntDefault);

    }

}



int Experiments::getFlag(std::string_view key, int defaultValue) const {

    std::lock_guard guard(mMutex);

    auto it = mFlagsMapInt.find(key);

    if (it != mFlagsMapInt.end()) {

    if (it != mFlagsMapInt.end() && it->second != Experiments::kFlagIntDefault) {

        return it->second;

    }

    return defaultValue;

Experiments.h

+3 −0
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


#pragma once



#include <climits>

#include <mutex>

#include <string>

#include <string_view>
@@ -49,6 +50,8 @@ class Experiments { 
    // (retry_count, retransmission_time_interval, dot_connect_timeout_ms)

    static constexpr const char* const kExperimentFlagKeyList[] = {

            "keep_listening_udp", "parallel_lookup", "parallel_lookup_sleep_time"};

    // This value is used in updateInternal as the default value if any flags can't be found.

    static constexpr int kFlagIntDefault = INT_MIN;

    // For testing.

    friend class ExperimentsTest;

    const GetExperimentFlagIntFunction mGetExperimentFlagIntFunction;

ExperimentsTest.cpp

+30 −0
Original line number Diff line number Diff line
@@ -59,6 +59,19 @@ class ExperimentsTest : public ::testing::Test { 
        EXPECT_THAT(mExperiments.mFlagsMapInt, ::testing::ContainerEq(sFakeFlagsMapInt));

    }



    void expectFlagsMapIntDefault() {

        std::lock_guard guard(mExperiments.mMutex);

        for (const auto& [key, value] : mExperiments.mFlagsMapInt) {

            EXPECT_EQ(value, Experiments::kFlagIntDefault);

        }

    }



    void expectGetDnsExperimentFlagIntDefault(int value) {

        for (const auto& key : Experiments::kExperimentFlagKeyList) {

            EXPECT_EQ(mExperiments.getFlag(key, value), value);

        }

    }



    void expectGetDnsExperimentFlagInt() {

        std::unordered_map<std::string_view, int> tempMap;

        for (const auto& key : Experiments::kExperimentFlagKeyList) {
@@ -78,6 +91,9 @@ class ExperimentsTest : public ::testing::Test { 
        std::lock_guard guard(mExperiments.mMutex);

        for (const auto& [key, value] : mExperiments.mFlagsMapInt) {

            std::string flagDump = fmt::format("{}: {}", key, value);

            if (value == Experiments::kFlagIntDefault) {

                flagDump = fmt::format("{}: UNSET", key);

            }

            SCOPED_TRACE(flagDump);

            size_t pos = dumpString.find(flagDump, startPos);

            EXPECT_NE(pos, std::string::npos);
@@ -110,6 +126,17 @@ TEST_F(ExperimentsTest, getDnsExperimentFlagInt) { 
    }

}



TEST_F(ExperimentsTest, getDnsExperimentFlagIntDefaultValue) {

    // Clear the map and make mExperiments initialized with our default int value.

    sFakeFlagsMapInt.clear();

    mExperiments.update();

    expectFlagsMapIntDefault();

    std::vector<int> testValues = {100, 50, 30, 5};

    for (int testValue : testValues) {

        expectGetDnsExperimentFlagIntDefault(testValue);

    }

}



TEST_F(ExperimentsTest, dump) {

    std::vector<int> testValues = {100, 37, 0, 30};

    for (int testValue : testValues) {
@@ -117,6 +144,9 @@ TEST_F(ExperimentsTest, dump) { 
        mExperiments.update();

        expectDumpOutput();

    }

    sFakeFlagsMapInt.clear();

    mExperiments.update();

    expectDumpOutput();

}



}  // namespace android::net