[RND-696] .NET routing/validation #338

bradbanister · 2024-01-11T17:58:36Z

No description provided.

bradbanister · 2024-01-11T18:00:41Z

This version does endpoint validation. Document validation still in progress.

…on for overpost pruning

bradbanister · 2024-01-11T23:06:14Z

Got schema validation working.

Edit

The NJsonSchema library, does not provide enough information for total overpost pruning. It does provide enough information to prune extra properties at the top level of a document but not, for example, extra properties in an array object. Don't know if this is good enough.

bradbanister · 2024-01-12T17:52:39Z

The JsonSchema.Net library from json-everything does provide information for total overpost pruning. I demonstrated this with error messages showing the path to JSON elements that need pruning, separate from other schema validation errors.

andonyns · 2024-01-15T17:30:13Z

Meadowlark.net/Meadowlark.Net.Core/Handler/FrontendFacade.cs

+        .AndThen(DocumentValidation);
+
+      return frontendResponse ?? new(StatusCode: 200, Body: $"Success: {finalRequestModel.ResourceInfo.ToString()}");
+      // return frontendResponse ?? new(StatusCode: 500, Body: "FrontendResponse was null");


Should this comment be removed now?

andonyns · 2024-01-15T17:36:27Z

Meadowlark.net/Meadowlark.Net.Core/Middleware/ParsePathMiddleware.cs

