From 3972029814a6228e2a1521a8e5bb4a9052f0bf5c Mon Sep 17 00:00:00 2001
From: yunhanw-google <yunhanw@google.com>
Date: Mon, 6 Dec 2021 16:20:44 -0800
Subject: [PATCH] Remove EventSchema and put ConcreteEventPath  within Event
 Option (#12633)

---
 src/app/ConcreteEventPath.h              |  2 +-
 src/app/EventLoggingTypes.h              | 35 ++++------------
 src/app/EventManagement.cpp              | 53 +++++++++++-------------
 src/app/tests/TestEventLogging.cpp       | 33 +++++++--------
 src/app/tests/TestReadInteraction.cpp    | 13 +++---
 src/app/tests/integration/MockEvents.cpp |  6 +--
 6 files changed, 57 insertions(+), 85 deletions(-)

diff --git a/src/app/ConcreteEventPath.h b/src/app/ConcreteEventPath.h
index 819f4928927222..73e4a748b391fc 100644
--- a/src/app/ConcreteEventPath.h
+++ b/src/app/ConcreteEventPath.h
@@ -34,7 +34,7 @@ struct ConcreteEventPath
 
     ConcreteEventPath() {}
 
-    ConcreteEventPath & operator=(ConcreteEventPath && other)
+    ConcreteEventPath & operator=(const ConcreteEventPath & other)
     {
         if (&other == this)
             return *this;
diff --git a/src/app/EventLoggingTypes.h b/src/app/EventLoggingTypes.h
index 15585fcd899f12..8ef91c418bba8c 100644
--- a/src/app/EventLoggingTypes.h
+++ b/src/app/EventLoggingTypes.h
@@ -86,22 +86,6 @@ enum class PriorityLevel : uint8_t
 static_assert(sizeof(std::underlying_type_t<PriorityLevel>) <= sizeof(unsigned),
               "Logging that converts PriorityLevel to unsigned will be lossy");
 
-/**
- * @brief
- *   The structure that provides a full resolution of the cluster.
- */
-struct EventSchema
-{
-    EventSchema(NodeId aNodeId, EndpointId aEndpointId, ClusterId aClusterId, EventId aEventId, PriorityLevel aPriority) :
-        mNodeId(aNodeId), mEndpointId(aEndpointId), mClusterId(aClusterId), mEventId(aEventId), mPriority(aPriority)
-    {}
-    NodeId mNodeId          = 0;
-    EndpointId mEndpointId  = 0;
-    ClusterId mClusterId    = 0;
-    EventId mEventId        = 0;
-    PriorityLevel mPriority = PriorityLevel::Invalid;
-};
-
 /**
  * @brief
  *   The struct that provides an application set System or Epoch timestamp.
@@ -146,19 +130,16 @@ class EventOptions
         kUrgent = 0,
         kNotUrgent,
     };
-    EventOptions(void) : mpEventSchema(nullptr), mUrgent(Type::kNotUrgent) {}
-
-    EventOptions(Type aType) : mpEventSchema(nullptr), mUrgent(aType) {}
+    EventOptions() : mPriority(PriorityLevel::Invalid), mUrgent(Type::kNotUrgent) {}
+    EventOptions(Timestamp aTimestamp) : mTimestamp(aTimestamp), mPriority(PriorityLevel::Invalid), mUrgent(Type::kNotUrgent) {}
 
-    EventOptions(Timestamp aTimestamp) : mTimestamp(aTimestamp), mpEventSchema(nullptr), mUrgent(Type::kNotUrgent) {}
-
-    EventOptions(Timestamp aTimestamp, Type aUrgent) : mTimestamp(aTimestamp), mpEventSchema(nullptr), mUrgent(aUrgent) {}
+    EventOptions(Timestamp aTimestamp, Type aUrgent) : mTimestamp(aTimestamp), mPriority(PriorityLevel::Invalid), mUrgent(aUrgent)
+    {}
+    ConcreteEventPath mPath;
     Timestamp mTimestamp;
-
-    EventSchema * mpEventSchema = nullptr; /**< A pointer to the schema of the cluster instance.*/
-
-    Type mUrgent = Type::kNotUrgent; /**< A flag denoting if the event is time sensitive.  When kUrgent is set, it causes
-                                            the event log to be flushed. */
+    PriorityLevel mPriority = PriorityLevel::Invalid;
+    Type mUrgent            = Type::kNotUrgent; /**< A flag denoting if the event is time sensitive.  When kUrgent is set, it causes
+                                                       the event log to be flushed. */
 };
 
 /**
diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp
index a84d694cde6007..b59f26e9917057 100644
--- a/src/app/EventManagement.cpp
+++ b/src/app/EventManagement.cpp
@@ -93,7 +93,6 @@ struct EventEnvelopeContext
     Timestamp mDeltaTime = Timestamp::Epoch(System::Clock::kZero);
 #endif
     PriorityLevel mPriority = PriorityLevel::First;
-    NodeId mNodeId          = 0;
     ClusterId mClusterId    = 0;
     EndpointId mEndpointId  = 0;
     EventId mEventId        = 0;
@@ -278,8 +277,8 @@ CHIP_ERROR EventManagement::CalculateEventSize(EventLoggingDelegate * apDelegate
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
     System::PacketBufferTLVWriter writer;
-    EventLoadOutContext ctxt       = EventLoadOutContext(writer, apOptions->mpEventSchema->mPriority,
-                                                   GetPriorityBuffer(apOptions->mpEventSchema->mPriority)->GetLastEventNumber());
+    EventLoadOutContext ctxt =
+        EventLoadOutContext(writer, apOptions->mPriority, GetPriorityBuffer(apOptions->mPriority)->GetLastEventNumber());
     System::PacketBufferHandle buf = System::PacketBufferHandle::New(kMaxEventSizeReserve);
     if (buf.IsNull())
     {
@@ -287,8 +286,8 @@ CHIP_ERROR EventManagement::CalculateEventSize(EventLoggingDelegate * apDelegate
     }
     writer.Init(std::move(buf));
 
-    ctxt.mCurrentEventNumber = GetPriorityBuffer(apOptions->mpEventSchema->mPriority)->GetLastEventNumber();
-    ctxt.mCurrentTime.mValue = GetPriorityBuffer(apOptions->mpEventSchema->mPriority)->GetLastEventTimestamp();
+    ctxt.mCurrentEventNumber = GetPriorityBuffer(apOptions->mPriority)->GetLastEventNumber();
+    ctxt.mCurrentTime.mValue = GetPriorityBuffer(apOptions->mPriority)->GetLastEventTimestamp();
 
     TLVWriter checkpoint = ctxt.mWriter;
     err                  = ConstructEvent(&ctxt, apDelegate, apOptions);
@@ -327,11 +326,9 @@ CHIP_ERROR EventManagement::ConstructEvent(EventLoadOutContext * apContext, Even
     EventPathIB::Builder & eventPathBuilder = eventDataIBBuilder.CreatePath();
     ReturnErrorOnFailure(eventDataIBBuilder.GetError());
 
-    // TODO: Revisit NodeId since the the encoding spec and the IM seem to disagree on how this stuff works
-    eventPathBuilder.Node(apOptions->mpEventSchema->mNodeId)
-        .Endpoint(apOptions->mpEventSchema->mEndpointId)
-        .Cluster(apOptions->mpEventSchema->mClusterId)
-        .Event(apOptions->mpEventSchema->mEventId)
+    eventPathBuilder.Endpoint(apOptions->mPath.mEndpointId)
+        .Cluster(apOptions->mPath.mClusterId)
+        .Event(apOptions->mPath.mEventId)
         .IsUrgent(false)
         .EndOfEventPathIB();
     ReturnErrorOnFailure(eventPathBuilder.GetError());
@@ -483,8 +480,6 @@ CHIP_ERROR EventManagement::LogEvent(EventLoggingDelegate * apDelegate, EventOpt
 #endif // !CHIP_SYSTEM_CONFIG_NO_LOCKING
 
         VerifyOrExit(mState != EventManagementStates::Shutdown, err = CHIP_ERROR_INCORRECT_STATE);
-        VerifyOrExit(aEventOptions.mpEventSchema != nullptr, err = CHIP_ERROR_INCORRECT_STATE);
-
         err = LogEventPrivate(apDelegate, aEventOptions, aEventNumber);
     }
 exit:
@@ -500,8 +495,8 @@ CHIP_ERROR EventManagement::LogEventPrivate(EventLoggingDelegate * apDelegate, E
     aEventNumber                   = 0;
     CircularEventBuffer checkpoint = *mpEventBuffer;
     CircularEventBuffer * buffer   = nullptr;
-    EventLoadOutContext ctxt       = EventLoadOutContext(writer, aEventOptions.mpEventSchema->mPriority,
-                                                   GetPriorityBuffer(aEventOptions.mpEventSchema->mPriority)->GetLastEventNumber());
+    EventLoadOutContext ctxt =
+        EventLoadOutContext(writer, aEventOptions.mPriority, GetPriorityBuffer(aEventOptions.mPriority)->GetLastEventNumber());
     EventOptions opts;
 #if CHIP_CONFIG_EVENT_LOGGING_UTC_TIMESTAMPS & CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_TIME
     Timestamp timestamp;
@@ -519,23 +514,24 @@ CHIP_ERROR EventManagement::LogEventPrivate(EventLoggingDelegate * apDelegate, E
     writer.Init(*mpEventBuffer);
 
     // check whether the entry is to be logged or discarded silently
-    VerifyOrExit(aEventOptions.mpEventSchema->mPriority >= CHIP_CONFIG_EVENT_GLOBAL_PRIORITY, /* no-op */);
+    VerifyOrExit(aEventOptions.mPriority >= CHIP_CONFIG_EVENT_GLOBAL_PRIORITY, /* no-op */);
 
