Updated JsonConfigLoader to parse access for area configs

If access is populated in VehicleAreaConfig, access field will remain
empty in VehiclePropConfig.

Updated VHAL layer tests to test new access mode granularity logic

Added new tests and modified others in JsonConfigLoaderUnitTest to test
new parsing logic.

Added VTS test to enforce exclusively one of VehiclePropConig.access and
the VehicleAreaConfig.access fields are populated.

Bug: 290801790
Test: atest JsonConfigLoaderUnitTest
Test: atest VtsHalAutomotiveVehicle_TargetTest
Change-Id: I6cc4f20c289141123d3e2093e45084109fbf4d9d
diff --git a/automotive/vehicle/aidl/impl/default_config/JsonConfigLoader/include/JsonConfigLoader.h b/automotive/vehicle/aidl/impl/default_config/JsonConfigLoader/include/JsonConfigLoader.h
index f3bdbd2..82e5860 100644
--- a/automotive/vehicle/aidl/impl/default_config/JsonConfigLoader/include/JsonConfigLoader.h
+++ b/automotive/vehicle/aidl/impl/default_config/JsonConfigLoader/include/JsonConfigLoader.h
@@ -130,12 +130,9 @@
                                      std::vector<T>* outPtr, std::vector<std::string>* errors);
     // Parses a JSON field to VehiclePropertyAccess or VehiclePropertyChangeMode.
     template <class T>
-    void parseAccessChangeMode(
-            const Json::Value& parentJsonNode, const std::string& fieldName, int propId,
-            const std::string& propStr,
-            const std::unordered_map<aidl::android::hardware::automotive::vehicle::VehicleProperty,
-                                     T>& defaultMap,
-            T* outPtr, std::vector<std::string>* errors);
+    void parseAccessChangeMode(const Json::Value& parentJsonNode, const std::string& fieldName,
+                               const std::string& propStr, const T* defaultAccessChangeModePtr,
+                               T* outPtr, std::vector<std::string>* errors);
 
     // Parses a JSON field to RawPropValues.
     //
@@ -145,8 +142,10 @@
                          std::vector<std::string>* errors);
 
     // Prase a JSON field as an array of area configs.
-    void parseAreas(const Json::Value& parentJsonNode, const std::string& fieldName,
-                    ConfigDeclaration* outPtr, std::vector<std::string>* errors);
+    void parseAreas(
+            const Json::Value& parentJsonNode, const std::string& fieldName,
+            ConfigDeclaration* outPtr, std::vector<std::string>* errors,
+            aidl::android::hardware::automotive::vehicle::VehiclePropertyAccess defaultAccess);
 };
 
 }  // namespace jsonconfigloader_impl
diff --git a/automotive/vehicle/aidl/impl/default_config/JsonConfigLoader/src/JsonConfigLoader.cpp b/automotive/vehicle/aidl/impl/default_config/JsonConfigLoader/src/JsonConfigLoader.cpp
index 3e6e7dc..76db891 100644
--- a/automotive/vehicle/aidl/impl/default_config/JsonConfigLoader/src/JsonConfigLoader.cpp
+++ b/automotive/vehicle/aidl/impl/default_config/JsonConfigLoader/src/JsonConfigLoader.cpp
@@ -487,10 +487,11 @@
 }
 
 template <class T>
