Address review comments.

src/darwin/Framework/CHIP/MTRBaseDevice.mm

@@ -23,6 +23,7 @@
 #import "MTRCluster.h"
 #import "MTRClusterStateCacheContainer_Internal.h"
 #import "MTRCluster_Internal.h"
+#import "MTRDeviceDataValidation.h"
 #import "MTRDevice_Internal.h"
 #import "MTRError_Internal.h"
 #import "MTREventTLVValueDecoder_Internal.h"

src/darwin/Framework/CHIP/MTRBaseDevice_Internal.h

@@ -18,7 +18,7 @@
 #import "MTRBaseDevice.h"
 #import <Foundation/Foundation.h>
 
-#import "MTRDeviceDataValueDictionary.h"
+#import "MTRDefines_Internal.h"
 
 #include <app/AttributePathParams.h>
 #include <app/ConcreteAttributePath.h>

src/darwin/Framework/CHIP/MTRDefines_Internal.h

@@ -65,6 +65,11 @@ typedef struct {} variable_hidden_by_mtr_hide;
 // Default timed interaction timeout, in ms, if another one is not provided.
 #define MTR_DEFAULT_TIMED_INTERACTION_TIMEOUT_MS 10000
 
+// Useful building block for type-checking machinery.  Uses C-style cast so it
+// can be used in .m files as well.
+#define MTR_SAFE_CAST(object, classname) \
+    ([object isKindOfClass:[classname class]] ? (classname *) (object) : nil)
+
 #pragma mark - XPC Defines
 
 #define MTR_SIMPLE_REMOTE_XPC_GETTER(XPC_CONNECTION, NAME, TYPE, DEFAULT_VALUE, GETTER_NAME, PREFIX)   \
@@ -179,3 +184,15 @@ typedef struct {} variable_hidden_by_mtr_hide;
 
 #define MTR_ABSTRACT_METHOD() \
     _MTR_ABSTRACT_METHOD_IMPL("%@ or some ancestor must implement %@", self.class, NSStringFromSelector(_cmd))
+
+#pragma mark - Typedefs for some commonly used types.
+
+/**
+ * A data-value as defined in MTRBaseDevice.h.
+ */
+typedef NSDictionary<NSString *, id> * MTRDeviceDataValueDictionary;
+
+/**
+ * A response-value as defined in MTRBaseDevice.h.
+ */
+typedef NSDictionary<NSString *, id> * MTRDeviceResponseValueDictionary;

src/darwin/Framework/CHIP/MTRDevice.mm

@@ -25,7 +25,6 @@
 #import "MTRConversion.h"
 #import "MTRDefines_Internal.h"
 #import "MTRDeviceController_Internal.h"
-#import "MTRDeviceDataValueDictionary.h"
 #import "MTRDevice_Internal.h"
 #import "MTRError_Internal.h"
 #import "MTRLogging_Internal.h"
@@ -759,248 +758,3 @@ - (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
 }
 
 @end