+  {
+    if (str.Length == 0) return str;
+    if (str.Length == 1) return str.ToLower();
+    return str[0..1].ToLower() + str[1..];


Although not much of a difference, should the left side be str[0].ToLower() for clarity?

Sadly, C# wants str[0] to be a Char type., while the [0..1] range remains a string with the ToLower method.

andonyns · 2024-01-15T17:38:55Z

Meadowlark.net/Meadowlark.Net.Core/Middleware/ParsePathMiddleware.cs

+
+    Match? match = pathExpression.Match(path);
+
+    if (match == null)


We should go single line here

andonyns · 2024-01-15T17:53:19Z

Meadowlark.net/Meadowlark.Net.Core/Middleware/ValidateDocumentMiddleware.cs

+  /**
+   * Validates JSON document shape
+   */
+  public static async Task<MiddlewareModel> DocumentValidation(MiddlewareModel middlewareModel)


I got a warning when building the solution because of this function being async but not having any awaits:

warning CS1998: This async method lacks 'await' operators and will run synchronously.

Some of the 3rd-party libs I tried wanted await. Current one doesn't but I'm going to "wait" on changing this. There will other awaits in the middleware with the DB stuff but removing this one first cascades errors in several places.

andonyns

Simple comments

stephenfuqua · 2024-01-16T15:29:31Z

Meadowlark.net/Meadowlark.Net.Core/ApiSchema/ApiSchemaLoader.cs

+  public static JObject LoadApiSchemaFromFile()
+  {
+    // Hardcoded and synchronous way to read the API Schema file
+    return JObject.Parse(File.ReadAllText($"{AppContext.BaseDirectory}/DataStandard-5.0.0-pre.1.json"));


Long-term, we'll want to try System.Text.Json instead of Newtonsoft.Json. The former is a much higher performance rewrite, by the same creator but now directly employed by Microsoft. There are a few known limitations where Newtonsoft has continued to be better, but hopefully we won't run into those.

stephenfuqua · 2024-01-16T15:54:07Z

Meadowlark.net/Meadowlark.Net.Core/ApiSchema/Model/EndpointName.cs

+ * as a decapitalized MetaEd entity name. However, there are exceptions, for example descriptors have a
+ * "Descriptor" suffix on their endpoint name.
+ */
+public record struct EndpointName(string Value);


Have read about records, but first time I've actually seen them. Seems like a good use.

stephenfuqua · 2024-01-16T15:56:14Z

Meadowlark.net/Meadowlark.Net.Core/ApiSchema/Model/EndpointName.cs

+/**
+ * A string type branded as an EndpointName, which is the name of an API resource endpoint. Typically, this is the same
+ * as a decapitalized MetaEd entity name. However, there are exceptions, for example descriptors have a
+ * "Descriptor" suffix on their endpoint name.
+ */


For the eventual implementation, we'll want to use C# "xml" doc strings

Suggested change

/**

* A string type branded as an EndpointName, which is the name of an API resource endpoint. Typically, this is the same

* as a decapitalized MetaEd entity name. However, there are exceptions, for example descriptors have a

* "Descriptor" suffix on their endpoint name.

*/

/// <summary>

/// A string type branded as an EndpointName, which is the name of an API resource endpoint.

/// Typically, this is the same as a decapitalized MetaEd entity name. However, there are

/// exceptions, for example descriptors have a "Descriptor" suffix on their endpoint name.

/// </summary>

stephenfuqua · 2024-01-16T15:56:56Z

Meadowlark.net/Meadowlark.Net.Core/ApiSchema/ProjectSchemaFinder.cs

+  public static JObject? FindProjectSchema(JObject apiSchema, ProjectNamespace projectNamespace)
+  {
+    JToken? projectSchema = apiSchema.SelectToken($"$.projectSchemas.{projectNamespace.Value}");
+    return (JObject?)projectSchema;


Good article on tips for improving this performance: https://medium.com/@mbearz/strange-c-tricks-4-just-some-json-tricks-aa74ed02463e

stephenfuqua · 2024-01-16T16:01:06Z

Meadowlark.net/Meadowlark.Net.Core/Meadowlark.Net.Core.csproj

+  <PropertyGroup>
+    <TargetFramework>net8.0</TargetFramework>
+    <ImplicitUsings>enable</ImplicitUsings>
+    <Nullable>enable</Nullable>


Glad to see this already present. Making nullability an opt-in instead of always-on is a great new-ish feature in C#.

stephenfuqua · 2024-01-16T16:02:02Z

Meadowlark.net/Meadowlark.Net.Core/Handler/FrontendFacade.cs

+namespace Meadowlark.Net.Core;
+
+
+public static class FrontendFacade


Some of these static classes will need to become instance classes so that they can implement interfaces and thus be used in dependency injection... thereby facilitating mocking in unit tests.

stephenfuqua · 2024-01-16T16:04:30Z

Meadowlark.net/Meadowlark.Net.Core/Handler/FrontendFacade.cs

+        .SendTo(LoadApiSchema)
+        .AndThen(ParsePath)
+        .AndThen(EndpointValidation)
+        .AndThen(DocumentValidation);


If these same steps are taken for all requests, we may want to implement them as ASP.NET Core middleware. The pipeline approach above looks good, but there may be some advantages to using the middleware. One advantage might be greater familiarity for veteran C# developers.

stephenfuqua · 2024-01-16T16:08:10Z

Meadowlark.net/Meadowlark.Net.Core/Middleware/ParsePathMiddleware.cs

+
+public static class ParsePathMiddleware
+{
+  private static string Decapitalize(string str)


Might want to make this an extension method in the future, if it might be needed elsewhere in the code base.

stephenfuqua · 2024-01-16T16:09:05Z

Meadowlark.net/Meadowlark.Net.Core/Middleware/ParsePathMiddleware.cs

+    // /ed-fi/Sections
+    // /ed-fi/Sections/
+    // /ed-fi/Sections/idValue
+    Regex pathExpression = new(@"\/(?<projectNamespace>[^/]+)\/(?<endpointName>[^/]+)(\/|$)((?<documentUuid>[^/]*$))?");


For full implementation, we'll want this regex to be compiled and defined in a lazy static property.

stephenfuqua · 2024-01-16T16:12:07Z

Meadowlark.net/Meadowlark.Net.Frontend.MinimalAPI/CrudHandler.cs

+
+namespace Meadowlark.Net.Frontend.MinimalAPI
+{
+  public static class CrudHandler


We might want to call this a Controller, to help new people who are searching around for the Controllers.

This particular controller will work with two different API definitions: Descriptors and Resources. For now, "Crud" seems as good a unifying name as anything 😆 .

[RND-696] .NET routing/validation

a2a1df2

bradbanister added 2 commits January 11, 2024 16:13

[RND-696] Schema validation

6a8260c

[RND-696] tests showing NJsonSchema does not provide enough informati…

7caeff8

…on for overpost pruning

[RND-696] prove overposts can be removed

fc15992

bradbanister marked this pull request as ready for review January 12, 2024 17:52

bradbanister closed this Jan 12, 2024

bradbanister reopened this Jan 12, 2024

andonyns reviewed Jan 15, 2024

View reviewed changes

andonyns suggested changes Jan 15, 2024

View reviewed changes

pr fixes

e2f9390

andonyns approved these changes Jan 15, 2024

View reviewed changes

add Meadowlark.net to paths

72848f9

bradbanister requested a review from a team as a code owner January 15, 2024 19:23

bradbanister and others added 2 commits January 15, 2024 19:23

Merge 72848f9 into 2e54fe6

e87856f

v0.4.1-pre.5

f270b3f

bradbanister closed this Jan 16, 2024

bradbanister reopened this Jan 16, 2024

stephenfuqua approved these changes Jan 16, 2024

View reviewed changes

stephenfuqua merged commit 2e141a1 into main Jan 16, 2024
15 checks passed

stephenfuqua deleted the RND-696 branch January 16, 2024 16:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RND-696] .NET routing/validation #338

[RND-696] .NET routing/validation #338

bradbanister commented Jan 11, 2024

bradbanister commented Jan 11, 2024

bradbanister commented Jan 11, 2024 •

edited

Loading

bradbanister commented Jan 12, 2024

andonyns Jan 15, 2024

andonyns Jan 15, 2024

bradbanister Jan 15, 2024 •

edited

Loading

andonyns Jan 15, 2024

andonyns Jan 15, 2024

bradbanister Jan 15, 2024

andonyns left a comment

stephenfuqua Jan 16, 2024

stephenfuqua Jan 16, 2024

stephenfuqua Jan 16, 2024

stephenfuqua Jan 16, 2024

stephenfuqua Jan 16, 2024

stephenfuqua Jan 16, 2024

stephenfuqua Jan 16, 2024

stephenfuqua Jan 16, 2024

stephenfuqua Jan 16, 2024

stephenfuqua Jan 16, 2024


		Match? match = pathExpression.Match(path);

		if (match == null)

-/**
- * A string type branded as an EndpointName, which is the name of an API resource endpoint. Typically, this is the same
- * as a decapitalized MetaEd entity name. However, there are exceptions, for example descriptors have a
- * "Descriptor" suffix on their endpoint name.
- */
+/// <summary>
+/// A string type branded as an EndpointName, which is the name of an API resource endpoint.
+/// Typically, this is the same as a decapitalized MetaEd entity name. However, there are
+/// exceptions, for example descriptors have a "Descriptor" suffix on their endpoint name.
+/// </summary>

		namespace Meadowlark.Net.Core;


		public static class FrontendFacade

[RND-696] .NET routing/validation #338

[RND-696] .NET routing/validation #338

Conversation

bradbanister commented Jan 11, 2024

bradbanister commented Jan 11, 2024

bradbanister commented Jan 11, 2024 • edited Loading

bradbanister commented Jan 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bradbanister Jan 15, 2024 • edited Loading

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

andonyns left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bradbanister commented Jan 11, 2024 •

edited

Loading

bradbanister Jan 15, 2024 •

edited

Loading