Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nullable fixes - nullability on Base #184

Merged
merged 12 commits into from
Dec 3, 2024
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#nullable disable

using System.Reflection;
using GraphQL.Client.Abstractions.Utilities;
using Speckle.Newtonsoft.Json;
Expand All @@ -9,7 +7,7 @@ namespace Speckle.Sdk.Api.GraphQL.Serializer;

internal class ConstantCaseEnumConverter : StringEnumConverter
{
public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
public override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer)
{
if (value == null)
{
Expand All @@ -18,7 +16,7 @@ public override void WriteJson(JsonWriter writer, object value, JsonSerializer s
else
{
var enumString = ((Enum)value).ToString("G");
var memberName = value
string? memberName = value
.GetType()
.GetMember(enumString, BindingFlags.DeclaredOnly | BindingFlags.Static | BindingFlags.Public)
.FirstOrDefault()
Expand All @@ -34,7 +32,9 @@ public override void WriteJson(JsonWriter writer, object value, JsonSerializer s
}
else
{
#pragma warning disable CS8604 // Possible null reference argument. //never null?
writer.WriteValue(memberName.ToConstantCase());
#pragma warning restore CS8604 // Possible null reference argument.
}
}
}
Expand Down
30 changes: 18 additions & 12 deletions src/Speckle.Sdk/Api/GraphQL/Serializer/MapConverter.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#nullable disable
using System.Diagnostics.CodeAnalysis;
using GraphQL;
using Speckle.Newtonsoft.Json;
Expand All @@ -8,7 +7,7 @@ namespace Speckle.Sdk.Api.GraphQL.Serializer;

internal sealed class MapConverter : JsonConverter<Map>
{
public override void WriteJson(JsonWriter writer, Map value, JsonSerializer serializer)
public override void WriteJson(JsonWriter writer, Map? value, JsonSerializer serializer)
{
throw new NotImplementedException(
"This converter currently is only intended to be used to read a JSON object into a strongly-typed representation."
Expand All @@ -18,7 +17,7 @@ public override void WriteJson(JsonWriter writer, Map value, JsonSerializer seri
public override Map ReadJson(
JsonReader reader,
Type objectType,
Map existingValue,
Map? existingValue,
bool hasExistingValue,
JsonSerializer serializer
)
Expand All @@ -39,16 +38,23 @@ JsonSerializer serializer
)]
private object ReadToken(JToken token)
{
return token switch
switch (token)
{
JObject jObject => ReadDictionary(jObject, new Dictionary<string, object>()),
JArray jArray => ReadArray(jArray).ToList(),
JValue jValue => jValue.Value,
JConstructor => throw new ArgumentOutOfRangeException(nameof(token), "cannot deserialize a JSON constructor"),
JProperty => throw new ArgumentOutOfRangeException(nameof(token), "cannot deserialize a JSON property"),
JContainer => throw new ArgumentOutOfRangeException(nameof(token), "cannot deserialize a JSON comment"),
_ => throw new ArgumentOutOfRangeException(nameof(token), $"Invalid token type {token?.Type}"),
};
case JObject jObject:
return ReadDictionary(jObject, new Dictionary<string, object>());
case JArray jArray:
return ReadArray(jArray).ToList();
case JValue jValue:
return jValue.Value ?? string.Empty;
case JConstructor:
throw new ArgumentOutOfRangeException(nameof(token), "cannot deserialize a JSON constructor");
case JProperty:
throw new ArgumentOutOfRangeException(nameof(token), "cannot deserialize a JSON property");
case JContainer:
throw new ArgumentOutOfRangeException(nameof(token), "cannot deserialize a JSON comment");
default:
throw new ArgumentOutOfRangeException(nameof(token), $"Invalid token type {token?.Type}");
}
}

private Dictionary<string, object> ReadDictionary(JToken element, Dictionary<string, object> to)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
#nullable disable
using System.Text;
using GraphQL;
using GraphQL.Client.Abstractions;
using GraphQL.Client.Abstractions.Websocket;
using Speckle.Newtonsoft.Json;
using Speckle.Newtonsoft.Json.Serialization;
using Speckle.Sdk.Common;
using Speckle.Sdk.Serialisation;

namespace Speckle.Sdk.Api.GraphQL.Serializer;
Expand Down Expand Up @@ -51,10 +51,9 @@ public Task<WebsocketMessageWrapper> DeserializeToWebsocketResponseWrapperAsync(

public GraphQLWebSocketResponse<TResponse> DeserializeToWebsocketResponse<TResponse>(byte[] bytes)
{
return JsonConvert.DeserializeObject<GraphQLWebSocketResponse<TResponse>>(
Encoding.UTF8.GetString(bytes),
JsonSerializerSettings
);
return JsonConvert
.DeserializeObject<GraphQLWebSocketResponse<TResponse>>(Encoding.UTF8.GetString(bytes), JsonSerializerSettings)
.NotNull();
JR-Morgan marked this conversation as resolved.
Show resolved Hide resolved
}

public Task<GraphQLResponse<TResponse>> DeserializeFromUtf8StreamAsync<TResponse>(
Expand All @@ -79,6 +78,6 @@ JsonSerializerSettings serializerSettings
using var sr = new StreamReader(stream);
using JsonReader reader = SpeckleObjectSerializerPool.Instance.GetJsonTextReader(sr);
var serializer = JsonSerializer.Create(serializerSettings);
return Task.FromResult(serializer.Deserialize<T>(reader));
return Task.FromResult(serializer.Deserialize<T>(reader) ?? throw new ArgumentException("Serialized data is null"));
}
}
11 changes: 5 additions & 6 deletions src/Speckle.Sdk/Credentials/Account.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#nullable disable
using System.Runtime.InteropServices;
using Speckle.Sdk.Api.GraphQL.Models;
using Speckle.Sdk.Helpers;
Expand All @@ -9,13 +8,13 @@ namespace Speckle.Sdk.Credentials;
[ComVisible(true)]
public class Account : IEquatable<Account>
{
private string _id;
private string? _id;

/// <remarks>
/// The account id is unique to user and server url.
/// </remarks>
/// <exception cref="InvalidOperationException">Account object invalid: missing required info</exception>
public string id
public string? id
adamhathcock marked this conversation as resolved.
Show resolved Hide resolved
{
get
{
Expand Down Expand Up @@ -48,7 +47,7 @@ public string id

private static string CleanURL(string server)
{
if (Uri.TryCreate(server, UriKind.Absolute, out Uri newUri))
if (Uri.TryCreate(server, UriKind.Absolute, out Uri? newUri))
{
server = newUri.Authority;
}
Expand Down Expand Up @@ -77,12 +76,12 @@ public override string ToString()
return $"Account ({userInfo.email} | {serverInfo.url})";
}

public bool Equals(Account other)
public bool Equals(Account? other)
{
return other is not null && other.userInfo.email == userInfo.email && other.serverInfo.url == serverInfo.url;
}

public override bool Equals(object obj)
public override bool Equals(object? obj)
{
return obj is Account acc && Equals(acc);
}
Expand Down
10 changes: 5 additions & 5 deletions src/Speckle.Sdk/Credentials/AccountManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ public IEnumerable<Account> GetAccounts()
acc.NotNull();
if (IsInvalid(acc))
{
RemoveAccount(acc.id);
RemoveAccount(acc.id.NotNull());
}
else
{
Expand Down Expand Up @@ -432,7 +432,7 @@ public async Task UpdateAccounts(CancellationToken ct = default)
}

ct.ThrowIfCancellationRequested();
_accountStorage.SaveObject(account.id, JsonConvert.SerializeObject(account));
_accountStorage.SaveObject(account.id.NotNull(), JsonConvert.SerializeObject(account));
}
}

Expand All @@ -449,7 +449,7 @@ public void RemoveAccount(string id)

if (accounts.Length != 0 && !accounts.Any(x => x.isDefault))
{
ChangeDefaultAccount(accounts.First().id);
ChangeDefaultAccount(accounts.First().id.NotNull());
}
}

Expand All @@ -469,7 +469,7 @@ public void ChangeDefaultAccount(string id)
{
account.isDefault = true;
}
_accountStorage.SaveObject(account.id, JsonConvert.SerializeObject(account));
_accountStorage.SaveObject(account.id.NotNull(), JsonConvert.SerializeObject(account));
}
}

Expand Down Expand Up @@ -709,7 +709,7 @@ public async Task AddAccount(Uri? server = null)
var account = await CreateAccount(accessCode, challenge, server).ConfigureAwait(false);

//if the account already exists it will not be added again
_accountStorage.SaveObject(account.id, JsonConvert.SerializeObject(account));
_accountStorage.SaveObject(account.id.NotNull(), JsonConvert.SerializeObject(account));
logger.LogDebug("Finished adding account {accountId} for {serverUrl}", account.id, server);
}
catch (SpeckleAccountManagerException ex)
Expand Down
7 changes: 3 additions & 4 deletions src/Speckle.Sdk/Host/Attributes.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#nullable disable
namespace Speckle.Sdk.Host;