-void JsonConfigParser::parseAccessChangeMode(
-        const Json::Value& parentJsonNode, const std::string& fieldName, int propId,
-        const std::string& propStr, const std::unordered_map<VehicleProperty, T>& defaultMap,
-        T* outPtr, std::vector<std::string>* errors) {
+void JsonConfigParser::parseAccessChangeMode(const Json::Value& parentJsonNode,
+                                             const std::string& fieldName,
+                                             const std::string& propStr,
+                                             const T* defaultAccessChangeModeValuePtr, T* outPtr,
+                                             std::vector<std::string>* errors) {
     if (!parentJsonNode.isObject()) {
         errors->push_back("Node: " + parentJsonNode.toStyledString() + " is not an object");
         return;
@@ -504,12 +505,11 @@
         *outPtr = static_cast<T>(result.value());
         return;
     }
-    auto it = defaultMap.find(static_cast<VehicleProperty>(propId));
-    if (it == defaultMap.end()) {
+    if (defaultAccessChangeModeValuePtr == NULL) {
         errors->push_back("No " + fieldName + " specified for property: " + propStr);
         return;
     }
-    *outPtr = it->second;
+    *outPtr = *defaultAccessChangeModeValuePtr;
     return;
 }
 
@@ -538,7 +538,8 @@
 }
 
 void JsonConfigParser::parseAreas(const Json::Value& parentJsonNode, const std::string& fieldName,
-                                  ConfigDeclaration* config, std::vector<std::string>* errors) {
+                                  ConfigDeclaration* config, std::vector<std::string>* errors,
+                                  VehiclePropertyAccess defaultAccess) {
     if (!parentJsonNode.isObject()) {
         errors->push_back("Node: " + parentJsonNode.toStyledString() + " is not an object");
         return;
@@ -546,6 +547,7 @@
     if (!parentJsonNode.isMember(fieldName)) {
         return;
     }
+    std::string propStr = parentJsonNode["property"].toStyledString();
     const Json::Value& jsonValue = parentJsonNode[fieldName];
 
     if (!jsonValue.isArray()) {
@@ -561,6 +563,8 @@
         }
         VehicleAreaConfig areaConfig = {};
         areaConfig.areaId = areaId;
+        parseAccessChangeMode(jsonAreaConfig, "access", propStr, &defaultAccess, &areaConfig.access,
+                              errors);
         tryParseJsonValueToVariable(jsonAreaConfig, "minInt32Value", /*optional=*/true,
                                     &areaConfig.minInt32Value, errors);
         tryParseJsonValueToVariable(jsonAreaConfig, "maxInt32Value", /*optional=*/true,
@@ -608,12 +612,21 @@
 
     configDecl.config.prop = propId;
     std::string propStr = propJsonValue["property"].toStyledString();
+    VehiclePropertyAccess* defaultAccessMode = NULL;
+    auto itAccess = AccessForVehicleProperty.find(static_cast<VehicleProperty>(propId));
+    if (itAccess != AccessForVehicleProperty.end()) {
+        defaultAccessMode = &itAccess->second;
+    }
+    VehiclePropertyChangeMode* defaultChangeMode = NULL;
+    auto itChangeMode = ChangeModeForVehicleProperty.find(static_cast<VehicleProperty>(propId));
+    if (itChangeMode != ChangeModeForVehicleProperty.end()) {
+        defaultChangeMode = &itChangeMode->second;
+    }
+    VehiclePropertyAccess access = VehiclePropertyAccess::NONE;
+    parseAccessChangeMode(propJsonValue, "access", propStr, defaultAccessMode, &access, errors);
 
-    parseAccessChangeMode(propJsonValue, "access", propId, propStr, AccessForVehicleProperty,
-                          &configDecl.config.access, errors);
-
-    parseAccessChangeMode(propJsonValue, "changeMode", propId, propStr,
-                          ChangeModeForVehicleProperty, &configDecl.config.changeMode, errors);
+    parseAccessChangeMode(propJsonValue, "changeMode", propStr, defaultChangeMode,
+                          &configDecl.config.changeMode, errors);
 
     tryParseJsonValueToVariable(propJsonValue, "configString", /*optional=*/true,
                                 &configDecl.config.configString, errors);
@@ -629,21 +642,23 @@
     tryParseJsonValueToVariable(propJsonValue, "maxSampleRate", /*optional=*/true,
                                 &configDecl.config.maxSampleRate, errors);
 
-    parseAreas(propJsonValue, "areas", &configDecl, errors);
-
-    if (errors->size() != initialErrorCount) {
-        return std::nullopt;
-    }
+    parseAreas(propJsonValue, "areas", &configDecl, errors, access);
 
     // If there is no area config, by default we allow variable update rate, so we have to add
     // a global area config.
     if (configDecl.config.areaConfigs.size() == 0) {
         VehicleAreaConfig areaConfig = {
                 .areaId = 0,
+                .access = access,
                 .supportVariableUpdateRate = true,
         };
         configDecl.config.areaConfigs.push_back(std::move(areaConfig));
     }
+
+    if (errors->size() != initialErrorCount) {
+        return std::nullopt;
+    }
+
     return configDecl;
 }
 
diff --git a/automotive/vehicle/aidl/impl/default_config/JsonConfigLoader/test/JsonConfigLoaderUnitTest.cpp b/automotive/vehicle/aidl/impl/default_config/JsonConfigLoader/test/JsonConfigLoaderUnitTest.cpp
index 9882653..a13d3df 100644
--- a/automotive/vehicle/aidl/impl/default_config/JsonConfigLoader/test/JsonConfigLoaderUnitTest.cpp
+++ b/automotive/vehicle/aidl/impl/default_config/JsonConfigLoader/test/JsonConfigLoaderUnitTest.cpp
@@ -286,7 +286,8 @@
     ASSERT_EQ(configs.size(), 1u);
 
     const VehiclePropConfig& propConfig = configs.begin()->second.config;
-    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::READ);
+    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::NONE);
+    ASSERT_EQ(propConfig.areaConfigs[0].access, VehiclePropertyAccess::READ);
     ASSERT_EQ(propConfig.changeMode, VehiclePropertyChangeMode::STATIC);
 }
 
