Implement validation of extended fields

ndsev · Dec 13, 2024 · 1998b97 · 1998b97
1 parent a7353f9
commit 1998b97
Show file tree

Hide file tree

Showing 10 changed files with 352 additions and 75 deletions.
diff --git a/extension/freemarker/Structure.cpp.ftl b/extension/freemarker/Structure.cpp.ftl
@@ -288,7 +288,51 @@ bool operator>=(const View<${fullName}>& lhs, const View<${fullName}>& rhs)
 }
 <@namespace_begin ["detail"]/>
 
-<#macro structure_check_constraint field indent=1>
+<#assign numExtendedFields=0/>
+<#macro structure_validate_field field indent>
+    <#local I>${""?left_pad(indent * 4)}</#local>
+    <#if field.isExtended>
+        <#if numExtendedFields == 0>
+${I}uint32_t numExtendedFields = 0;
+        </#if>
+        <#assign numExtendedFields++/>
+${I}if (view.${field.getterName}().isPresent())
+${I}{
+${I}    if (++numExtendedFields != ${numExtendedFields})
+${I}    {
+${I}        throw ExtendedFieldException(
+${I}                "Some of preceding extended fields is not present before '${name}.${field.name}'!");
+${I}    }
+        <@structure_validate_field_optional field, indent+1/>
+${I}}
+    <#else>
+    <@structure_validate_field_optional field, indent/>
+    </#if>
+</#macro>
+<#macro structure_validate_field_optional field indent>
+    <#local I>${""?left_pad(indent * 4)}</#local>
+    <#if field.optional??>
+        <#if field.optional.viewIndirectClause??>
+${I}// check non-auto optional
+${I}if ((${field.optional.viewIndirectClause}) && !view.${field.getterName}()<#if field.isExtended>.value()</#if>.has_value())
+${I}{
+${I}    throw MissedOptionalException("Optional field '${name}.${field.name}' is used but not set!");
+${I}}
+        </#if>
+${I}if (view.${field.getterName}()<#if field.isExtended>-><#else>.</#if>has_value())
+${I}{
+        <@structure_validate_field_inner field, indent+1/>
+${I}}
+    <#else>
+    <@structure_validate_field_inner field, indent/>
+    </#if>
+</#macro>
+<#macro structure_validate_field_inner field indent>
+    <#local I>${""?left_pad(indent * 4)}</#local>
+    <@structure_check_constraint field, indent/>
+${I}validate(<#if field.isExtended>*</#if><#if field.optional??>*</#if>view.${field.getterName}(), "'${name}.${field.name}'");
+</#macro>
+<#macro structure_check_constraint field indent>
     <#local I>${""?left_pad(indent * 4)}</#local>
     <#if field.constraint??>
 ${I}// check constraint
@@ -304,28 +348,9 @@ void validate(const View<${fullName}>&<#if fieldList?has_content || parameterLis
 <#list parameterList as parameter>
     validate(view.${parameter.getterName}(), "'${name}.${parameter.name}'");
 </#list>
-    <#if fieldList?has_content>(void)view;</#if>
-<#-- TODO[Mi-L@]: Consider extended fields!
 <#list fieldList as field>
-    <#if field.optional??>
-        <#if field.optional.viewIndirectClause??>
-    // check non-auto optional
-    if ((${field.optional.viewIndirectClause}) && !view.${field.getterName}())
-    {
-        throw MissedOptionalException("Optional field '${name}.${field.name}' is used but not set!");
-    }
-        </#if>
-    if (view.${field.getterName}())
-    {
-        <@structure_check_constraint field, 2/>
-        validate(*view.${field.getterName}(), "'${name}.${field.name}'");
-    }
-    <#else>
-    <@structure_check_constraint field/>
-    validate(view.${field.getterName}(), "'${name}.${field.name}'");
-    </#if>
+    <@structure_validate_field field, 1/>
 </#list>
--->
 }
 
 <#macro structure_bitsizeof_field_extended field indent>
@@ -505,7 +530,7 @@ View<${fullName}> read(BitStreamReader&<#if fieldList?has_content> reader</#if>,
             <#lt>);
 <#list fieldList as field>
     <@structure_read_field_extended fullName, field, 1/>
-    <@structure_check_constraint field/>
+    <@structure_check_constraint field, 1/>
 </#list>
     return view;
 }
@@ -577,7 +602,7 @@ void read(PackingContext<${fullName}>&<#if needs_packing_context(fieldList)> pac
             <#lt>);
 <#list fieldList as field>
     <@structure_read_field fullName, field, 1, true/>
-    <@structure_check_constraint field/>
+    <@structure_check_constraint field, 1/>
 </#list>
     (void)view;
 }

diff --git a/extension/src/zserio/extension/cpp17/CompoundFieldTemplateData.java b/extension/src/zserio/extension/cpp17/CompoundFieldTemplateData.java
@@ -44,6 +44,7 @@ public CompoundFieldTemplateData(TemplateDataContext context, CompoundType paren
         if (isExtended)
         {
             includeCollector.addHeaderSystemIncludes(Arrays.asList("zserio/Extended.h"));
+            includeCollector.addCppSystemIncludes(Arrays.asList("zserio/ExtendedFieldException.h"));
         }
         isPackable = field.isPackable();
 

diff --git a/runtime/src/CMakeLists.txt b/runtime/src/CMakeLists.txt
@@ -43,6 +43,7 @@ set(ZSERIO_CPP17_RUNTIME_LIB_SRCS
     zserio/ConstraintException.h
     zserio/DeltaContext.h
     zserio/Extended.h
+    zserio/ExtendedFieldException.h
     zserio/FileUtil.cpp
     zserio/FileUtil.h
     zserio/FloatUtil.cpp

diff --git a/runtime/src/zserio/ExtendedFieldException.h b/runtime/src/zserio/ExtendedFieldException.h
@@ -0,0 +1,20 @@
+#ifndef ZSERIO_EXTENDED_FIELD_EXCEPTION_H_INC
+#define ZSERIO_EXTENDED_FIELD_EXCEPTION_H_INC
+
+#include "zserio/ValidationException.h"
+
+namespace zserio
+{
+
+/**
+ * Exception thrown when a check of an extended field fails.
+ */
+class ExtendedFieldException : public ValidationException
+{
+public:
+    using ValidationException::ValidationException;
+};
+
+} // namespace zserio
+
+#endif // ifndef ZSERIO_EXTENDED_FIELD_EXCEPTION_H_INC
diff --git a/runtime/test/CMakeLists.txt b/runtime/test/CMakeLists.txt
@@ -36,6 +36,7 @@ set(ZSERIO_CPP17_RUNTIME_TEST_SRCS
     zserio/ConstraintExceptionTest.cpp
     zserio/DataViewTest.cpp
     zserio/EnumsTest.cpp
+    zserio/ExtendedFieldExceptionTest.cpp
     zserio/ExtendedTest.cpp
     zserio/FileUtilTest.cpp
     zserio/FloatUtilTest.cpp

diff --git a/runtime/test/zserio/ExtendedFieldExceptionTest.cpp b/runtime/test/zserio/ExtendedFieldExceptionTest.cpp
@@ -0,0 +1,17 @@
+#include "gtest/gtest.h"
+#include "zserio/ExtendedFieldException.h"
+
+namespace zserio
+{
+
+TEST(ExtendedFieldExceptionTest, correctTypeAfterAppend)
+{
+    ASSERT_THROW(
+            {
+                throw ExtendedFieldException()
+                        << "Test that appending using operator<< persists the exception type!";
+            },
+            ExtendedFieldException);
+}
+
+} // namespace zserio
diff --git a/test/language/extended_members/CMakeLists.txt b/test/language/extended_members/CMakeLists.txt
@@ -25,6 +25,7 @@ add_custom_test(extended_members
         ${CMAKE_CURRENT_SOURCE_DIR}/cpp/ExtendedOptionalParameterizedFieldTest.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/cpp/ExtendedPackedArrayTest.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/cpp/ExtendedSimpleFieldTest.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/cpp/MultipleExtendedFieldsTest.cpp
     GENERATED_SOURCES
         ${GENERATED_SOURCES}
 )