Skip to content

Commit

Permalink
Detect collisions and generate parse errors when they are encountered (
Browse files Browse the repository at this point in the history
…#2171)

Fix for #1852.

* source/shared/**
  * Introduce new `ParseContext.cpp` and `ParseContext.h` 
    * `ParseContext` encapsulates objects that need to be passed around at parse time
      * Add new `std::unordered_set<std::string>` to track ids
      * (previously: `ElementParserRegistration`, `ActionParserRegistration`, and `std::vector<AdaptiveParseWarning>`)
  * Various wiring to replace passing the old context items with `ParseContext`
  * New unit tests for single and nested duplicate id scenarios
  * Fixes to refrain from marking default dtors as virtual (cppcore)
  * Some `#include` cleanup
  * Reorder STL includes in `pch.h` for sanity's sake

* source/uwp/**
  * Update expected results for new failing tests
  * Add new files to shared model projection
  * Updates for new `Deserialize` signature

* samples/tests
  * tests for duplicate ids in single and nested scenarios

* source/android/**
  * Fixes to consume new header/object
  * SWIG'd changes

* source/dotnet/Library/adaptivecards/**
  * Introduce new `HashSet<string>` to track element ids
  * Add id tracking during [de]serialization (throw on collision)
  * Introduce a way to indicate when a card begins/ends parsing (needed to reset our id tracking member)
  * Account for expected failures for new tests

* source/ios/**
  * Publish/consume new cpp/h
  • Loading branch information
paulcam206 committed Nov 16, 2018
1 parent a312324 commit 97d01cf
Show file tree
Hide file tree
Showing 126 changed files with 2,016 additions and 2,203 deletions.
27 changes: 27 additions & 0 deletions samples/Tests/Action.DuplicateIds.json
@@ -0,0 +1,27 @@
{
"$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
"type": "AdaptiveCard",
"version": "1.0",
"body": [
{
"type": "TextBlock",
"text": "This card as duplicate ids and shouldn't render!"
},
{
"type": "Input.Text",
"id": "duplicate",
"placeholder": "uh-oh"
},
{
"type": "Input.Text",
"id": "duplicate",
"placeholder": "not again!"
}
],
"actions": [
{
"type": "Action.Submit",
"title": "Action.Submit"
}
]
}
45 changes: 45 additions & 0 deletions samples/Tests/Action.NestedDuplicateIds.json
@@ -0,0 +1,45 @@
{
"$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
"type": "AdaptiveCard",
"version": "1.0",
"body": [
{
"type": "Input.Text",
"placeholder": "Name",
"style": "text",
"maxLength": 0,
"id": "duplicate"
}
],
"actions": [
{
"type": "Action.Submit",
"title": "Submit",
"data": {
"id": "1234567890"
}
},
{
"type": "Action.ShowCard",
"title": "Show Card",
"card": {
"type": "AdaptiveCard",
"body": [
{
"type": "Input.Text",
"placeholder": "enter comment",
"style": "text",
"maxLength": 0,
"id": "duplicate"
}
],
"actions": [
{
"type": "Action.Submit",
"title": "OK"
}
]
}
}
]
}
1 change: 1 addition & 0 deletions source/android/adaptivecards/CMakeLists.txt
Expand Up @@ -51,6 +51,7 @@ add_library( # Sets the name of the library.
../../shared/cpp/ObjectModel/MediaSource.cpp
../../shared/cpp/ObjectModel/NumberInput.cpp
../../shared/cpp/ObjectModel/OpenUrlAction.cpp
../../shared/cpp/ObjectModel/ParseContext.cpp
../../shared/cpp/ObjectModel/ParseResult.cpp
../../shared/cpp/ObjectModel/ParseUtil.cpp
../../shared/cpp/ObjectModel/SemanticVersion.cpp
Expand Down
3 changes: 3 additions & 0 deletions source/android/adaptivecards/src/AdaptiveCardObjectModel.i
Expand Up @@ -84,6 +84,7 @@ struct tm {
#include "../../../shared/cpp/ObjectModel/OpenUrlAction.h"
#include "../../../shared/cpp/ObjectModel/ShowCardAction.h"
#include "../../../shared/cpp/ObjectModel/SubmitAction.h"
#include "../../../shared/cpp/ObjectModel/ParseContext.h"
#include "../../../shared/cpp/ObjectModel/ParseResult.h"
#include "../../../shared/cpp/ObjectModel/SharedAdaptiveCard.h"
#include "../../../shared/cpp/ObjectModel/AdaptiveCardParseException.h"
Expand Down Expand Up @@ -124,6 +125,7 @@ struct tm {
%shared_ptr(AdaptiveCards::ShowCardAction)
%shared_ptr(AdaptiveCards::SubmitAction)
%shared_ptr(AdaptiveCards::AdaptiveCardParseWarning)
%shared_ptr(AdaptiveCards::ParseContext)
%shared_ptr(AdaptiveCards::ParseResult)
%shared_ptr(AdaptiveCards::RemoteResourceInformation)
%shared_ptr(AdaptiveCards::AdaptiveCard)
Expand Down Expand Up @@ -599,6 +601,7 @@ namespace Json {
%include "../../../shared/cpp/ObjectModel/OpenUrlAction.h"
%include "../../../shared/cpp/ObjectModel/ShowCardAction.h"
%include "../../../shared/cpp/ObjectModel/SubmitAction.h"
%include "../../../shared/cpp/ObjectModel/ParseContext.h"
%include "../../../shared/cpp/ObjectModel/ParseResult.h"
%include "../../../shared/cpp/ObjectModel/SharedAdaptiveCard.h"
%include "../../../shared/cpp/ObjectModel/AdaptiveCardParseException.h"
Expand Down
2,084 changes: 909 additions & 1,175 deletions source/android/adaptivecards/src/main/cpp/objectmodel_wrap.cpp

Large diffs are not rendered by default.

11 changes: 7 additions & 4 deletions source/android/adaptivecards/src/main/cpp/objectmodel_wrap.h
Expand Up @@ -74,7 +74,8 @@ class SwigDirector_ActionElementParser : public AdaptiveCards::ActionElementPars
public:
void swig_connect_director(JNIEnv *jenv, jobject jself, jclass jcls, bool swig_mem_own, bool weak_global);
SwigDirector_ActionElementParser(JNIEnv *jenv);
virtual std::shared_ptr< AdaptiveCards::BaseActionElement > Deserialize(std::shared_ptr< AdaptiveCards::ElementParserRegistration > elementParserRegistration, std::shared_ptr< AdaptiveCards::ActionParserRegistration > actionParserRegistration, std::vector< std::shared_ptr< AdaptiveCards::AdaptiveCardParseWarning > > &warnings, Json::Value const &value);
virtual ~SwigDirector_ActionElementParser();
virtual std::shared_ptr< AdaptiveCards::BaseActionElement > Deserialize(AdaptiveCards::ParseContext &context, Json::Value const &value);
public:
bool swig_overrides(int n) {
return (n < 1 ? swig_override[n] : false);
Expand All @@ -88,13 +89,15 @@ class SwigDirector_BaseCardElementParser : public AdaptiveCards::BaseCardElement
public:
void swig_connect_director(JNIEnv *jenv, jobject jself, jclass jcls, bool swig_mem_own, bool weak_global);
SwigDirector_BaseCardElementParser(JNIEnv *jenv);
virtual std::shared_ptr< AdaptiveCards::BaseCardElement > Deserialize(std::shared_ptr< AdaptiveCards::ElementParserRegistration > elementParserRegistration, std::shared_ptr< AdaptiveCards::ActionParserRegistration > actionParserRegistration, std::vector< std::shared_ptr< AdaptiveCards::AdaptiveCardParseWarning > > &warnings, Json::Value const &value);
virtual ~SwigDirector_BaseCardElementParser();
virtual std::shared_ptr< AdaptiveCards::BaseCardElement > Deserialize(AdaptiveCards::ParseContext &context, Json::Value const &value);
virtual std::shared_ptr< AdaptiveCards::BaseCardElement > DeserializeFromString(AdaptiveCards::ParseContext &context, std::string const &value);
public:
bool swig_overrides(int n) {
return (n < 1 ? swig_override[n] : false);
return (n < 2 ? swig_override[n] : false);
}
protected:
Swig::BoolArray<1> swig_override;
Swig::BoolArray<2> swig_override;
};


Expand Down
Expand Up @@ -50,8 +50,8 @@ public void swigTakeOwnership() {
AdaptiveCardObjectModelJNI.ActionElementParser_change_ownership(this, swigCPtr, true);
}

public BaseActionElement Deserialize(ElementParserRegistration elementParserRegistration, ActionParserRegistration actionParserRegistration, AdaptiveCardParseWarningVector warnings, JsonValue value) {
long cPtr = AdaptiveCardObjectModelJNI.ActionElementParser_Deserialize(swigCPtr, this, ElementParserRegistration.getCPtr(elementParserRegistration), elementParserRegistration, ActionParserRegistration.getCPtr(actionParserRegistration), actionParserRegistration, AdaptiveCardParseWarningVector.getCPtr(warnings), warnings, JsonValue.getCPtr(value), value);
public BaseActionElement Deserialize(ParseContext context, JsonValue value) {
long cPtr = AdaptiveCardObjectModelJNI.ActionElementParser_Deserialize(swigCPtr, this, ParseContext.getCPtr(context), context, JsonValue.getCPtr(value), value);
return (cPtr == 0) ? null : new BaseActionElement(cPtr, true);
}

Expand Down
Expand Up @@ -136,48 +136,28 @@ public CardElementType GetElementType() {
return CardElementType.swigToEnum(AdaptiveCardObjectModelJNI.AdaptiveCard_GetElementType(swigCPtr, this));
}

public static ParseResult DeserializeFromFile(String jsonFile, String rendererVersion, ElementParserRegistration elementParserRegistration, ActionParserRegistration actionParserRegistration) throws java.io.IOException {
long cPtr = AdaptiveCardObjectModelJNI.AdaptiveCard_DeserializeFromFile__SWIG_0(jsonFile, rendererVersion, ElementParserRegistration.getCPtr(elementParserRegistration), elementParserRegistration, ActionParserRegistration.getCPtr(actionParserRegistration), actionParserRegistration);
return (cPtr == 0) ? null : new ParseResult(cPtr, true);
}

public static ParseResult DeserializeFromFile(String jsonFile, String rendererVersion, ElementParserRegistration elementParserRegistration) throws java.io.IOException {
long cPtr = AdaptiveCardObjectModelJNI.AdaptiveCard_DeserializeFromFile__SWIG_1(jsonFile, rendererVersion, ElementParserRegistration.getCPtr(elementParserRegistration), elementParserRegistration);
public static ParseResult DeserializeFromFile(String jsonFile, String rendererVersion, ParseContext context) throws java.io.IOException {
long cPtr = AdaptiveCardObjectModelJNI.AdaptiveCard_DeserializeFromFile__SWIG_0(jsonFile, rendererVersion, ParseContext.getCPtr(context), context);
return (cPtr == 0) ? null : new ParseResult(cPtr, true);
}

public static ParseResult DeserializeFromFile(String jsonFile, String rendererVersion) throws java.io.IOException {
long cPtr = AdaptiveCardObjectModelJNI.AdaptiveCard_DeserializeFromFile__SWIG_2(jsonFile, rendererVersion);
return (cPtr == 0) ? null : new ParseResult(cPtr, true);
}

public static ParseResult Deserialize(JsonValue json, String rendererVersion, ElementParserRegistration elementParserRegistration, ActionParserRegistration actionParserRegistration) throws java.io.IOException {
long cPtr = AdaptiveCardObjectModelJNI.AdaptiveCard_Deserialize__SWIG_0(JsonValue.getCPtr(json), json, rendererVersion, ElementParserRegistration.getCPtr(elementParserRegistration), elementParserRegistration, ActionParserRegistration.getCPtr(actionParserRegistration), actionParserRegistration);
return (cPtr == 0) ? null : new ParseResult(cPtr, true);
}

public static ParseResult Deserialize(JsonValue json, String rendererVersion, ElementParserRegistration elementParserRegistration) throws java.io.IOException {
long cPtr = AdaptiveCardObjectModelJNI.AdaptiveCard_Deserialize__SWIG_1(JsonValue.getCPtr(json), json, rendererVersion, ElementParserRegistration.getCPtr(elementParserRegistration), elementParserRegistration);
return (cPtr == 0) ? null : new ParseResult(cPtr, true);
}

public static ParseResult Deserialize(JsonValue json, String rendererVersion) throws java.io.IOException {
long cPtr = AdaptiveCardObjectModelJNI.AdaptiveCard_Deserialize__SWIG_2(JsonValue.getCPtr(json), json, rendererVersion);
long cPtr = AdaptiveCardObjectModelJNI.AdaptiveCard_DeserializeFromFile__SWIG_1(jsonFile, rendererVersion);
return (cPtr == 0) ? null : new ParseResult(cPtr, true);
}

public static ParseResult DeserializeFromString(String jsonString, String rendererVersion, ElementParserRegistration elementParserRegistration, ActionParserRegistration actionParserRegistration) throws java.io.IOException {
long cPtr = AdaptiveCardObjectModelJNI.AdaptiveCard_DeserializeFromString__SWIG_0(jsonString, rendererVersion, ElementParserRegistration.getCPtr(elementParserRegistration), elementParserRegistration, ActionParserRegistration.getCPtr(actionParserRegistration), actionParserRegistration);
public static ParseResult Deserialize(JsonValue json, String rendererVersion, ParseContext context) throws java.io.IOException {
long cPtr = AdaptiveCardObjectModelJNI.AdaptiveCard_Deserialize(JsonValue.getCPtr(json), json, rendererVersion, ParseContext.getCPtr(context), context);
return (cPtr == 0) ? null : new ParseResult(cPtr, true);
}

public static ParseResult DeserializeFromString(String jsonString, String rendererVersion, ElementParserRegistration elementParserRegistration) throws java.io.IOException {
long cPtr = AdaptiveCardObjectModelJNI.AdaptiveCard_DeserializeFromString__SWIG_1(jsonString, rendererVersion, ElementParserRegistration.getCPtr(elementParserRegistration), elementParserRegistration);
public static ParseResult DeserializeFromString(String jsonString, String rendererVersion, ParseContext context) throws java.io.IOException {
long cPtr = AdaptiveCardObjectModelJNI.AdaptiveCard_DeserializeFromString__SWIG_0(jsonString, rendererVersion, ParseContext.getCPtr(context), context);
return (cPtr == 0) ? null : new ParseResult(cPtr, true);
}

public static ParseResult DeserializeFromString(String jsonString, String rendererVersion) throws java.io.IOException {
long cPtr = AdaptiveCardObjectModelJNI.AdaptiveCard_DeserializeFromString__SWIG_2(jsonString, rendererVersion);
long cPtr = AdaptiveCardObjectModelJNI.AdaptiveCard_DeserializeFromString__SWIG_1(jsonString, rendererVersion);
return (cPtr == 0) ? null : new ParseResult(cPtr, true);
}

Expand Down

0 comments on commit 97d01cf

Please sign in to comment.