@@ -307,7 +308,8 @@
     ASSERT_EQ(configs.size(), 1u);
 
     const VehiclePropConfig& propConfig = configs.begin()->second.config;
-    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::WRITE);
+    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::NONE);
+    ASSERT_EQ(propConfig.areaConfigs[0].access, VehiclePropertyAccess::WRITE);
     ASSERT_EQ(propConfig.changeMode, VehiclePropertyChangeMode::STATIC);
 }
 
@@ -328,7 +330,8 @@
     ASSERT_EQ(configs.size(), 1u);
 
     const VehiclePropConfig& propConfig = configs.begin()->second.config;
-    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::READ);
+    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::NONE);
+    ASSERT_EQ(propConfig.areaConfigs[0].access, VehiclePropertyAccess::READ);
     ASSERT_EQ(propConfig.changeMode, VehiclePropertyChangeMode::ON_CHANGE);
 }
 
@@ -350,7 +353,8 @@
     ASSERT_EQ(configs.size(), 1u);
 
     const VehiclePropConfig& propConfig = configs.begin()->second.config;
-    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::WRITE);
+    ASSERT_EQ(propConfig.access, VehiclePropertyAccess::NONE);
+    ASSERT_EQ(propConfig.areaConfigs[0].access, VehiclePropertyAccess::WRITE);
     ASSERT_EQ(propConfig.changeMode, VehiclePropertyChangeMode::ON_CHANGE);
 }
 
@@ -550,10 +554,12 @@
     ASSERT_EQ(configs.size(), 1u);
 
     const VehiclePropConfig& config = configs.begin()->second.config;
+    ASSERT_EQ(config.access, VehiclePropertyAccess::NONE);
     ASSERT_EQ(config.areaConfigs.size(), 1u);
     const VehicleAreaConfig& areaConfig = config.areaConfigs[0];
     ASSERT_EQ(areaConfig.minInt32Value, 1);
     ASSERT_EQ(areaConfig.maxInt32Value, 7);
+    ASSERT_EQ(areaConfig.access, VehiclePropertyAccess::READ);
     ASSERT_EQ(areaConfig.areaId, HVAC_ALL);
 }
 
@@ -635,9 +641,11 @@
     ASSERT_EQ(configs.size(), 1u);
 
     const VehiclePropConfig& config = configs.begin()->second.config;
+    ASSERT_EQ(config.access, VehiclePropertyAccess::NONE);
     ASSERT_EQ(config.areaConfigs.size(), 1u);
 
     const VehicleAreaConfig& areaConfig = config.areaConfigs[0];
