From fe7d004fec1d2204163d6ee7d458e8540d9f1201 Mon Sep 17 00:00:00 2001 From: Matthew Parker Date: Tue, 10 Dec 2024 22:40:01 +0000 Subject: [PATCH 1/4] fixed code quality issue for multiple classes per file --- .../CreateDocumentRequestGenericTests.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/PinguApps.Appwrite.Shared.Tests/Requests/Databases/CreateDocumentRequestGenericTests.cs b/tests/PinguApps.Appwrite.Shared.Tests/Requests/Databases/CreateDocumentRequestGenericTests.cs index e14d0911..ef83e4a3 100644 --- a/tests/PinguApps.Appwrite.Shared.Tests/Requests/Databases/CreateDocumentRequestGenericTests.cs +++ b/tests/PinguApps.Appwrite.Shared.Tests/Requests/Databases/CreateDocumentRequestGenericTests.cs @@ -2,18 +2,19 @@ using PinguApps.Appwrite.Shared.Requests.Databases; using PinguApps.Appwrite.Shared.Requests.Databases.Validators; using PinguApps.Appwrite.Shared.Utils; +using static PinguApps.Appwrite.Shared.Tests.Requests.Databases.CreateDocumentRequestGenericTests; namespace PinguApps.Appwrite.Shared.Tests.Requests.Databases; -// Test model for our generic type -public class CreateDocumentTestData -{ - public string Name { get; set; } = string.Empty; - public int Age { get; set; } -} - public class CreateDocumentRequestGenericTests : DatabaseCollectionIdBaseRequestTests, CreateDocumentRequestValidator> { + // Test model for our generic type + public class CreateDocumentTestData + { + public string Name { get; set; } = string.Empty; + public int Age { get; set; } + } + protected override CreateDocumentRequest CreateValidDatabaseCollectionIdRequest => new() { DocumentId = IdUtils.GenerateUniqueId(), From 303682910b57d43cccbee481be19120dffce6271 Mon Sep 17 00:00:00 2001 From: Matthew Parker Date: Wed, 11 Dec 2024 00:17:57 +0000 Subject: [PATCH 2/4] Split up read method of Doc Generic Converter as it was too complex --- .../Converters/DocumentGenericConverter.cs | 155 ++++++++++-------- .../DocumentGenericConverterTests.cs | 36 +--- 2 files changed, 84 insertions(+), 107 deletions(-) diff --git a/src/PinguApps.Appwrite.Shared/Converters/DocumentGenericConverter.cs b/src/PinguApps.Appwrite.Shared/Converters/DocumentGenericConverter.cs index ade68935..3aa27074 100644 --- a/src/PinguApps.Appwrite.Shared/Converters/DocumentGenericConverter.cs +++ b/src/PinguApps.Appwrite.Shared/Converters/DocumentGenericConverter.cs @@ -10,102 +10,92 @@ namespace PinguApps.Appwrite.Shared.Converters; public class DocumentGenericConverter : JsonConverter> where TData : class, new() { + private class DocumentFields + { + public string? Id { get; set; } + public string? CollectionId { get; set; } + public string? DatabaseId { get; set; } + public DateTime? CreatedAt { get; set; } + public DateTime? UpdatedAt { get; set; } + public List? Permissions { get; set; } + public Dictionary DataProperties { get; set; } = new(); + } + public override Document Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - string? id = null; - string? collectionId = null; - string? databaseId = null; - DateTime? createdAt = null; - DateTime? updatedAt = null; - List? permissions = null; - TData data = new(); + ValidateStartObject(ref reader); - var dataProperties = new Dictionary(); + var documentFields = ReadDocumentFields(ref reader, options); - var dateTimeConverter = new MultiFormatDateTimeConverter(); - var permissionListConverter = new PermissionListConverter(); + ValidateRequiredFields(documentFields); + + var data = DeserializeCustomData(documentFields.DataProperties, options); + + return new Document(documentFields.Id!, documentFields.CollectionId!, documentFields.DatabaseId!, documentFields.CreatedAt, + documentFields.UpdatedAt, documentFields.Permissions!, data); + } + private static void ValidateStartObject(ref Utf8JsonReader reader) + { if (reader.TokenType is not JsonTokenType.StartObject) { throw new JsonException("Expected StartObject token"); } + } + + private DocumentFields ReadDocumentFields(ref Utf8JsonReader reader, JsonSerializerOptions options) + { + var dateTimeConverter = new MultiFormatDateTimeConverter(); + var permissionListConverter = new PermissionListConverter(); + var fields = new DocumentFields(); while (reader.Read()) { - if (reader.TokenType == JsonTokenType.EndObject) + if (reader.TokenType is JsonTokenType.EndObject) { break; } var propertyName = reader.GetString()!; - reader.Read(); - switch (propertyName) - { - case "$id": - id = reader.GetString(); - break; - case "$collectionId": - collectionId = reader.GetString(); - break; - case "$databaseId": - databaseId = reader.GetString(); - break; - case "$createdAt": - createdAt = dateTimeConverter.Read(ref reader, typeof(DateTime), options); - break; - case "$updatedAt": - updatedAt = dateTimeConverter.Read(ref reader, typeof(DateTime), options); - break; - case "$permissions": - permissions = permissionListConverter.Read(ref reader, typeof(List), options); - break; - default: - var value = ReadValue(ref reader, options); - dataProperties[propertyName] = value; - break; - } - } - - if (id is null) - { - throw new JsonException("Unable to find a value for Id"); - } - - if (collectionId is null) - { - throw new JsonException("Unable to find a value for CollectionId"); - } - - if (databaseId is null) - { - throw new JsonException("Unable to find a value for DatabaseId"); + ProcessProperty(ref reader, propertyName, fields, dateTimeConverter, permissionListConverter, options); } - if (createdAt is null) - { - throw new JsonException("Unable to find a value for CreatedAt"); - } - - if (updatedAt is null) - { - throw new JsonException("Unable to find a value for UpdatedAt"); - } + return fields; + } - if (permissions is null) + private static void ProcessProperty(ref Utf8JsonReader reader, string propertyName, DocumentFields fields, + MultiFormatDateTimeConverter dateTimeConverter, PermissionListConverter permissionListConverter, JsonSerializerOptions options) + { + switch (propertyName) { - throw new JsonException("Unable to find a value for Permissions"); + case "$id": + fields.Id = reader.GetString(); + break; + case "$collectionId": + fields.CollectionId = reader.GetString(); + break; + case "$databaseId": + fields.DatabaseId = reader.GetString(); + break; + case "$createdAt": + fields.CreatedAt = dateTimeConverter.Read(ref reader, typeof(DateTime), options); + break; + case "$updatedAt": + fields.UpdatedAt = dateTimeConverter.Read(ref reader, typeof(DateTime), options); + break; + case "$permissions": + fields.Permissions = permissionListConverter.Read(ref reader, typeof(List), options); + break; + default: + var value = ReadValue(ref reader, options); + fields.DataProperties[propertyName] = value; + break; } - - // Deserialize the remaining properties into TData - var dataJson = JsonSerializer.Serialize(dataProperties, options); - data = JsonSerializer.Deserialize(dataJson, options) ?? new TData(); - - return new Document(id, collectionId, databaseId, createdAt.Value, updatedAt.Value, permissions, data); } - internal object? ReadValue(ref Utf8JsonReader reader, JsonSerializerOptions options) + internal static object? ReadValue(ref Utf8JsonReader reader, JsonSerializerOptions options) { switch (reader.TokenType) { @@ -143,7 +133,7 @@ public override Document Read(ref Utf8JsonReader reader, Type typeToConve } } - private IReadOnlyCollection ReadArray(ref Utf8JsonReader reader, JsonSerializerOptions options) + private static IReadOnlyCollection ReadArray(ref Utf8JsonReader reader, JsonSerializerOptions options) { var list = new List(); @@ -161,7 +151,7 @@ public override Document Read(ref Utf8JsonReader reader, Type typeToConve return list; } - private Dictionary ReadObject(ref Utf8JsonReader reader, JsonSerializerOptions options) + private static Dictionary ReadObject(ref Utf8JsonReader reader, JsonSerializerOptions options) { var dict = new Dictionary(); @@ -184,6 +174,27 @@ public override Document Read(ref Utf8JsonReader reader, Type typeToConve return dict; } + private static void ValidateRequiredFields(DocumentFields fields) + { + if (fields.Id is null) + throw new JsonException("Unable to find a value for Id"); + + if (fields.CollectionId is null) + throw new JsonException("Unable to find a value for CollectionId"); + + if (fields.DatabaseId is null) + throw new JsonException("Unable to find a value for DatabaseId"); + + if (fields.Permissions is null) + throw new JsonException("Unable to find a value for Permissions"); + } + + private static TData DeserializeCustomData(Dictionary dataProperties, JsonSerializerOptions options) + { + var dataJson = JsonSerializer.Serialize(dataProperties, options); + return JsonSerializer.Deserialize(dataJson, options) ?? new TData(); + } + public override void Write(Utf8JsonWriter writer, Document value, JsonSerializerOptions options) { writer.WriteStartObject(); diff --git a/tests/PinguApps.Appwrite.Shared.Tests/Converters/DocumentGenericConverterTests.cs b/tests/PinguApps.Appwrite.Shared.Tests/Converters/DocumentGenericConverterTests.cs index 9a18580c..129dd713 100644 --- a/tests/PinguApps.Appwrite.Shared.Tests/Converters/DocumentGenericConverterTests.cs +++ b/tests/PinguApps.Appwrite.Shared.Tests/Converters/DocumentGenericConverterTests.cs @@ -430,38 +430,6 @@ public void Read_MissingDatabaseId_ThrowsJsonException() Assert.Throws(() => JsonSerializer.Deserialize>(json, _options)); } - [Fact] - public void Read_MissingCreatedAt_ThrowsJsonException() - { - var json = @" - { - ""$id"": ""1"", - ""$collectionId"": ""col1"", - ""$databaseId"": ""db1"", - ""$updatedAt"": ""2020-10-15T06:38:00.000+00:00"", - ""$permissions"": [""read(\""any\"")""], - ""Field1"": ""value1"" - }"; - - Assert.Throws(() => JsonSerializer.Deserialize>(json, _options)); - } - - [Fact] - public void Read_MissingUpdatedAt_ThrowsJsonException() - { - var json = @" - { - ""$id"": ""1"", - ""$collectionId"": ""col1"", - ""$databaseId"": ""db1"", - ""$createdAt"": ""2020-10-15T06:38:00.000+00:00"", - ""$permissions"": [""read(\""any\"")""], - ""Field1"": ""value1"" - }"; - - Assert.Throws(() => JsonSerializer.Deserialize>(json, _options)); - } - [Fact] public void Read_MissingPermissions_ThrowsJsonException() { @@ -568,8 +536,6 @@ public void ReadValue_UnsupportedTokenType_ThrowsJsonException() var reader = new Utf8JsonReader(bytes, readerOptions); - var converter = new DocumentGenericConverter(); - // Read the StartObject token reader.Read(); // JsonTokenType.StartObject @@ -585,7 +551,7 @@ public void ReadValue_UnsupportedTokenType_ThrowsJsonException() // Calling ReadValue should now hit the default case and throw JsonException try { - converter.ReadValue(ref reader, _options); + DocumentGenericConverter.ReadValue(ref reader, _options); Assert.Fail("Did not throw JsonException"); } catch (JsonException) From 98e38c3dfb237d09185e0599952e7f8c5f4bbfc1 Mon Sep 17 00:00:00 2001 From: Matthew Parker Date: Wed, 11 Dec 2024 00:40:08 +0000 Subject: [PATCH 3/4] Spit up complex method in permission json converter --- .../Converters/PermissionJsonConverter.cs | 70 +++++++++++++++---- 1 file changed, 55 insertions(+), 15 deletions(-) diff --git a/src/PinguApps.Appwrite.Shared/Converters/PermissionJsonConverter.cs b/src/PinguApps.Appwrite.Shared/Converters/PermissionJsonConverter.cs index 0cc00eab..004c1cbc 100644 --- a/src/PinguApps.Appwrite.Shared/Converters/PermissionJsonConverter.cs +++ b/src/PinguApps.Appwrite.Shared/Converters/PermissionJsonConverter.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics.CodeAnalysis; using System.Text.Json; using System.Text.Json.Serialization; using PinguApps.Appwrite.Shared.Enums; @@ -8,15 +9,27 @@ namespace PinguApps.Appwrite.Shared.Converters; public class PermissionJsonConverter : JsonConverter { public override Permission? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + ValidateTokenType(ref reader); + + var value = reader.GetString()!; + + var (permissionType, roleString) = ParsePermissionString(value); + var permissionBuilder = CreatePermissionBuilder(permissionType); + + return ResolveRole(permissionBuilder, roleString); + } + + private static void ValidateTokenType(ref Utf8JsonReader reader) { if (reader.TokenType is not JsonTokenType.String) { throw new JsonException("Expected string value for Permission"); } + } - var value = reader.GetString()!; - - // Format is "permissionType(\"roleString\")" + private static (string permissionType, string roleString) ParsePermissionString(string value) + { var openParenIndex = value.IndexOf('('); var closeParenIndex = value.LastIndexOf(')'); @@ -25,31 +38,58 @@ public class PermissionJsonConverter : JsonConverter throw new JsonException("Invalid Permission format"); } - var permissionTypeStr = value[..openParenIndex].ToLower(); + var permissionType = value[..openParenIndex].ToLower(); // Remove the quotes from the role string var roleString = value[(openParenIndex + 2)..(closeParenIndex - 1)]; - var permissionBuilder = permissionTypeStr switch + return (permissionType, roleString); + } + + private static Permission.PermissionBuilder CreatePermissionBuilder(string permissionType) + { + return permissionType switch { "read" => Permission.Read(), "write" => Permission.Write(), "create" => Permission.Create(), "update" => Permission.Update(), "delete" => Permission.Delete(), - _ => throw new JsonException($"Unknown permission type: {permissionTypeStr}") + _ => throw new JsonException($"Unknown permission type: {permissionType}") }; + } + + private static Permission ResolveRole(Permission.PermissionBuilder builder, string roleString) + { + if (IsSimpleRole(roleString, builder, out var simplePermission)) + { + return simplePermission; + } + + return ResolveComplexRole(builder, roleString); + } - // Parse the role string + private static bool IsSimpleRole(string roleString, Permission.PermissionBuilder builder, [NotNullWhen(true)] out Permission? permission) + { + permission = roleString switch + { + "any" => builder.Any(), + "users" => builder.Users(), + "guests" => builder.Guests(), + _ => null + }; + + return permission != null; + } + + private static Permission ResolveComplexRole(Permission.PermissionBuilder builder, string roleString) + { return roleString switch { - "any" => permissionBuilder.Any(), - "users" => permissionBuilder.Users(), - "guests" => permissionBuilder.Guests(), - var s when s.StartsWith("user:") => ParseUserRole(permissionBuilder, s[5..]), - var s when s.StartsWith("users/") => ParseUsersRole(permissionBuilder, s[6..]), - var s when s.StartsWith("team:") => ParseTeamRole(permissionBuilder, s[5..]), - var s when s.StartsWith("member:") => permissionBuilder.Member(s[7..]), - var s when s.StartsWith("label:") => permissionBuilder.Label(s[6..]), + var s when s.StartsWith("user:") => ParseUserRole(builder, s[5..]), + var s when s.StartsWith("users/") => ParseUsersRole(builder, s[6..]), + var s when s.StartsWith("team:") => ParseTeamRole(builder, s[5..]), + var s when s.StartsWith("member:") => builder.Member(s[7..]), + var s when s.StartsWith("label:") => builder.Label(s[6..]), _ => throw new JsonException($"Unknown role format: {roleString}") }; } From 5c5a16721e64f8a1db0fc02cbf8f0409fadc2dec Mon Sep 17 00:00:00 2001 From: Matthew Parker Date: Wed, 11 Dec 2024 00:54:03 +0000 Subject: [PATCH 4/4] Simplified complex read method in attribute json converter --- .../Converters/AttributeJsonConverter.cs | 63 +++++++++++++++---- 1 file changed, 50 insertions(+), 13 deletions(-) diff --git a/src/PinguApps.Appwrite.Shared/Converters/AttributeJsonConverter.cs b/src/PinguApps.Appwrite.Shared/Converters/AttributeJsonConverter.cs index 8845814b..118e385e 100644 --- a/src/PinguApps.Appwrite.Shared/Converters/AttributeJsonConverter.cs +++ b/src/PinguApps.Appwrite.Shared/Converters/AttributeJsonConverter.cs @@ -10,36 +10,73 @@ public class AttributeJsonConverter : JsonConverter public override Attribute? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { using var jsonDoc = JsonDocument.ParseValue(ref reader); + var jsonObject = jsonDoc.RootElement; + var attributeType = ResolveAttributeType(jsonObject); + + return DeserializeAttribute(jsonObject, attributeType, options); + } + + private static Type ResolveAttributeType(JsonElement jsonObject) + { + var type = GetRequiredTypeProperty(jsonObject); + var format = GetOptionalFormatProperty(jsonObject); + + return DetermineAttributeType(type, format, jsonObject); + } + + private static string GetRequiredTypeProperty(JsonElement jsonObject) + { if (!jsonObject.TryGetProperty("type", out var typeProperty)) { throw new JsonException("Missing `Type` property"); } - var type = typeProperty.GetString(); + return typeProperty.GetString()!; + } + + private static string? GetOptionalFormatProperty(JsonElement jsonObject) + { jsonObject.TryGetProperty("format", out var formatProperty); - var format = formatProperty.ValueKind == JsonValueKind.String ? formatProperty.GetString() : null; - var derivedType = type switch + return formatProperty.ValueKind is JsonValueKind.String ? formatProperty.GetString() : null; + } + + private static Type DetermineAttributeType(string type, string? format, JsonElement jsonObject) + { + return type switch { "boolean" => typeof(AttributeBoolean), "integer" => typeof(AttributeInteger), "double" => typeof(AttributeFloat), "datetime" => typeof(AttributeDatetime), - "string" => format switch - { - "email" => typeof(AttributeEmail), - "url" => typeof(AttributeUrl), - "ip" => typeof(AttributeIp), - "enum" => typeof(AttributeEnum), - null or "" => jsonObject.TryGetProperty("relatedCollection", out _) ? typeof(AttributeRelationship) : typeof(AttributeString), - _ => throw new JsonException($"Unknown format: {format}") - }, + "string" => ResolveStringAttributeType(format, jsonObject), _ => throw new JsonException($"Unknown type: {type}") }; + } + + private static Type ResolveStringAttributeType(string? format, JsonElement jsonObject) + { + return format switch + { + "email" => typeof(AttributeEmail), + "url" => typeof(AttributeUrl), + "ip" => typeof(AttributeIp), + "enum" => typeof(AttributeEnum), + null or "" => ResolveBasicStringAttributeType(jsonObject), + _ => throw new JsonException($"Unknown format: {format}") + }; + } + + private static Type ResolveBasicStringAttributeType(JsonElement jsonObject) + { + return jsonObject.TryGetProperty("relatedCollection", out _) ? typeof(AttributeRelationship) : typeof(AttributeString); + } - return (Attribute?)JsonSerializer.Deserialize(jsonObject.GetRawText(), derivedType, options); + private static Attribute? DeserializeAttribute(JsonElement jsonObject, Type attributeType, JsonSerializerOptions options) + { + return (Attribute?)JsonSerializer.Deserialize(jsonObject.GetRawText(), attributeType, options); } public override void Write(Utf8JsonWriter writer, Attribute value, JsonSerializerOptions options)