From c1b27cd5a7d3002282a0cf935a1bf2264445a372 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 1 Jun 2026 11:47:35 -0500 Subject: [PATCH 1/6] Limit upload batches by event count Enforce the OneCollector per-request event-count limit by applying maxEventsPerUpload when upload contexts are created. Expose the new configuration key through public C++, Android, and Objective-C surfaces, and cover the default and override behavior in TPM tests. Files changed: - lib/include/public/ILogConfiguration.hpp - lib/config/RuntimeConfig_Default.hpp - lib/tpm/TransmissionPolicyManager.cpp - tests/unittests/TransmissionPolicyManagerTests.cpp - lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogConfigurationKey.java - lib/android_build/app/src/androidTest/java/com/microsoft/applications/events/maesdktest/LogManagerDDVUnitTest.java - wrappers/obj-c/ODWLogConfiguration.h - wrappers/obj-c/ODWLogConfiguration.mm Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../maesdktest/LogManagerDDVUnitTest.java | 1 + .../events/LogConfigurationKey.java | 3 ++- lib/config/RuntimeConfig_Default.hpp | 2 +- lib/include/public/ILogConfiguration.hpp | 6 ++++- lib/tpm/TransmissionPolicyManager.cpp | 2 ++ .../TransmissionPolicyManagerTests.cpp | 27 ++++++++++++++++++- wrappers/obj-c/ODWLogConfiguration.h | 5 ++++ wrappers/obj-c/ODWLogConfiguration.mm | 5 ++++ 8 files changed, 47 insertions(+), 4 deletions(-) diff --git a/lib/android_build/app/src/androidTest/java/com/microsoft/applications/events/maesdktest/LogManagerDDVUnitTest.java b/lib/android_build/app/src/androidTest/java/com/microsoft/applications/events/maesdktest/LogManagerDDVUnitTest.java index 5f784a563..e87e17225 100644 --- a/lib/android_build/app/src/androidTest/java/com/microsoft/applications/events/maesdktest/LogManagerDDVUnitTest.java +++ b/lib/android_build/app/src/androidTest/java/com/microsoft/applications/events/maesdktest/LogManagerDDVUnitTest.java @@ -578,6 +578,7 @@ public void getDefaultConfig() { assertThat(tpm, is(notNullValue())); assertThat(tpm, isA(ILogConfiguration.class)); assertThat(tpm.getLong(LogConfigurationKey.CFG_INT_TPM_MAX_BLOB_BYTES), is(2L * 1024 * 1024)); + assertThat(tpm.getLong(LogConfigurationKey.CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD), is(1500L)); assertThat(tpm.getLong(LogConfigurationKey.CFG_INT_TPM_MAX_RETRY), is(5L)); assertThat(tpm.getBoolean(LogConfigurationKey.CFG_BOOL_TPM_CLOCK_SKEW_ENABLED), is(true)); ILogConfiguration compat = defaultConfig.getLogConfiguration(LogConfigurationKey.CFG_MAP_COMPAT); diff --git a/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogConfigurationKey.java b/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogConfigurationKey.java index 329f17680..0ab7f6ea1 100644 --- a/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogConfigurationKey.java +++ b/lib/android_build/maesdk/src/main/java/com/microsoft/applications/events/LogConfigurationKey.java @@ -166,6 +166,8 @@ public enum LogConfigurationKey { CFG_INT_TPM_MAX_BLOB_BYTES("maxBlobSize", Long.class), + CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD("maxEventsPerUpload", Long.class), + CFG_BOOL_SESSION_RESET_ENABLED("sessionResetEnabled", Boolean.class); private String key; @@ -184,4 +186,3 @@ public Class getValueType() { return valueType; } } - diff --git a/lib/config/RuntimeConfig_Default.hpp b/lib/config/RuntimeConfig_Default.hpp index 504aeefe3..ce0e9e078 100644 --- a/lib/config/RuntimeConfig_Default.hpp +++ b/lib/config/RuntimeConfig_Default.hpp @@ -68,6 +68,7 @@ namespace MAT_NS_BEGIN {CFG_MAP_TPM, { {CFG_INT_TPM_MAX_BLOB_BYTES, 2097152}, + {CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD, 1500}, {CFG_INT_TPM_MAX_RETRY, 5}, {CFG_BOOL_TPM_CLOCK_SKEW_ENABLED, true}, {CFG_STR_TPM_BACKOFF, "E,3000,300000,2,1"}, @@ -233,4 +234,3 @@ namespace MAT_NS_BEGIN } MAT_NS_END - diff --git a/lib/include/public/ILogConfiguration.hpp b/lib/include/public/ILogConfiguration.hpp index 1cb8103b8..47ff396e3 100644 --- a/lib/include/public/ILogConfiguration.hpp +++ b/lib/include/public/ILogConfiguration.hpp @@ -391,6 +391,11 @@ namespace MAT_NS_BEGIN /// static constexpr const char* const CFG_INT_TPM_MAX_BLOB_BYTES = "maxBlobSize"; + /// + /// TPM configuration: maximum number of events per upload request. A value of 0 means unlimited. + /// + static constexpr const char* const CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD = "maxEventsPerUpload"; + /// /// TPM configuration map /// @@ -471,4 +476,3 @@ namespace MAT_NS_BEGIN } MAT_NS_END #endif - diff --git a/lib/tpm/TransmissionPolicyManager.cpp b/lib/tpm/TransmissionPolicyManager.cpp index 83b82cf2a..82651e300 100644 --- a/lib/tpm/TransmissionPolicyManager.cpp +++ b/lib/tpm/TransmissionPolicyManager.cpp @@ -218,6 +218,7 @@ namespace MAT_NS_BEGIN { auto ctx = m_system.createEventsUploadContext(); ctx->requestedMinLatency = m_runningLatency; + ctx->requestedMaxCount = m_config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD]; addUpload(ctx); initiateUpload(ctx); } @@ -336,6 +337,7 @@ namespace MAT_NS_BEGIN { if (event->record.latency > EventLatency_RealTime) { auto ctx = m_system.createEventsUploadContext(); ctx->requestedMinLatency = event->record.latency; + ctx->requestedMaxCount = m_config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD]; addUpload(ctx); initiateUpload(ctx); return; diff --git a/tests/unittests/TransmissionPolicyManagerTests.cpp b/tests/unittests/TransmissionPolicyManagerTests.cpp index 6cbdb99f5..908e2ce96 100644 --- a/tests/unittests/TransmissionPolicyManagerTests.cpp +++ b/tests/unittests/TransmissionPolicyManagerTests.cpp @@ -246,6 +246,7 @@ TEST_F(TransmissionPolicyManagerTests, ImmediateIncomingEventStartsUploadImmedia ASSERT_THAT(upload, NotNull()); EXPECT_THAT(upload->requestedMinLatency, EventLatency_Max); + EXPECT_THAT(upload->requestedMaxCount, 1500u); } TEST_F(TransmissionPolicyManagerTests, UploadDoesNothingWhenPaused) @@ -288,13 +289,37 @@ TEST_F(TransmissionPolicyManagerTests, UploadInitiatesUpload) EventsUploadContextPtr upload; EXPECT_CALL(*this, resultInitiateUpload(_)) .WillOnce(SaveArg<0>(&upload)); - tpm.uploadAsync(EventLatency_Normal); + EXPECT_CALL(tpm, uploadAsync(_)).Times(AnyNumber()); + tpm.uploadAsyncParent(EventLatency_Normal); EXPECT_THAT(tpm.uploadScheduled(), false); EXPECT_THAT(upload, NotNull()); + EXPECT_THAT(upload->requestedMaxCount, 1500u); EXPECT_THAT(tpm.activeUploads(), Contains(upload)); } +TEST_F(TransmissionPolicyManagerTests, UploadUsesConfiguredMaxEventCount) +{ + auto& config = testing::getSystem().getConfig(); + unsigned previousMaxCount = config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD]; + config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD] = 3; + + tpm.uploadScheduled(true); + tpm.paused(false); + + EventsUploadContextPtr upload; + EXPECT_CALL(*this, resultInitiateUpload(_)) + .WillOnce(SaveArg<0>(&upload)); + EXPECT_CALL(tpm, uploadAsync(_)).Times(AnyNumber()); + tpm.uploadAsyncParent(EventLatency_Normal); + + unsigned requestedMaxCount = upload ? upload->requestedMaxCount : 0; + config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD] = previousMaxCount; + + ASSERT_THAT(upload, NotNull()); + EXPECT_THAT(requestedMaxCount, 3u); +} + TEST_F(TransmissionPolicyManagerTests, EmptyUploadCeasesUploadingForRunningLatencyNormal) { auto upload = tpm.fakeActiveUpload(EventLatency_Normal); diff --git a/wrappers/obj-c/ODWLogConfiguration.h b/wrappers/obj-c/ODWLogConfiguration.h index 3cbdaaa61..6f785394c 100644 --- a/wrappers/obj-c/ODWLogConfiguration.h +++ b/wrappers/obj-c/ODWLogConfiguration.h @@ -271,6 +271,11 @@ extern NSString * _Nonnull const ODWCFG_STR_TPM_BACKOFF; */ extern NSString * _Nonnull const ODWCFG_INT_TPM_MAX_BLOB_BYTES; +/*! + TPM configuration map +*/ +extern NSString * _Nonnull const ODWCFG_INT_TPM_MAX_EVENTS_PER_UPLOAD; + /*! TPM configuration map */ diff --git a/wrappers/obj-c/ODWLogConfiguration.mm b/wrappers/obj-c/ODWLogConfiguration.mm index d69ddf70c..2f3bc8c72 100644 --- a/wrappers/obj-c/ODWLogConfiguration.mm +++ b/wrappers/obj-c/ODWLogConfiguration.mm @@ -277,6 +277,11 @@ The minimum time (ms) between storage full notifications. */ NSString *const ODWCFG_INT_TPM_MAX_BLOB_BYTES = @"maxBlobSize"; +/*! + TPM configuration map +*/ +NSString *const ODWCFG_INT_TPM_MAX_EVENTS_PER_UPLOAD = @"maxEventsPerUpload"; + /*! TPM configuration map */ From 88396535d05a283ec1eb7c57cf388c2f143b3df8 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 1 Jun 2026 12:13:32 -0500 Subject: [PATCH 2/6] Add upload batch count integration test Add TPM coverage that routes an upload through StorageObserver, MemoryStorage, and Packager with maxEventsPerUpload set to 3. This proves that the configured event-count limit reaches package creation and leaves excess records queued for later batches. Files changed: - tests/unittests/TransmissionPolicyManagerTests.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../TransmissionPolicyManagerTests.cpp | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/unittests/TransmissionPolicyManagerTests.cpp b/tests/unittests/TransmissionPolicyManagerTests.cpp index 908e2ce96..77aee7c9a 100644 --- a/tests/unittests/TransmissionPolicyManagerTests.cpp +++ b/tests/unittests/TransmissionPolicyManagerTests.cpp @@ -10,6 +10,9 @@ #include "common/MockIBandwidthController.hpp" #include "tpm/TransmissionPolicyManager.hpp" #include "TransmitProfiles.hpp" +#include "offline/MemoryStorage.hpp" +#include "offline/StorageObserver.hpp" +#include "packager/Packager.hpp" using namespace testing; using namespace MAT; @@ -77,6 +80,7 @@ class TransmissionPolicyManagerTests : public StrictMock { RouteSink initiateUpload{this, &TransmissionPolicyManagerTests::resultInitiateUpload}; RouteSink allUploadsFinished{this, &TransmissionPolicyManagerTests::resultAllUploadsFinished}; + RouteSink packagedEvents{this, &TransmissionPolicyManagerTests::resultPackagedEvents}; protected: TransmissionPolicyManagerTests() @@ -88,6 +92,7 @@ class TransmissionPolicyManagerTests : public StrictMock { MOCK_METHOD1(resultInitiateUpload, void(EventsUploadContextPtr const &)); MOCK_METHOD0(resultAllUploadsFinished, void()); + MOCK_METHOD1(resultPackagedEvents, void(EventsUploadContextPtr const &)); virtual void SetUp() override { @@ -320,6 +325,58 @@ TEST_F(TransmissionPolicyManagerTests, UploadUsesConfiguredMaxEventCount) EXPECT_THAT(requestedMaxCount, 3u); } +TEST_F(TransmissionPolicyManagerTests, UploadPackagesNoMoreThanConfiguredMaxEventCount) +{ + auto& system = testing::getSystem(); + auto& config = system.getConfig(); + unsigned previousMaxCount = config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD]; + config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD] = 3; + + MemoryStorage storage(system.getLogManager(), config); + StorageObserver storageObserver(system, storage); + Packager packager(config); + + storageObserver.retrievedEvent >> packager.addEventToPackage; + storageObserver.retrievalFinished >> packager.finalizePackage; + packager.packagedEvents >> packagedEvents; + + ASSERT_THAT(storageObserver.start(), true); + for (unsigned i = 0; i < 10; ++i) + { + StorageRecord record( + "r" + std::to_string(i), + "tenant-token", + EventLatency_Normal, + EventPersistence_Normal, + 1234567890 + i, + std::vector{static_cast(i)}); + ASSERT_THAT(storage.StoreRecord(record), true); + } + + EventsUploadContextPtr packaged; + EXPECT_CALL(*this, resultInitiateUpload(_)) + .WillOnce(Invoke([&storageObserver](EventsUploadContextPtr const& ctx) { + storageObserver.retrieveEvents(ctx); + })); + EXPECT_CALL(*this, resultPackagedEvents(_)) + .WillOnce(SaveArg<0>(&packaged)); + + tpm.uploadScheduled(true); + tpm.paused(false); + EXPECT_CALL(tpm, uploadAsync(_)).Times(AnyNumber()); + tpm.uploadAsyncParent(EventLatency_Normal); + + config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD] = previousMaxCount; + + ASSERT_THAT(packaged, NotNull()); + EXPECT_THAT(packaged->requestedMaxCount, 3u); + EXPECT_THAT(packaged->recordIdsAndTenantIds, SizeIs(3)); + EXPECT_THAT(storage.LastReadRecordCount(), 3u); + EXPECT_THAT(storage.GetRecordCount(), 7u); + EXPECT_THAT(storage.GetReservedCount(), 3u); + EXPECT_THAT(packaged->fromMemory, true); +} + TEST_F(TransmissionPolicyManagerTests, EmptyUploadCeasesUploadingForRunningLatencyNormal) { auto upload = tpm.fakeActiveUpload(EventLatency_Normal); From dc8112385f1792d0220a2efeaf5adbf8a8e6cf1d Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 1 Jun 2026 12:47:02 -0500 Subject: [PATCH 3/6] Fix max-events test payloads for debug builds CI failure: the integration-style max-events test used fake byte blobs, which passed Release but tripped BondSplicer debug assertions because the records were not valid Bond-serialized payloads. Serialize real CsProtocol::Record payloads before storing records so the test exercises the same package path without debug/release divergence. Verified: - Win32 Debug UnitTests build and TransmissionPolicyManagerTests.* - x64 Release UnitTests build and TransmissionPolicyManagerTests.* Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/unittests/TransmissionPolicyManagerTests.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/unittests/TransmissionPolicyManagerTests.cpp b/tests/unittests/TransmissionPolicyManagerTests.cpp index 77aee7c9a..dbf082991 100644 --- a/tests/unittests/TransmissionPolicyManagerTests.cpp +++ b/tests/unittests/TransmissionPolicyManagerTests.cpp @@ -13,6 +13,8 @@ #include "offline/MemoryStorage.hpp" #include "offline/StorageObserver.hpp" #include "packager/Packager.hpp" +#include "bond/All.hpp" +#include "bond/generated/CsProtocol_writers.hpp" using namespace testing; using namespace MAT; @@ -343,13 +345,19 @@ TEST_F(TransmissionPolicyManagerTests, UploadPackagesNoMoreThanConfiguredMaxEven ASSERT_THAT(storageObserver.start(), true); for (unsigned i = 0; i < 10; ++i) { + ::CsProtocol::Record sourceRecord; + sourceRecord.name = "testEvent" + std::to_string(i); + std::vector recordBlob; + bond_lite::CompactBinaryProtocolWriter writer(recordBlob); + bond_lite::Serialize(writer, sourceRecord); + StorageRecord record( "r" + std::to_string(i), "tenant-token", EventLatency_Normal, EventPersistence_Normal, 1234567890 + i, - std::vector{static_cast(i)}); + std::move(recordBlob)); ASSERT_THAT(storage.StoreRecord(record), true); } From 7087ce8dcf027aee776b595170182c918a47eb87 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 1 Jun 2026 13:13:14 -0500 Subject: [PATCH 4/6] Address Copilot round 1 config restoration issue Comment 3336129493: the max-events tests modified the shared runtime configuration and manually restored it, which could leak state if a fatal ASSERT returned early. Add a scoped restoration helper and use it in both config-mutating tests so the previous value is restored on every exit path. Verified in tests/unittests/TransmissionPolicyManagerTests.cpp. Validation: - Win32 Debug UnitTests build and TransmissionPolicyManagerTests.* - x64 Release UnitTests build and TransmissionPolicyManagerTests.* Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../TransmissionPolicyManagerTests.cpp | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/tests/unittests/TransmissionPolicyManagerTests.cpp b/tests/unittests/TransmissionPolicyManagerTests.cpp index dbf082991..fd77ba824 100644 --- a/tests/unittests/TransmissionPolicyManagerTests.cpp +++ b/tests/unittests/TransmissionPolicyManagerTests.cpp @@ -20,6 +20,28 @@ using namespace testing; using namespace MAT; +class ScopedTpmMaxEventsPerUploadConfig { + public: + ScopedTpmMaxEventsPerUploadConfig(IRuntimeConfig& config, unsigned value) + : m_config(config), + m_previousMaxCount(config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD]) + { + m_config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD] = value; + } + + ~ScopedTpmMaxEventsPerUploadConfig() + { + m_config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD] = m_previousMaxCount; + } + + ScopedTpmMaxEventsPerUploadConfig(const ScopedTpmMaxEventsPerUploadConfig&) = delete; + ScopedTpmMaxEventsPerUploadConfig& operator=(const ScopedTpmMaxEventsPerUploadConfig&) = delete; + + private: + IRuntimeConfig& m_config; + unsigned m_previousMaxCount; +}; + class TransmissionPolicyManager4Test : public TransmissionPolicyManager { public: TransmissionPolicyManager4Test(ITelemetrySystem& system, IBandwidthController* bandwidthController) @@ -308,8 +330,8 @@ TEST_F(TransmissionPolicyManagerTests, UploadInitiatesUpload) TEST_F(TransmissionPolicyManagerTests, UploadUsesConfiguredMaxEventCount) { auto& config = testing::getSystem().getConfig(); - unsigned previousMaxCount = config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD]; - config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD] = 3; + ScopedTpmMaxEventsPerUploadConfig maxEventsConfig(config, 3); + UNREFERENCED_PARAMETER(maxEventsConfig); tpm.uploadScheduled(true); tpm.paused(false); @@ -321,7 +343,6 @@ TEST_F(TransmissionPolicyManagerTests, UploadUsesConfiguredMaxEventCount) tpm.uploadAsyncParent(EventLatency_Normal); unsigned requestedMaxCount = upload ? upload->requestedMaxCount : 0; - config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD] = previousMaxCount; ASSERT_THAT(upload, NotNull()); EXPECT_THAT(requestedMaxCount, 3u); @@ -331,8 +352,8 @@ TEST_F(TransmissionPolicyManagerTests, UploadPackagesNoMoreThanConfiguredMaxEven { auto& system = testing::getSystem(); auto& config = system.getConfig(); - unsigned previousMaxCount = config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD]; - config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD] = 3; + ScopedTpmMaxEventsPerUploadConfig maxEventsConfig(config, 3); + UNREFERENCED_PARAMETER(maxEventsConfig); MemoryStorage storage(system.getLogManager(), config); StorageObserver storageObserver(system, storage); @@ -374,8 +395,6 @@ TEST_F(TransmissionPolicyManagerTests, UploadPackagesNoMoreThanConfiguredMaxEven EXPECT_CALL(tpm, uploadAsync(_)).Times(AnyNumber()); tpm.uploadAsyncParent(EventLatency_Normal); - config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD] = previousMaxCount; - ASSERT_THAT(packaged, NotNull()); EXPECT_THAT(packaged->requestedMaxCount, 3u); EXPECT_THAT(packaged->recordIdsAndTenantIds, SizeIs(3)); From 2a6c05a8fa1434b9650bc3de13d16f7038a58c35 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 1 Jun 2026 13:33:25 -0500 Subject: [PATCH 5/6] Address Copilot round 2 test and wrapper comments Comments 3336266349/3336266435: Obj-C wrapper comments described the max-events key as a TPM map rather than a scalar key. Update both comments to describe max events per upload request and the 0=unlimited behavior. Comments 3336266468/3336266492/3336266521: redundant mock expectations implied uploadAsync should route through the mock even though these tests call uploadAsyncParent directly. Remove the no-op expectations. Validation: - Win32 Debug UnitTests build and TransmissionPolicyManagerTests.* - x64 Release UnitTests build and TransmissionPolicyManagerTests.* Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/unittests/TransmissionPolicyManagerTests.cpp | 5 +---- wrappers/obj-c/ODWLogConfiguration.h | 2 +- wrappers/obj-c/ODWLogConfiguration.mm | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/unittests/TransmissionPolicyManagerTests.cpp b/tests/unittests/TransmissionPolicyManagerTests.cpp index fd77ba824..889bfd8d7 100644 --- a/tests/unittests/TransmissionPolicyManagerTests.cpp +++ b/tests/unittests/TransmissionPolicyManagerTests.cpp @@ -318,7 +318,6 @@ TEST_F(TransmissionPolicyManagerTests, UploadInitiatesUpload) EventsUploadContextPtr upload; EXPECT_CALL(*this, resultInitiateUpload(_)) .WillOnce(SaveArg<0>(&upload)); - EXPECT_CALL(tpm, uploadAsync(_)).Times(AnyNumber()); tpm.uploadAsyncParent(EventLatency_Normal); EXPECT_THAT(tpm.uploadScheduled(), false); @@ -339,7 +338,6 @@ TEST_F(TransmissionPolicyManagerTests, UploadUsesConfiguredMaxEventCount) EventsUploadContextPtr upload; EXPECT_CALL(*this, resultInitiateUpload(_)) .WillOnce(SaveArg<0>(&upload)); - EXPECT_CALL(tpm, uploadAsync(_)).Times(AnyNumber()); tpm.uploadAsyncParent(EventLatency_Normal); unsigned requestedMaxCount = upload ? upload->requestedMaxCount : 0; @@ -389,10 +387,9 @@ TEST_F(TransmissionPolicyManagerTests, UploadPackagesNoMoreThanConfiguredMaxEven })); EXPECT_CALL(*this, resultPackagedEvents(_)) .WillOnce(SaveArg<0>(&packaged)); - tpm.uploadScheduled(true); tpm.paused(false); - EXPECT_CALL(tpm, uploadAsync(_)).Times(AnyNumber()); + tpm.paused(false); tpm.uploadAsyncParent(EventLatency_Normal); ASSERT_THAT(packaged, NotNull()); diff --git a/wrappers/obj-c/ODWLogConfiguration.h b/wrappers/obj-c/ODWLogConfiguration.h index 6f785394c..c176ff96a 100644 --- a/wrappers/obj-c/ODWLogConfiguration.h +++ b/wrappers/obj-c/ODWLogConfiguration.h @@ -272,7 +272,7 @@ extern NSString * _Nonnull const ODWCFG_STR_TPM_BACKOFF; extern NSString * _Nonnull const ODWCFG_INT_TPM_MAX_BLOB_BYTES; /*! - TPM configuration map + TPM configuration: maximum events per upload request. Set to 0 for unlimited. */ extern NSString * _Nonnull const ODWCFG_INT_TPM_MAX_EVENTS_PER_UPLOAD; diff --git a/wrappers/obj-c/ODWLogConfiguration.mm b/wrappers/obj-c/ODWLogConfiguration.mm index 2f3bc8c72..ab44d409f 100644 --- a/wrappers/obj-c/ODWLogConfiguration.mm +++ b/wrappers/obj-c/ODWLogConfiguration.mm @@ -278,7 +278,7 @@ The minimum time (ms) between storage full notifications. NSString *const ODWCFG_INT_TPM_MAX_BLOB_BYTES = @"maxBlobSize"; /*! - TPM configuration map + TPM configuration: maximum events per upload request. Set to 0 for unlimited. */ NSString *const ODWCFG_INT_TPM_MAX_EVENTS_PER_UPLOAD = @"maxEventsPerUpload"; From f73902afa790a5258b222438b398c414cfe9c3d2 Mon Sep 17 00:00:00 2001 From: Bhagirath Mehta Date: Mon, 1 Jun 2026 13:50:36 -0500 Subject: [PATCH 6/6] Address Copilot round 3 max-count edge cases Comment 3336387520: the scoped test helper restored the prior config as unsigned, which could fail to restore the exact Variant value. Store and restore the previous value as Variant. Comments 3336387612/3336387635: maxEventsPerUpload converted Variant to unsigned directly in both upload paths. Add one helper that treats non-positive values as 0/unlimited and caps oversized values at unsigned max, then use it for scheduled and immediate uploads. Comment 3336387579: remove the duplicate paused(false) call in the batching test. Validation: - Win32 Debug UnitTests build and TransmissionPolicyManagerTests.* (43 tests) - x64 Release UnitTests build and TransmissionPolicyManagerTests.* (43 tests) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/tpm/TransmissionPolicyManager.cpp | 18 +++++- .../TransmissionPolicyManagerTests.cpp | 64 ++++++++++++++++++- 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/lib/tpm/TransmissionPolicyManager.cpp b/lib/tpm/TransmissionPolicyManager.cpp index 82651e300..b84dd2410 100644 --- a/lib/tpm/TransmissionPolicyManager.cpp +++ b/lib/tpm/TransmissionPolicyManager.cpp @@ -41,6 +41,20 @@ namespace MAT_NS_BEGIN { return (a > b) ? (a - b) : (b - a); } + unsigned getMaxEventsPerUpload(IRuntimeConfig& config) + { + const int64_t configuredMaxCount = config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD]; + if (configuredMaxCount <= 0) + { + return 0; + } + if (configuredMaxCount > static_cast(std::numeric_limits::max())) + { + return std::numeric_limits::max(); + } + return static_cast(configuredMaxCount); + } + MATSDK_LOG_INST_COMPONENT_CLASS(TransmissionPolicyManager, "EventsSDK.TPM", "Events telemetry client - TransmissionPolicyManager class") TransmissionPolicyManager::TransmissionPolicyManager(ITelemetrySystem& system, ITaskDispatcher& taskDispatcher, IBandwidthController* bandwidthController) : @@ -218,7 +232,7 @@ namespace MAT_NS_BEGIN { auto ctx = m_system.createEventsUploadContext(); ctx->requestedMinLatency = m_runningLatency; - ctx->requestedMaxCount = m_config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD]; + ctx->requestedMaxCount = getMaxEventsPerUpload(m_config); addUpload(ctx); initiateUpload(ctx); } @@ -337,7 +351,7 @@ namespace MAT_NS_BEGIN { if (event->record.latency > EventLatency_RealTime) { auto ctx = m_system.createEventsUploadContext(); ctx->requestedMinLatency = event->record.latency; - ctx->requestedMaxCount = m_config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD]; + ctx->requestedMaxCount = getMaxEventsPerUpload(m_config); addUpload(ctx); initiateUpload(ctx); return; diff --git a/tests/unittests/TransmissionPolicyManagerTests.cpp b/tests/unittests/TransmissionPolicyManagerTests.cpp index 889bfd8d7..36e671012 100644 --- a/tests/unittests/TransmissionPolicyManagerTests.cpp +++ b/tests/unittests/TransmissionPolicyManagerTests.cpp @@ -16,13 +16,15 @@ #include "bond/All.hpp" #include "bond/generated/CsProtocol_writers.hpp" +#include + using namespace testing; using namespace MAT; class ScopedTpmMaxEventsPerUploadConfig { public: - ScopedTpmMaxEventsPerUploadConfig(IRuntimeConfig& config, unsigned value) + ScopedTpmMaxEventsPerUploadConfig(IRuntimeConfig& config, Variant value) : m_config(config), m_previousMaxCount(config[CFG_MAP_TPM][CFG_INT_TPM_MAX_EVENTS_PER_UPLOAD]) { @@ -39,7 +41,7 @@ class ScopedTpmMaxEventsPerUploadConfig { private: IRuntimeConfig& m_config; - unsigned m_previousMaxCount; + Variant m_previousMaxCount; }; class TransmissionPolicyManager4Test : public TransmissionPolicyManager { @@ -278,6 +280,26 @@ TEST_F(TransmissionPolicyManagerTests, ImmediateIncomingEventStartsUploadImmedia EXPECT_THAT(upload->requestedMaxCount, 1500u); } +TEST_F(TransmissionPolicyManagerTests, ImmediateIncomingEventTreatsNonPositiveMaxEventCountAsUnlimited) +{ + auto& config = testing::getSystem().getConfig(); + ScopedTpmMaxEventsPerUploadConfig maxEventsConfig(config, static_cast(-1)); + UNREFERENCED_PARAMETER(maxEventsConfig); + + tpm.paused(false); + + auto event = new IncomingEventContext(); + event->record.latency = EventLatency_Max; + EventsUploadContextPtr upload; + EXPECT_CALL(*this, resultInitiateUpload(_)) + .WillOnce(SaveArg<0>(&upload)); + tpm.eventArrived(event); + + ASSERT_THAT(upload, NotNull()); + EXPECT_THAT(upload->requestedMinLatency, EventLatency_Max); + EXPECT_THAT(upload->requestedMaxCount, 0u); +} + TEST_F(TransmissionPolicyManagerTests, UploadDoesNothingWhenPaused) { tpm.uploadScheduled(true); @@ -346,6 +368,43 @@ TEST_F(TransmissionPolicyManagerTests, UploadUsesConfiguredMaxEventCount) EXPECT_THAT(requestedMaxCount, 3u); } +TEST_F(TransmissionPolicyManagerTests, UploadTreatsNonPositiveMaxEventCountAsUnlimited) +{ + auto& config = testing::getSystem().getConfig(); + ScopedTpmMaxEventsPerUploadConfig maxEventsConfig(config, static_cast(-1)); + UNREFERENCED_PARAMETER(maxEventsConfig); + + tpm.uploadScheduled(true); + tpm.paused(false); + + EventsUploadContextPtr upload; + EXPECT_CALL(*this, resultInitiateUpload(_)) + .WillOnce(SaveArg<0>(&upload)); + tpm.uploadAsyncParent(EventLatency_Normal); + + ASSERT_THAT(upload, NotNull()); + EXPECT_THAT(upload->requestedMaxCount, 0u); +} + +TEST_F(TransmissionPolicyManagerTests, UploadCapsOversizedMaxEventCount) +{ + auto& config = testing::getSystem().getConfig(); + int64_t oversizedMaxCount = static_cast(std::numeric_limits::max()) + 1; + ScopedTpmMaxEventsPerUploadConfig maxEventsConfig(config, oversizedMaxCount); + UNREFERENCED_PARAMETER(maxEventsConfig); + + tpm.uploadScheduled(true); + tpm.paused(false); + + EventsUploadContextPtr upload; + EXPECT_CALL(*this, resultInitiateUpload(_)) + .WillOnce(SaveArg<0>(&upload)); + tpm.uploadAsyncParent(EventLatency_Normal); + + ASSERT_THAT(upload, NotNull()); + EXPECT_THAT(upload->requestedMaxCount, std::numeric_limits::max()); +} + TEST_F(TransmissionPolicyManagerTests, UploadPackagesNoMoreThanConfiguredMaxEventCount) { auto& system = testing::getSystem(); @@ -389,7 +448,6 @@ TEST_F(TransmissionPolicyManagerTests, UploadPackagesNoMoreThanConfiguredMaxEven .WillOnce(SaveArg<0>(&packaged)); tpm.uploadScheduled(true); tpm.paused(false); - tpm.paused(false); tpm.uploadAsyncParent(EventLatency_Normal); ASSERT_THAT(packaged, NotNull());