[AttributeUsage(AttributeTargets.Constructor)]
Expand All @@ -7,17 +6,17 @@ public sealed class SchemaInfoAttribute : Attribute
public SchemaInfoAttribute(string name, string description)
: this(name, description, null, null) { }

public SchemaInfoAttribute(string name, string description, string category, string subcategory)
public SchemaInfoAttribute(string name, string description, string? category, string? subcategory)
{
Name = name;
Description = description;
Category = category;
Subcategory = subcategory;
}

public string Subcategory { get; }
public string? Subcategory { get; }

public string Category { get; }
public string? Category { get; }

public string Description { get; }

Expand Down
7 changes: 2 additions & 5 deletions src/Speckle.Sdk/Models/Base.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#nullable disable
using System.Collections;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
Expand All @@ -24,15 +23,13 @@ namespace Speckle.Sdk.Models;
[SuppressMessage("Style", "IDE1006:Naming Styles", Justification = "Serialized property names are camelCase by design")]
public class Base : DynamicBase, ISpeckleObject
{
private string _type;
private string? _type;

/// <summary>
/// A speckle object's id is an unique hash based on its properties. <b>NOTE: this field will be null unless the object was deserialised from a source. Use the <see cref="GetId"/> function to get it.</b>
/// </summary>
[SchemaIgnore]
public virtual string id { get; set; }

#nullable enable //Starting nullability syntax here so that `id` null oblivious,
public virtual string? id { get; set; }

/// <summary>
/// Secondary, ideally host application driven, object identifier.
Expand Down
10 changes: 5 additions & 5 deletions src/Speckle.Sdk/Models/Blob.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#nullable disable
using System.Runtime.Serialization;
using System.Runtime.Serialization;
using Speckle.Newtonsoft.Json;

namespace Speckle.Sdk.Models;
Expand Down Expand Up @@ -38,13 +37,13 @@ public string filePath
/// <summary>
/// For blobs, the id is the same as the file hash. Please note, when deserialising, the id will be set from the original hash generated on sending.
/// </summary>
public override string id
public override string? id
{
get => GetFileHash();
set => base.id = value;
}

public string GetFileHash()
public string? GetFileHash()
{
if ((_isHashExpired || _hash == null) && filePath != null)
{
Expand All @@ -63,6 +62,7 @@ internal void OnDeserialized(StreamingContext context)
public string GetLocalDestinationPath(string blobStorageFolder)
{
var fileName = Path.GetFileName(filePath);
return Path.Combine(blobStorageFolder, $"{id[..10]}-{fileName}");
var x = id ?? throw new ArgumentException("id is empty");
return Path.Combine(blobStorageFolder, $"{x[..10]}-{fileName}");
}
}
5 changes: 3 additions & 2 deletions src/Speckle.Sdk/Models/Extensions/BaseExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Collections;
using System.Diagnostics.Contracts;
using Speckle.Sdk.Common;
using Speckle.Sdk.Models.Collections;

namespace Speckle.Sdk.Models.Extensions;
Expand Down Expand Up @@ -31,7 +32,7 @@ public static IEnumerable<Base> Flatten(this Base root, BaseRecursionBreaker? re
root,
b =>
{
if (!cache.Add(b.id))
if (!cache.Add(b.id.NotNull()))
{
return true;
}
Expand All @@ -42,7 +43,7 @@ public static IEnumerable<Base> Flatten(this Base root, BaseRecursionBreaker? re

foreach (var b in traversal)
{
if (!cache.Contains(b.id))
if (!cache.Contains(b.id.NotNull()))
{
yield return b;
}
Expand Down
5 changes: 1 addition & 4 deletions src/Speckle.Sdk/Models/ISpeckleObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@

public interface ISpeckleObject
{
#nullable disable
public string id { get; }

#nullable enable //Starting nullability syntax here so that `id` null oblivious,
public string? id { get; }

public string? applicationId { get; }

Expand Down
2 changes: 1 addition & 1 deletion src/Speckle.Sdk/Serialisation/SpeckleObjectSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ private Id SerializeBaseObject(Base baseObj, JsonWriter writer, IReadOnlyDiction
}
else
{
id = new Id(((Blob)baseObj).id);
id = new Id(((Blob)baseObj).id.NotNull());
}
writer.WritePropertyName("id");
writer.WriteValue(id.Value);
Expand Down
4 changes: 2 additions & 2 deletions src/Speckle.Sdk/Serialisation/V2/Send/ObjectSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ private void SerializeProperty(object? obj, JsonWriter writer, PropertyAttribute
Json json;
if (_baseCache.TryGetValue(baseObj, out var info))
{
id = new Id(baseObj.id);
id = new Id(baseObj.id.NotNull());
childClosures = info.Closures;
json = info.Json;
MergeClosures(_currentClosures, childClosures);
Expand Down Expand Up @@ -311,7 +311,7 @@ private Id SerializeBaseObject(Base baseObj, JsonWriter writer, Closures closure
}
else
{
id = new Id(((Blob)baseObj).id);
id = new Id(((Blob)baseObj).id.NotNull());
}
writer.WritePropertyName("id");
writer.WriteValue(id.Value);
Expand Down
2 changes: 1 addition & 1 deletion src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public async Task<SerializeProcessResults> Serialize(Base root, CancellationToke
var channelTask = Start(cancellationToken);
await Traverse(root, true, cancellationToken).ConfigureAwait(false);
await channelTask.ConfigureAwait(false);
return new(root.id, _objectReferences.Freeze());
return new(root.id.NotNull(), _objectReferences.Freeze());
}

private async Task Traverse(Base obj, bool isEnd, CancellationToken cancellationToken)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ public async Task Roundtrip_Test_Old(string fileName, string rootId, int count)
var j = serializer.Serialize(base1);
//j.ShouldBe(objJson);
JToken.DeepEquals(JObject.Parse(j), JObject.Parse(objJson));
newIds.Add(base1.id, j);
newIds.Add(base1.id.NotNull(), j);
oldIds.Add(id, j);
idToBase.Add(id, base1);
}
Expand Down
Loading