+    ASSERT_EQ(areaConfig.access, VehiclePropertyAccess::READ);
     ASSERT_EQ(areaConfig.areaId, 0);
     ASSERT_FALSE(areaConfig.supportedEnumValues);
 }
@@ -662,9 +670,11 @@
     ASSERT_EQ(configs.size(), 1u);
 
     const VehiclePropConfig& config = configs.begin()->second.config;
+    ASSERT_EQ(config.access, VehiclePropertyAccess::NONE);
     ASSERT_EQ(config.areaConfigs.size(), 1u);
 
     const VehicleAreaConfig& areaConfig = config.areaConfigs[0];
+    ASSERT_EQ(areaConfig.access, VehiclePropertyAccess::READ);
     ASSERT_EQ(areaConfig.areaId, 0);
     ASSERT_TRUE(areaConfig.supportedEnumValues);
     ASSERT_EQ(areaConfig.supportedEnumValues.value().size(), 2u);
@@ -692,13 +702,107 @@
     ASSERT_EQ(configs.size(), 1u);
 
     const VehiclePropConfig& config = configs.begin()->second.config;
+    ASSERT_EQ(config.access, VehiclePropertyAccess::NONE);
     ASSERT_EQ(config.areaConfigs.size(), 1u);
 
     const VehicleAreaConfig& areaConfig = config.areaConfigs[0];
+    ASSERT_EQ(areaConfig.access, VehiclePropertyAccess::READ);
     ASSERT_EQ(areaConfig.areaId, 0);
     ASSERT_FALSE(areaConfig.supportedEnumValues);
 }
 
+TEST_F(JsonConfigLoaderUnitTest, testAccess_areaOverrideGlobalDefault) {
+    std::istringstream iss(R"(
+    {
+        "properties": [{
+            "property": "VehicleProperty::CABIN_LIGHTS_SWITCH",
+            "areas": [{
+                "access": "VehiclePropertyAccess::READ",
+                "areaId": 0
+            }]
+        }]
+    }
+    )");
+
+    auto result = mLoader.loadPropConfig(iss);
+    ASSERT_TRUE(result.ok());
+
+    auto configs = result.value();
+    ASSERT_EQ(configs.size(), 1u);
+
+    const VehiclePropConfig& config = configs.begin()->second.config;
+    ASSERT_EQ(config.access, VehiclePropertyAccess::NONE);
+    ASSERT_EQ(config.areaConfigs.size(), 1u);
+
+    const VehicleAreaConfig& areaConfig = config.areaConfigs[0];
+    ASSERT_EQ(areaConfig.access, VehiclePropertyAccess::READ);
+    ASSERT_EQ(areaConfig.areaId, 0);
+}
+
+TEST_F(JsonConfigLoaderUnitTest, testAccess_globalOverrideDefault) {
+    std::istringstream iss(R"(
+    {
+        "properties": [{
+            "property": "VehicleProperty::CABIN_LIGHTS_SWITCH",
+            "areas": [{
+                "areaId": 0
+            }],
+            "access": "VehiclePropertyAccess::READ"
+        }]
+    }
+    )");
+
+    auto result = mLoader.loadPropConfig(iss);
+    ASSERT_TRUE(result.ok());
+
+    auto configs = result.value();
+    ASSERT_EQ(configs.size(), 1u);
+
+    const VehiclePropConfig& config = configs.begin()->second.config;
+    ASSERT_EQ(config.access, VehiclePropertyAccess::NONE);
+    ASSERT_EQ(config.areaConfigs.size(), 1u);
+
+    const VehicleAreaConfig& areaConfig = config.areaConfigs[0];
+    ASSERT_EQ(areaConfig.access, VehiclePropertyAccess::READ);
+    ASSERT_EQ(areaConfig.areaId, 0);
+}
+
+TEST_F(JsonConfigLoaderUnitTest, testAccess_areaOverrideGlobal) {
+    std::istringstream iss(R"(
+    {
+        "properties": [{
+            "property": "VehicleProperty::CABIN_LIGHTS_SWITCH",
+            "areas": [{
+                "access": "VehiclePropertyAccess::WRITE",
+                "areaId": 0
+            },
+            {
+                "areaId": 1
+            }],
+            "access": "VehiclePropertyAccess::READ",
+        }]
+    }
+    )");
+
+    auto result = mLoader.loadPropConfig(iss);
+    ASSERT_TRUE(result.ok());
+
+    auto configs = result.value();
+    ASSERT_EQ(configs.size(), 1u);
+
+    const VehiclePropConfig& config = configs.begin()->second.config;
+    ASSERT_EQ(config.access, VehiclePropertyAccess::NONE);
+    ASSERT_EQ(config.areaConfigs.size(), 2u);
+
+    const VehicleAreaConfig& areaConfig1 = config.areaConfigs[0];
+    ASSERT_EQ(areaConfig1.access, VehiclePropertyAccess::WRITE);
+    ASSERT_EQ(areaConfig1.areaId, 0);
+
+    const VehicleAreaConfig& areaConfig2 = config.areaConfigs[1];
+    ASSERT_EQ(areaConfig2.access, VehiclePropertyAccess::READ);
+    ASSERT_EQ(areaConfig2.areaId, 1);
+}
+
 }  // namespace vehicle
 }  // namespace automotive
 }  // namespace hardware