-
-static BOOL MTRArrayIsStructurallyValid(NSArray * input, NSArray * structure)
-{
-    if (structure.count != 1) {
-        // Unexpected structure.  Just claim NO; later we might give meaning to
-        // such things.
-        MTR_LOG_ERROR("Don't know how to make sense of array structure with more than one element: %@", structure);
-        return NO;
-    }
-
-    id elementStructure = structure[0];
-    for (id inputElement in input) {
-        if (!MTRInputIsStructurallyValid(inputElement, elementStructure)) {
-            MTR_LOG_ERROR("Array element not structurally valid: %@, %@", inputElement, elementStructure);
-            return NO;
-        }
-    }
-    return YES;
-}
-
-static BOOL MTRDictionaryIsStructurallyValid(NSDictionary * input, NSDictionary * structure)
-{
-    for (id key in structure) {
-        id inputValue = input[key];
-        if (!inputValue) {
-            MTR_LOG_ERROR("Input does not have key %@: %@, %@", key, input, structure);
-            return NO;
-        }
-        id valueStructure = structure[key];
-        if (!MTRInputIsStructurallyValid(inputValue, valueStructure)) {
-            MTR_LOG_ERROR("Dictionary value not structurally valid: %@, %@", inputValue, valueStructure);
-            return NO;
-        }
-    }
-
-    return YES;
-}
-
-static BOOL MTRAttributeReportIsStructurallyValid(id input)
-{
-    // Input is an array of values, but they could have slightly different
-    // structures, so we can't do a single MTRInputIsStructurallyValid to check
-    // that.  Check the items one at a time.
-    if (![input isKindOfClass:NSArray.class]) {
-        MTR_LOG_ERROR("Attribute report is not an array: %@", input);
-        return NO;
-    }
-
-    NSArray * inputArray = input;
-    for (id item in inputArray) {
-        // item can be a value report or an error report.
-        if (!MTRInputIsStructurallyValid(item, @ {
-                MTRAttributePathKey : MTRAttributePath.class,
-                MTRDataKey : MTRInputStructureDataValue,
-            })
-            && !MTRInputIsStructurallyValid(item, @ {
-                   MTRAttributePathKey : MTRAttributePath.class,
-                   MTRErrorKey : NSError.class,
-               })) {
-            MTR_LOG_ERROR("Attribute report contains a weird entry: %@", item);
-            return NO;
-        }
-
-        // Now we know item is in fact a dictionary, and it has at least one of MTRDataKey and MTRErrorKey.  Make sure it's
-        // not claiming both, which could confuse code that examines it.
-        NSDictionary * itemDictionary = item;
-        if (itemDictionary[MTRDataKey] != nil && itemDictionary[MTRErrorKey] != nil) {
-            MTR_LOG_ERROR("Attribute report contains an entry that claims to be both data and error: %@", item);
-            return NO;
-        }
-    }
-
-    return YES;
-}
-
-static BOOL MTREventReportIsStructurallyValid(id input)
-{
-    // Input is an array of values, but they could have slightly different
-    // structures, so we can't do a single MTRInputIsStructurallyValid to check
-    // that.  Check the items one at a time.
-    if (![input isKindOfClass:NSArray.class]) {
-        MTR_LOG_ERROR("Event report is not an array: %@", input);
-        return NO;
-    }
-
-    NSArray * inputArray = input;
-    for (id item in inputArray) {
-        // item can be a value report or an error report.
-        // MTREventIsHistoricalKey is claimed to be present no matter what, as
-        // long as MTREventPathKey is present.
-        if (!MTRInputIsStructurallyValid(item, @ {
-                MTREventPathKey : MTREventPath.class,
-                MTRDataKey : MTRInputStructureDataValue,
-                MTREventIsHistoricalKey : NSNumber.class,
-            })
-            && !MTRInputIsStructurallyValid(item, @ {
-                   MTREventPathKey : MTREventPath.class,
-                   MTRErrorKey : NSError.class,
-                   MTREventIsHistoricalKey : NSNumber.class,
-               })) {
-            MTR_LOG_ERROR("Event report contains a weird entry: %@", item);
-            return NO;
-        }
-
-        // Now we know item is in fact a dictionary, and it has at least one of MTRDataKey and MTRErrorKey.  Make sure it's
-        // not claiming both, which could confuse code that examines it.
-        NSDictionary * itemDictionary = item;
-        if (itemDictionary[MTRDataKey] != nil && itemDictionary[MTRErrorKey] != nil) {
-            MTR_LOG_ERROR("Event report contains an entry that claims to be both data and error: %@", item);
-            return NO;
-        }
-
-        if (itemDictionary[MTRDataKey]) {
-            // In this case, the structure is supposed to contain some more
-            // values.
-            if (!MTRInputIsStructurallyValid(itemDictionary, @ {
-                    MTREventNumberKey : NSNumber.class,
-                    MTREventPriorityKey : NSNumber.class,
-                    MTREventTimeTypeKey : NSNumber.class,
-                })) {
-                MTR_LOG_ERROR("Event report fields that depend on MTRDataKey failed validation: %@", itemDictionary);
-                return NO;
-            }
-
-            // And check well-formedness of our timestamps.
-            uint64_t eventTimeType = [itemDictionary[MTREventTimeTypeKey] unsignedLongLongValue];
-            switch (eventTimeType) {
-            case MTREventTimeTypeSystemUpTime: {
-                if (!MTRInputIsStructurallyValid(itemDictionary, @ { MTREventSystemUpTimeKey : NSNumber.class })) {
-                    MTR_LOG_ERROR("Event report claims system uptime timing but does not have the time: %@", itemDictionary);
-                    return NO;
-                }
-                break;
-            }
-            case MTREventTimeTypeTimestampDate: {
-                if (!MTRInputIsStructurallyValid(itemDictionary, @ { MTREventTimestampDateKey : NSDate.class })) {
-                    MTR_LOG_ERROR("Event report claims epoch timing but does not have the time: %@", itemDictionary);
-                    return NO;
-                }
-                break;
-            }
-            default:
-                MTR_LOG_ERROR("Uknown time type for event report: %@", itemDictionary);
-                return NO;
-            }
-        }
-    }
-
-    return YES;
-}
-
-static BOOL MTRInvokeResponseIsStructurallyValid(id input)
-{
-    // Input is an array with a single value that must have MTRCommandPathKey.
-    if (!MTRInputIsStructurallyValid(input, @[ @ { MTRCommandPathKey : MTRCommandPath.class } ])) {
-        MTR_LOG_ERROR("Invoke response is not an array with the right things in it: %@", input);
-        return NO;
-    }
-
-    NSArray * inputArray = input;
-    if (inputArray.count != 1) {
-        MTR_LOG_ERROR("Invoke response is not an array with exactly one entry: %@", input);
-        return NO;
-    }
-
-    // Note that we have already checked the one entry is a dictionary that has
-    // MTRCommandPathKey.
-    NSDictionary * responseValue = inputArray[0];
-    id data = responseValue[MTRDataKey];
-    id error = responseValue[MTRErrorKey];
-
-    if (data != nil && error != nil) {
-        MTR_LOG_ERROR("Invoke response claims to have both data and error: %@", responseValue);
-        return NO;
-    }
-
-    if (error != nil) {
-        return MTRInputIsStructurallyValid(error, NSError.class);
-    }
-
-    if (data == nil) {
-        // This is valid: indicates a success status response.
-        return YES;
-    }
-
-    if (!MTRInputIsStructurallyValid(data, MTRInputStructureDataValue)) {
-        MTR_LOG_ERROR("Invoke response claims to have data that is not a data-value: %@", data);
-        return NO;
-    }
-
-    // Now we know data is a dictionary (in fact a data-value).  The only thing
-    // we promise about it is that it has type MTRStructureValueType.
-    NSDictionary * dataDictionary = data;
-    if (dataDictionary[MTRTypeKey] != MTRStructureValueType) {
-        MTR_LOG_ERROR("Invoke response data is not of structure type: %@", data);
-        return NO;
-    }
-
-    return YES;
-}
-
-BOOL MTRInputIsStructurallyValid(id input, id structure)
-{
-    if ([structure isEqual:MTRInputStructureDataValue]) {
-        return MTRDataValueDictionaryIsWellFormed(input);
-    }
-
-    if ([structure isEqual:MTRInputStructureAttributeReport]) {
-        return MTRAttributeReportIsStructurallyValid(input);
-    }
-
-    if ([structure isEqual:MTRInputStructureEventReport]) {
-        return MTREventReportIsStructurallyValid(input);
-    }
-
-    if ([structure isEqual:MTRInputStructureInvokeResponse]) {
-        return MTRInvokeResponseIsStructurallyValid(input);
-    }
-
-    // Literal dictionaries and arrays actually have classes that are not
-    // exactly NSArray.class and NSDictionary.class, so checking isKindOfClass
-    // against [structure class] needs to be avoided.
-    if ([structure isKindOfClass:NSDictionary.class] &&
-        [input isKindOfClass:NSDictionary.class]) {
-        return MTRDictionaryIsStructurallyValid(input, structure);
-    }
-
-    if ([structure isKindOfClass:NSArray.class] &&
-        [input isKindOfClass:NSArray.class]) {
-        return MTRArrayIsStructurallyValid(input, structure);
-    }
-
-    if ([structure class] != structure) {
-        // Not a class object, not sure what to do with it.
-        MTR_LOG_ERROR("Unknown structure value: %@", structure);
-        return NO;
-    }
-
-    if (![input isKindOfClass:structure]) {
-        MTR_LOG_ERROR("Input not of the right class: %@, %@", input, structure);
-        return NO;
-    }
-
-    return YES;
-}

