Update to 2.1 specification #141

bingenito · 2024-07-29T16:43:35Z

Update to 2.1. Checklist and work detailed in #74

THIS SOFTWARE IS CONTRIBUTED SUBJECT TO THE TERMS OF THE FINOS CORPORATE CONTRIBUTOR LICENSE AGREEMENT.

THIS SOFTWARE IS LICENSED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE AND ANY WARRANTY OF NON-INFRINGEMENT, ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. THIS SOFTWARE MAY BE REDISTRIBUTED TO OTHERS ONLY BY EFFECTIVELY USING THIS OR ANOTHER EQUIVALENT DISCLAIMER IN ADDITION TO ANY OTHER REQUIRED LICENSE TERMS.

… market identifiers

…nto fdc3-2_1

codecov-commenter · 2024-07-29T17:10:20Z

Codecov Report

Attention: Patch coverage is 81.35593% with 22 lines in your changes missing coverage. Please review.

Project coverage is 77.22%. Comparing base (c783922) to head (f13c648).
Report is 35 commits behind head on main.

Files	Patch %	Lines
...3.NewtonsoftJson/Serialization/IntentsConverter.cs	50.00%	3 Missing and 2 partials ⚠️
src/Fdc3/IntentMetadata.cs	0.00%	5 Missing ⚠️
src/Fdc3/Context/TransactionResultStatus.cs	0.00%	4 Missing ⚠️
src/Fdc3/Errors.cs	0.00%	3 Missing ⚠️
...c3.Json/Serialization/Fdc3CamelCaseNamingPolicy.cs	0.00%	2 Missing ⚠️
src/Fdc3.Json/Serialization/IntentsConverter.cs	66.66%	1 Missing and 1 partial ⚠️
src/Fdc3.AppDirectory/AppChannel.cs	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #141      +/-   ##
==========================================
+ Coverage   76.56%   77.22%   +0.66%     
==========================================
  Files          53       62       +9     
  Lines         431      527      +96     
  Branches       44       48       +4     
==========================================
+ Hits          330      407      +77     
- Misses         82       98      +16     
- Partials       19       22       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bingenito · 2024-07-29T17:39:49Z

src/Examples/WpfFdc3/Fdc3/DesktopAgent.cs

@@ -33,7 +33,7 @@ public DesktopAgent()

        public Task<IListener> AddContextListener<T>(string? contextType, ContextHandler<T> handler) where T : IContext
        {
-            return _currentChannel?.AddContextListener<T>(contextType, handler);
+            return _currentChannel?.AddContextListener<T>(contextType, handler) ?? throw new Exception("Unable to create listener");


This is a long existing build warning in the example project about possible null return value when the return is declared as non nullable

bingenito · 2024-07-29T17:41:02Z

src/Fdc3.AppDirectory/AppChannel.cs

        /// </summary>
-        public string Name { get; set; }
+        public string ID { get; set; }


AppChannel Name changed to ID to avoid confusion. This required changes to class and sample json used for deserialization in unit testing.

bingenito · 2024-07-29T17:41:49Z

src/Fdc3.AppDirectory/Fdc3App.cs

            Name = name ?? throw new ArgumentNullException( nameof(name));
+#pragma warning restore CS0618 // Type or member is obsolete


Even though Name is marked as obsolete/decomissioned we are still using it. In order to not have build errors in our own code about using obsolete member, disable the warning.

src/Fdc3.Json/Serialization/Fdc3CamelCaseNamingPolicy.cs

bingenito · 2024-07-29T17:46:25Z

src/Fdc3/Context/Contact.cs

@@ -28,6 +28,6 @@ public class ContactID
    {
        public string? Email { get; set; }

-        public string? FdsId { get; set; }
+        public string? FDS_ID { get; set; }


This was was previously incorrect and is being fied here.

bingenito · 2024-07-29T18:37:49Z

src/Fdc3/Context/ChatInitSettings.cs


        object? IContext<object>.ID => base.ID;
    }
+
+    public class ChatInitSettingsOptions


Not sure why I was so lazy in past implementation of just taking an object. I defined it as a custom type now.

bingenito · 2024-07-29T18:39:30Z

src/Fdc3/Context/ContextTypes.cs

        public static readonly string Nothing = "fdc3.nothing";
        public static readonly string Organization = "fdc3.organization";
        public static readonly string Portfolio = "fdc3.portfolio";
        public static readonly string Position = "fdc3.position";
-        public static readonly string TimeRange = "fdc3.timerange";
+        public static readonly string TimeRange = "fdc3.timeRange";
+        public static readonly string TransactionResult = "fdc3.transactionResult";
        public static readonly string Valuation = "fdc3.valuation";

        public static IDictionary<string, Type> Map = new Dictionary<string, Type>()


For those wondering, this is useful for DesktopAgents to use as a mapping when doing deserialization. Based on the context type string on the message, do the lookup here to get the concrete type and then deserialize to that type. Note that this is also public so your desktopagent can add/overwrite in their implementation as they see fit such as mapping to a derived context type rather than the base default.

bingenito · 2024-07-29T18:40:56Z

src/Fdc3/Context/TransactionResultStatus.cs

+ * and limitations under the License.
+ */
+
+namespace Finos.Fdc3.Context


As discussed on standards call. This is being defined as string constants for conditional statements and creating messages as an enum would be serialized to lower case and not be correct. The default serialization for enum is to lower case based on other areas of the api and contexts so to use an enum here would require custom serialization on TransactionResult

bingenito · 2024-07-29T18:41:33Z

src/Fdc3/IIntentMetadata.cs

@@ -27,6 +29,7 @@ public interface IIntentMetadata
        /// <summary>
        /// A friendly display name for the intent that should be used to render UI elements.
        /// </summary>
-        string DisplayName { get; }
+        [Obsolete("Use the intent name for display as display name may vary for each application  as it is defined in the app's AppD record.")]
+        string? DisplayName { get; }


obsolete/decommissioned and also changed to nullable

bingenito · 2024-07-29T18:43:59Z

src/Tests/Finos.Fdc3.NewtonsoftJson.Tests/Context/MessageTests.cs

+    [Fact]
+    public async Task Message_SerializedJsonMatchesSchema()
+    {
+        Message message = new Message(new MessageText() { TextPlain = "textplain", TextMarkdown = "textmarkdown" });


This is an example of how we defined the type with TextPlain/TextMarkdown string properties and the schema validation below will pass if they are serialized to text/plan and text/markdown

psmulovics · 2024-07-29T18:44:43Z

Any desire if you are already there changing the tests to be async Task instead of async void?

bingenito · 2024-07-29T18:47:31Z

Any desire if you are already there changing the tests to be async Task instead of async void?

They were changed from async void to async Task to cleanup warnings in preparation for next xunit release.

bingenito · 2024-07-30T10:26:44Z

src/Fdc3/Context/TransactionResult.cs

+            this.Message = message;
+        }
+
+        public string Status { get; set; }


See comment on TransactionResultStatus on this not being an enum but just a string

bingenito · 2024-07-30T10:27:26Z

src/Fdc3/Context/Message.cs

+        public MessageText Text { get; set; }
+    }
+
+    public class MessageText