diff --git a/automotive/vehicle/vts/src/VtsHalAutomotiveVehicle_TargetTest.cpp b/automotive/vehicle/vts/src/VtsHalAutomotiveVehicle_TargetTest.cpp
index b5ee335..b0e44ae 100644
--- a/automotive/vehicle/vts/src/VtsHalAutomotiveVehicle_TargetTest.cpp
+++ b/automotive/vehicle/vts/src/VtsHalAutomotiveVehicle_TargetTest.cpp
@@ -119,6 +119,7 @@
     bool checkIsSupported(int32_t propertyId);
 
   public:
+    void verifyAccessMode(int actualAccess, int expectedAccess);
     void verifyProperty(VehicleProperty propId, VehiclePropertyAccess access,
                         VehiclePropertyChangeMode changeMode, VehiclePropertyGroup group,
                         VehicleArea area, VehiclePropertyType propertyType);
@@ -250,8 +251,13 @@
         const IHalPropConfig& cfg = *cfgPtr;
         int32_t propId = cfg.getPropId();
         // test on boolean and writable property
-        if (cfg.getAccess() == toInt(VehiclePropertyAccess::READ_WRITE) &&
-            isBooleanGlobalProp(propId) && !hvacProps.count(propId)) {
+        bool isReadWrite = (cfg.getAccess() == toInt(VehiclePropertyAccess::READ_WRITE));
+        if (cfg.getAreaConfigSize() != 0 &&
+            cfg.getAreaConfigs()[0]->getAccess() != toInt(VehiclePropertyAccess::NONE)) {
+            isReadWrite = (cfg.getAreaConfigs()[0]->getAccess() ==
+                           toInt(VehiclePropertyAccess::READ_WRITE));
+        }
+        if (isReadWrite && isBooleanGlobalProp(propId) && !hvacProps.count(propId)) {
             auto propToGet = mVhalClient->createHalPropValue(propId);
             auto getValueResult = mVhalClient->getValueSync(*propToGet);
 
@@ -469,6 +475,53 @@
     }
 }
 