+    opts.mPriority = aEventOptions.mPriority;
     // Create all event specific data
     // Timestamp; encoded as a delta time
 
     opts.mTimestamp = aEventOptions.mTimestamp;
 
-    if (GetPriorityBuffer(aEventOptions.mpEventSchema->mPriority)->GetFirstEventTimestamp() == 0)
+    if (GetPriorityBuffer(aEventOptions.mPriority)->GetFirstEventTimestamp() == 0)
     {
-        GetPriorityBuffer(aEventOptions.mpEventSchema->mPriority)->UpdateFirstLastEventTime(opts.mTimestamp);
+        GetPriorityBuffer(aEventOptions.mPriority)->UpdateFirstLastEventTime(opts.mTimestamp);
     }
 
-    opts.mUrgent       = aEventOptions.mUrgent;
-    opts.mpEventSchema = aEventOptions.mpEventSchema;
+    opts.mUrgent = aEventOptions.mUrgent;
+    opts.mPath   = aEventOptions.mPath;
 
-    ctxt.mCurrentEventNumber = GetPriorityBuffer(opts.mpEventSchema->mPriority)->GetLastEventNumber();
-    ctxt.mCurrentTime.mValue = GetPriorityBuffer(opts.mpEventSchema->mPriority)->GetLastEventTimestamp();
+    ctxt.mCurrentEventNumber = GetPriorityBuffer(opts.mPriority)->GetLastEventNumber();
+    ctxt.mCurrentTime.mValue = GetPriorityBuffer(opts.mPriority)->GetLastEventTimestamp();
 
     err = CalculateEventSize(apDelegate, &opts, requestSize);
     SuccessOrExit(err);