src/darwin/Framework/CHIP/MTRDeviceClusterData.h

@@ -17,7 +17,6 @@
 #import <Foundation/Foundation.h>
 
 #import "MTRDefines_Internal.h"
-#import "MTRDeviceDataValueDictionary.h"
 
 NS_ASSUME_NONNULL_BEGIN
 

src/darwin/Framework/CHIP/MTRDeviceDataValidation.h

@@ -0,0 +1,42 @@
+/**
+ *    Copyright (c) 2024 Project CHIP Authors
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+
+#import <Foundation/Foundation.h>
+
+#import "MTRDefines_Internal.h"
+
+NS_ASSUME_NONNULL_BEGIN
+
+// Returns whether a data-value dictionary is well-formed (in the sense that all
+// the types of the objects inside are as expected, so it's actually a valid
+// representation of some TLV).  Implemented in MTRBaseDevice.mm because that's
+// where the pieces needed to implement it are, but declared here so our tests
+// can see it.
+MTR_EXTERN MTR_TESTABLE BOOL MTRDataValueDictionaryIsWellFormed(MTRDeviceDataValueDictionary value);
+
+// Returns whether the provided attribute report actually has the right sorts of
+// objects in the right places.
+MTR_EXTERN MTR_TESTABLE BOOL MTRAttributeReportIsWellFormed(NSArray<MTRDeviceResponseValueDictionary> * report);
+
+// Returns whether the provided event report actually has the right sorts of
+// objects in the right places.
+MTR_EXTERN MTR_TESTABLE BOOL MTREventReportIsWellFormed(NSArray<MTRDeviceResponseValueDictionary> * report);
+
+// Returns whether the provided invoke response actually has the right sorts of
+// objects in the right places.
+MTR_EXTERN MTR_TESTABLE BOOL MTRInvokeResponseIsWellFormed(NSArray<MTRDeviceResponseValueDictionary> * response);
+
+NS_ASSUME_NONNULL_END