+// Test that access mode is populated in exclusively one of the VehiclePropConfig or the
+// VehicleAreaConfigs. Either VehiclePropConfig.access must be populated, or all the
+// VehicleAreaConfig.access fields should be populated.
+TEST_P(VtsHalAutomotiveVehicleTargetTest, testAccessModeExclusivityAIDL) {
+    if (!mVhalClient->isAidlVhal()) {
+        GTEST_SKIP() << "Skip checking access mode for HIDL because the access mode field is only "
+                        "present for AIDL";
+    }
+
+    auto result = mVhalClient->getAllPropConfigs();
+    ASSERT_TRUE(result.ok());
+    for (const auto& cfgPtr : result.value()) {
+        const IHalPropConfig& cfg = *cfgPtr;
+
+        bool propAccessIsSet = (cfg.getAccess() != toInt(VehiclePropertyAccess::NONE));
+        bool unsetAreaAccessExists = false;
+        bool setAreaAccessExists = false;
+
+        for (const auto& areaConfig : cfg.getAreaConfigs()) {
+            if (areaConfig->getAccess() == toInt(VehiclePropertyAccess::NONE)) {
+                unsetAreaAccessExists = true;
+            } else {
+                setAreaAccessExists = true;
+            }
+        }
+
+        ASSERT_FALSE(propAccessIsSet && setAreaAccessExists) << StringPrintf(
+                "Both prop and area config access is set for propertyId %d", cfg.getPropId());
+        ASSERT_FALSE(!propAccessIsSet && !setAreaAccessExists) << StringPrintf(
+                "Neither prop and area config access is set for propertyId %d", cfg.getPropId());
+        ASSERT_FALSE(unsetAreaAccessExists && setAreaAccessExists) << StringPrintf(
+                "Area access is only set in some configs for propertyId %d", cfg.getPropId());
+    }
+}
+
+void VtsHalAutomotiveVehicleTargetTest::verifyAccessMode(int actualAccess, int expectedAccess) {
+    if (expectedAccess == toInt(VehiclePropertyAccess::READ_WRITE)) {
+        ASSERT_TRUE(actualAccess == expectedAccess ||
+                    actualAccess == toInt(VehiclePropertyAccess::READ))
+                << StringPrintf("Expect to get VehiclePropertyAccess: %i or %i, got %i",
+                                expectedAccess, toInt(VehiclePropertyAccess::READ), actualAccess);
+        return;
+    }
+    ASSERT_EQ(actualAccess, expectedAccess) << StringPrintf(
+            "Expect to get VehiclePropertyAccess: %i, got %i", expectedAccess, actualAccess);
+}
+
 // Helper function to compare actual vs expected property config
 void VtsHalAutomotiveVehicleTargetTest::verifyProperty(VehicleProperty propId,
                                                        VehiclePropertyAccess access,
@@ -511,7 +564,6 @@
 
     const auto& config = result.value().at(0);
     int actualPropId = config->getPropId();
-    int actualAccess = config->getAccess();
     int actualChangeMode = config->getChangeMode();
     int actualGroup = actualPropId & toInt(VehiclePropertyGroup::MASK);
     int actualArea = actualPropId & toInt(VehicleArea::MASK);
@@ -520,14 +572,17 @@
     ASSERT_EQ(actualPropId, expectedPropId)
             << StringPrintf("Expect to get property ID: %i, got %i", expectedPropId, actualPropId);
 
-    if (expectedAccess == toInt(VehiclePropertyAccess::READ_WRITE)) {
-        ASSERT_TRUE(actualAccess == expectedAccess ||
-                    actualAccess == toInt(VehiclePropertyAccess::READ))
-                << StringPrintf("Expect to get VehiclePropertyAccess: %i or %i, got %i",
-                                expectedAccess, toInt(VehiclePropertyAccess::READ), actualAccess);
+    int globalAccess = config->getAccess();
+    if (config->getAreaConfigSize() == 0) {
+        verifyAccessMode(globalAccess, expectedAccess);
     } else {
-        ASSERT_EQ(actualAccess, expectedAccess) << StringPrintf(
-                "Expect to get VehiclePropertyAccess: %i, got %i", expectedAccess, actualAccess);
+        for (const auto& areaConfig : config->getAreaConfigs()) {
+            int areaConfigAccess = areaConfig->getAccess();
+            int actualAccess = (areaConfigAccess != toInt(VehiclePropertyAccess::NONE))
+                                       ? areaConfigAccess
+                                       : globalAccess;
+            verifyAccessMode(actualAccess, expectedAccess);
+        }
     }
 
     ASSERT_EQ(actualChangeMode, expectedChangeMode)