@@ -553,7 +549,7 @@ CHIP_ERROR EventManagement::LogEventPrivate(EventLoggingDelegate * apDelegate, E
     while (true)
     {
         VerifyOrExit(buffer->GetTotalDataLength() >= writer.GetLengthWritten(), err = CHIP_ERROR_BUFFER_TOO_SMALL);
-        if (buffer->IsFinalDestinationForPriority(opts.mpEventSchema->mPriority))
+        if (buffer->IsFinalDestinationForPriority(opts.mPriority))
         {
             break;
         }
@@ -572,9 +568,9 @@ CHIP_ERROR EventManagement::LogEventPrivate(EventLoggingDelegate * apDelegate, E
     {
         *mpEventBuffer = checkpoint;
     }
-    else if (opts.mpEventSchema->mPriority >= CHIP_CONFIG_EVENT_GLOBAL_PRIORITY)
+    else if (opts.mPriority >= CHIP_CONFIG_EVENT_GLOBAL_PRIORITY)
     {
-        CircularEventBuffer * currentBuffer = GetPriorityBuffer(opts.mpEventSchema->mPriority);
+        CircularEventBuffer * currentBuffer = GetPriorityBuffer(opts.mPriority);
         aEventNumber                        = currentBuffer->VendEventNumber();
         currentBuffer->UpdateFirstLastEventTime(opts.mTimestamp);
 
@@ -582,14 +578,14 @@ CHIP_ERROR EventManagement::LogEventPrivate(EventLoggingDelegate * apDelegate, E
         ChipLogDetail(EventLogging,
                       "LogEvent event number: 0x" ChipLogFormatX64 " schema priority: %u cluster id: " ChipLogFormatMEI
                       " event id: 0x%" PRIx32 " %s timestamp: 0x" ChipLogFormatX64,
-                      ChipLogValueX64(aEventNumber), static_cast<unsigned>(opts.mpEventSchema->mPriority),
-                      ChipLogValueMEI(opts.mpEventSchema->mClusterId), opts.mpEventSchema->mEventId,
-                      opts.mTimestamp.mType == Timestamp::Type::kSystem ? "Sys" : "Epoch", ChipLogValueX64(opts.mTimestamp.mValue));
+                      ChipLogValueX64(aEventNumber), static_cast<unsigned>(opts.mPriority), ChipLogValueMEI(opts.mPath.mClusterId),
+                      opts.mPath.mEventId, opts.mTimestamp.mType == Timestamp::Type::kSystem ? "Sys" : "Epoch",
+                      ChipLogValueX64(opts.mTimestamp.mValue));
 #endif // CHIP_CONFIG_EVENT_LOGGING_VERBOSE_DEBUG_LOGS
 
         if (opts.mUrgent == EventOptions::Type::kUrgent)
         {
-            ConcreteEventPath path(opts.mpEventSchema->mEndpointId, opts.mpEventSchema->mClusterId, opts.mpEventSchema->mEventId);
+            ConcreteEventPath path(opts.mPath.mEndpointId, opts.mPath.mClusterId, opts.mPath.mEventId);
             err = InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleUrgentEventDelivery(path);
         }
     }
@@ -771,7 +767,6 @@ CHIP_ERROR EventManagement::FetchEventParameters(const TLVReader & aReader, size
     {
         EventPathIB::Parser path;
         ReturnErrorOnFailure(path.Init(aReader));
-        ReturnErrorOnFailure(path.GetNode(&(envelope->mNodeId)));
         ReturnErrorOnFailure(path.GetEndpoint(&(envelope->mEndpointId)));
         ReturnErrorOnFailure(path.GetCluster(&(envelope->mClusterId)));
         ReturnErrorOnFailure(path.GetEvent(&(envelope->mEventId)));
diff --git a/src/app/tests/TestEventLogging.cpp b/src/app/tests/TestEventLogging.cpp
index 34efc81e1113fb..5a0cee4a39ade8 100644
--- a/src/app/tests/TestEventLogging.cpp
+++ b/src/app/tests/TestEventLogging.cpp
@@ -126,7 +126,7 @@ static void CheckLogState(nlTestSuite * apSuite, chip::app::EventManagement & aL
     NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
 
     NL_TEST_ASSERT(apSuite, elementCount == expectedNumEvents);
-    printf("Num Events: %zu\n", elementCount);
+    printf("elementCount vs expectedNumEvents : %zu vs %zu \n", elementCount, expectedNumEvents);
 }
 
 static void CheckLogReadOut(nlTestSuite * apSuite, chip::app::EventManagement & alogMgmt, chip::app::PriorityLevel priority,
@@ -171,16 +171,14 @@ static void CheckLogEventWithEvictToNextBuffer(nlTestSuite * apSuite, void * apC
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
     chip::EventNumber eid1, eid2, eid3, eid4, eid5, eid6;
-    chip::app::EventSchema schema1 = { kTestDeviceNodeId1, kTestEndpointId1, kLivenessClusterId, kLivenessChangeEvent,
-                                       chip::app::PriorityLevel::Info };
-    chip::app::EventSchema schema2 = { kTestDeviceNodeId1, kTestEndpointId2, kLivenessClusterId, kLivenessChangeEvent,
-                                       chip::app::PriorityLevel::Info };
     chip::app::EventOptions options1;
     chip::app::EventOptions options2;
     TestEventGenerator testEventGenerator;
 
-    options1.mpEventSchema               = &schema1;
-    options2.mpEventSchema               = &schema2;
+    options1.mPath                       = { kTestEndpointId1, kLivenessClusterId, kLivenessChangeEvent };
+    options1.mPriority                   = chip::app::PriorityLevel::Info;
+    options2.mPath                       = { kTestEndpointId2, kLivenessClusterId, kLivenessChangeEvent };
+    options2.mPriority                   = chip::app::PriorityLevel::Info;
     chip::app::EventManagement & logMgmt = chip::app::EventManagement::GetInstance();
     testEventGenerator.SetStatus(0);
     err = logMgmt.LogEvent(&testEventGenerator, options1, eid1);
@@ -240,41 +238,42 @@ static void CheckLogEventWithDiscardLowEvent(nlTestSuite * apSuite, void * apCon
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
     chip::EventNumber eid1, eid2, eid3, eid4, eid5, eid6;
-    chip::app::EventSchema schema = { kTestDeviceNodeId1, kTestEndpointId1, kLivenessClusterId, kLivenessChangeEvent,
-                                      chip::app::PriorityLevel::Debug };
     chip::app::EventOptions options;
+    options.mPath     = { kTestEndpointId1, kLivenessClusterId, kLivenessChangeEvent };
+    options.mPriority = chip::app::PriorityLevel::Debug;
     TestEventGenerator testEventGenerator;
 
-    options.mpEventSchema = &schema;
-
     chip::app::EventManagement & logMgmt = chip::app::EventManagement::GetInstance();
     testEventGenerator.SetStatus(0);
     err = logMgmt.LogEvent(&testEventGenerator, options, eid1);
     NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
-    CheckLogState(apSuite, logMgmt, 3, chip::app::PriorityLevel::Debug);
+    CheckLogState(apSuite, logMgmt, 4, chip::app::PriorityLevel::Debug);
     testEventGenerator.SetStatus(1);
     err = logMgmt.LogEvent(&testEventGenerator, options, eid2);
     NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
-    CheckLogState(apSuite, logMgmt, 3, chip::app::PriorityLevel::Debug);
+    CheckLogState(apSuite, logMgmt, 4, chip::app::PriorityLevel::Debug);
     testEventGenerator.SetStatus(0);
     err = logMgmt.LogEvent(&testEventGenerator, options, eid3);
     NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
-    CheckLogState(apSuite, logMgmt, 3, chip::app::PriorityLevel::Debug);
+    CheckLogState(apSuite, logMgmt, 4, chip::app::PriorityLevel::Debug);
+    CheckLogState(apSuite, logMgmt, 8, chip::app::PriorityLevel::Info);
     // Start to drop off debug event since debug event can only be saved in debug buffer
     testEventGenerator.SetStatus(1);
     err = logMgmt.LogEvent(&testEventGenerator, options, eid4);
     NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
-    CheckLogState(apSuite, logMgmt, 3, chip::app::PriorityLevel::Debug);
+    CheckLogState(apSuite, logMgmt, 4, chip::app::PriorityLevel::Debug);
+    CheckLogState(apSuite, logMgmt, 8, chip::app::PriorityLevel::Info);
 
     testEventGenerator.SetStatus(0);
     err = logMgmt.LogEvent(&testEventGenerator, options, eid5);
     NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
-    CheckLogState(apSuite, logMgmt, 3, chip::app::PriorityLevel::Debug);
+    CheckLogState(apSuite, logMgmt, 4, chip::app::PriorityLevel::Debug);
+    CheckLogState(apSuite, logMgmt, 8, chip::app::PriorityLevel::Info);
 
     testEventGenerator.SetStatus(1);
     err = logMgmt.LogEvent(&testEventGenerator, options, eid6);
     NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
-    CheckLogState(apSuite, logMgmt, 3, chip::app::PriorityLevel::Debug);
+    CheckLogState(apSuite, logMgmt, 4, chip::app::PriorityLevel::Debug);
 }
 /**
  *   Test Suite. It lists all the test functions.
diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp
index d9ec63db51b8b3..b03ca4e0264471 100644
--- a/src/app/tests/TestReadInteraction.cpp
+++ b/src/app/tests/TestReadInteraction.cpp
@@ -48,7 +48,6 @@ uint8_t gDebugEventBuffer[128];
 uint8_t gInfoEventBuffer[128];
 uint8_t gCritEventBuffer[128];
 chip::app::CircularEventBuffer gCircularEventBuffer[3];
-chip::NodeId kTestNodeId              = 1;
 chip::ClusterId kTestClusterId        = 6;
 chip::ClusterId kInvalidTestClusterId = 7;
 chip::EndpointId kTestEndpointId      = 1;
@@ -110,16 +109,14 @@ void GenerateEvents(nlTestSuite * apSuite, void * apContext, bool aIsUrgent = fa
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
     chip::EventNumber eid1, eid2;
-    chip::app::EventSchema schema1 = { kTestNodeId, kTestEndpointId, kTestClusterId, kTestEventIdDebug,
-                                       chip::app::PriorityLevel::Info };
-    chip::app::EventSchema schema2 = { kTestNodeId, kTestEndpointId, kTestClusterId, kTestEventIdCritical,
-                                       chip::app::PriorityLevel::Critical };
     chip::app::EventOptions options1;
+    options1.mPath     = { kTestEndpointId, kTestClusterId, kTestEventIdDebug };
+    options1.mPriority = chip::app::PriorityLevel::Info;
+
     chip::app::EventOptions options2;
+    options2.mPath     = { kTestEndpointId, kTestClusterId, kTestEventIdCritical };
+    options2.mPriority = chip::app::PriorityLevel::Critical;
     TestEventGenerator testEventGenerator;
-
-    options1.mpEventSchema = &schema1;
-    options2.mpEventSchema = &schema2;
     if (aIsUrgent)
     {
         options2.mUrgent = chip::app::EventOptions::Type::kUrgent;
diff --git a/src/app/tests/integration/MockEvents.cpp b/src/app/tests/integration/MockEvents.cpp
index 1792050d68922d..fc0c0d0c341c42 100644
--- a/src/app/tests/integration/MockEvents.cpp
+++ b/src/app/tests/integration/MockEvents.cpp
@@ -121,10 +121,10 @@ chip::EventNumber LivenessEventGenerator::LogLiveness(chip::NodeId aNodeId, chip
 {
     chip::app::EventManagement & logManager = chip::app::EventManagement::GetInstance();
     chip::EventNumber number                = 0;
-    chip::app::EventSchema schema           = { aNodeId, aEndpointId, kTestClusterId, aEventId, aPriorityLevel };
     chip::app::EventOptions options;
-    mStatus               = static_cast<int32_t>(aStatus);
-    options.mpEventSchema = &schema;
+    options.mPath     = { aEndpointId, kTestClusterId, aEventId };
+    options.mPriority = aPriorityLevel;
+    mStatus           = static_cast<int32_t>(aStatus);
     logManager.LogEvent(this, options, number);
     return number;
 }