Custom serializer provided in NewtonsoftJson or Json packages correctly round-trip these by changing to text/plan and text/markdown

bingenito added 29 commits February 23, 2024 12:18

Added MalformedContext error to OpenError, ChannelError and ResolveError

b2b3bd0

Updated definition of the instrument context type to include optional…

f5eddd2

… market identifiers

Added MalformedContext error to OpenError, ChannelError and ResolveError

d5c2b04

Updated definition of the instrument context type to include optional…

f01ab52

… market identifiers

Update Chart otherConfig to array of Context

74016c7

Correct inconsistent casing of fdc3.timeRange

a00d372

Update Chart tests to pass aray of Context

9e64b3e

Rename ChatInitSettings.InitMessage to Message

8d7cdc7

Fix tests to use valid email and rename Fdsid to FDS_ID

414a9a8

Update schema version on tests

639b5a0

Merge branch 'fdc3-2_1' of https://github.com/bingenito/fdc3-dotnet i…

8b6cf5a

…nto fdc3-2_1

Add TransactionResult context type

606fc52

Add TransactionResultStatus helper static string values

8cea70d

Add Action context type

5c600df

Update version to prepare for 2.1.0 beta 1

5c95a62

Add ChatRoom context type

d59928a

Ignore obsolete warnings in tests as members still tested

2f803ad

Add ChatSearchCriteria context type

9985b46

Fix warnings

905a4b5

Add mising context Type property tests

1ec3fd9

Add Message context type

ba7180a

Add ChatMessage context type

483ed1f

Update ChatInitSettings to use defined types

616aa94

Mark AppD displayName as obsolete

01dded7

Mark AppD App Name as obsolete

da43598

Mark AppD App CustomConfig as obsolete

084d350

Fix AppD Interop AppChannel to use ID instead of Name

5e1c370

Changer Chart OtherConfig from array to IEnumerable

c196708

Fix build warnings

b6331db

bingenito requested a review from a team as a code owner July 29, 2024 16:43

Add back ctor including obsolete members

914ecee

bingenito marked this pull request as draft July 29, 2024 17:38

bingenito commented Jul 29, 2024

View reviewed changes

src/Fdc3.Json/Serialization/Fdc3CamelCaseNamingPolicy.cs Show resolved Hide resolved

bingenito commented Jul 29, 2024

View reviewed changes

bingenito added 2 commits July 29, 2024 14:33

Remove uncessary imports

ce68a38

Add context type map entries for new context types

f13c648

bingenito commented Jul 29, 2024

View reviewed changes

bingenito marked this pull request as ready for review July 29, 2024 18:52

bingenito commented Jul 30, 2024

View reviewed changes

kriswest linked an issue Aug 22, 2024 that may be closed by this pull request

Update to FDC3 2.1 #74

Open

18 tasks

kriswest mentioned this pull request Aug 23, 2024

Standard WG Meeting - August 22nd, 2024 finos/FDC3#1317

Closed

15 tasks

kriswest mentioned this pull request Sep 26, 2024

Standard WG Meeting - September 26th, 2024 finos/FDC3#1364

Open

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to 2.1 specification #141

Update to 2.1 specification #141

bingenito commented Jul 29, 2024 •

edited

Loading

codecov-commenter commented Jul 29, 2024 •

edited

Loading

bingenito Jul 29, 2024

bingenito Jul 29, 2024

bingenito Jul 29, 2024

bingenito Jul 29, 2024

bingenito Jul 29, 2024

bingenito Jul 29, 2024

bingenito Jul 29, 2024

bingenito Jul 29, 2024

bingenito Jul 29, 2024

psmulovics commented Jul 29, 2024

bingenito commented Jul 29, 2024

bingenito Jul 30, 2024

bingenito Jul 30, 2024

		Name = name ?? throw new ArgumentNullException( nameof(name));
		#pragma warning restore CS0618 // Type or member is obsolete

Update to 2.1 specification #141

Are you sure you want to change the base?

Update to 2.1 specification #141

Conversation

bingenito commented Jul 29, 2024 • edited Loading

codecov-commenter commented Jul 29, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

psmulovics commented Jul 29, 2024

bingenito commented Jul 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bingenito commented Jul 29, 2024 •

edited

Loading

codecov-commenter commented Jul 29, 2024 •

edited

Loading