From c5697b33e2f42651aecd74e647b58dc7bc6d8a8a Mon Sep 17 00:00:00 2001 From: Ske Date: Mon, 29 Jun 2020 14:15:30 +0200 Subject: [PATCH] Finally retire the PKMember setters! --- PluralKit.API/Controllers/v1/JsonModelExt.cs | 58 ++++++++++--------- .../Controllers/v1/MemberController.cs | 20 ++++--- PluralKit.Bot/Commands/MemberEdit.cs | 2 +- PluralKit.Bot/Commands/MemberProxy.cs | 2 - PluralKit.Core/Models/PKMember.cs | 40 ++++++------- PluralKit.Core/Models/Patch/MemberPatch.cs | 2 + PluralKit.Core/Services/DataFileService.cs | 12 ++-- PluralKit.Core/Utils/BulkImporter.cs | 42 +++++++------- PluralKit.Core/Utils/Partial.cs | 2 + 9 files changed, 94 insertions(+), 86 deletions(-) diff --git a/PluralKit.API/Controllers/v1/JsonModelExt.cs b/PluralKit.API/Controllers/v1/JsonModelExt.cs index ac92be48..1fa59c0d 100644 --- a/PluralKit.API/Controllers/v1/JsonModelExt.cs +++ b/PluralKit.API/Controllers/v1/JsonModelExt.cs @@ -85,60 +85,64 @@ namespace PluralKit.API return o; } - public static void ApplyJson(this PKMember member, JObject o) + public static MemberPatch ToMemberPatch(JObject o) { + var patch = new MemberPatch(); + if (o.ContainsKey("name") && o["name"].Type == JTokenType.Null) throw new JsonModelParseError("Member name can not be set to null."); - if (o.ContainsKey("name")) member.Name = o.Value("name").BoundsCheckField(Limits.MaxMemberNameLength, "Member name"); - if (o.ContainsKey("color")) member.Color = o.Value("color").NullIfEmpty()?.ToLower(); - if (o.ContainsKey("display_name")) member.DisplayName = o.Value("display_name").NullIfEmpty().BoundsCheckField(Limits.MaxMemberNameLength, "Member display name"); - if (o.ContainsKey("avatar_url")) member.AvatarUrl = o.Value("avatar_url").NullIfEmpty().BoundsCheckField(Limits.MaxUriLength, "Member avatar URL"); + if (o.ContainsKey("name")) patch.Name = o.Value("name").BoundsCheckField(Limits.MaxMemberNameLength, "Member name"); + if (o.ContainsKey("color")) patch.Color = o.Value("color").NullIfEmpty()?.ToLower(); + if (o.ContainsKey("display_name")) patch.DisplayName = o.Value("display_name").NullIfEmpty().BoundsCheckField(Limits.MaxMemberNameLength, "Member display name"); + if (o.ContainsKey("avatar_url")) patch.AvatarUrl = o.Value("avatar_url").NullIfEmpty().BoundsCheckField(Limits.MaxUriLength, "Member avatar URL"); if (o.ContainsKey("birthday")) { var str = o.Value("birthday").NullIfEmpty(); var res = DateTimeFormats.DateExportFormat.Parse(str); - if (res.Success) member.Birthday = res.Value; - else if (str == null) member.Birthday = null; + if (res.Success) patch.Birthday = res.Value; + else if (str == null) patch.Birthday = null; else throw new JsonModelParseError("Could not parse member birthday."); } - if (o.ContainsKey("pronouns")) member.Pronouns = o.Value("pronouns").NullIfEmpty().BoundsCheckField(Limits.MaxPronounsLength, "Member pronouns"); - if (o.ContainsKey("description")) member.Description = o.Value("description").NullIfEmpty().BoundsCheckField(Limits.MaxDescriptionLength, "Member descriptoin"); - if (o.ContainsKey("keep_proxy")) member.KeepProxy = o.Value("keep_proxy"); + if (o.ContainsKey("pronouns")) patch.Pronouns = o.Value("pronouns").NullIfEmpty().BoundsCheckField(Limits.MaxPronounsLength, "Member pronouns"); + if (o.ContainsKey("description")) patch.Description = o.Value("description").NullIfEmpty().BoundsCheckField(Limits.MaxDescriptionLength, "Member descriptoin"); + if (o.ContainsKey("keep_proxy")) patch.KeepProxy = o.Value("keep_proxy"); if (o.ContainsKey("prefix") || o.ContainsKey("suffix") && !o.ContainsKey("proxy_tags")) - member.ProxyTags = new[] {new ProxyTag(o.Value("prefix"), o.Value("suffix"))}; + patch.ProxyTags = new[] {new ProxyTag(o.Value("prefix"), o.Value("suffix"))}; else if (o.ContainsKey("proxy_tags")) { - member.ProxyTags = o.Value("proxy_tags") + patch.ProxyTags = o.Value("proxy_tags") .OfType().Select(o => new ProxyTag(o.Value("prefix"), o.Value("suffix"))) - .ToList(); + .ToArray(); } if(o.ContainsKey("privacy")) //TODO: Deprecate this completely in api v2 { var plevel = o.Value("privacy").ParsePrivacy("member"); - member.MemberVisibility = plevel; - member.NamePrivacy = plevel; - member.AvatarPrivacy = plevel; - member.DescriptionPrivacy = plevel; - member.BirthdayPrivacy = plevel; - member.PronounPrivacy = plevel; + patch.Visibility = plevel; + patch.NamePrivacy = plevel; + patch.AvatarPrivacy = plevel; + patch.DescriptionPrivacy = plevel; + patch.BirthdayPrivacy = plevel; + patch.PronounPrivacy = plevel; // member.ColorPrivacy = plevel; - member.MetadataPrivacy = plevel; + patch.MetadataPrivacy = plevel; } else { - if (o.ContainsKey("visibility")) member.MemberVisibility = o.Value("visibility").ParsePrivacy("member"); - if (o.ContainsKey("name_privacy")) member.NamePrivacy = o.Value("name_privacy").ParsePrivacy("member"); - if (o.ContainsKey("description_privacy")) member.DescriptionPrivacy = o.Value("description_privacy").ParsePrivacy("member"); - if (o.ContainsKey("avatar_privacy")) member.AvatarPrivacy = o.Value("avatar_privacy").ParsePrivacy("member"); - if (o.ContainsKey("birthday_privacy")) member.BirthdayPrivacy = o.Value("birthday_privacy").ParsePrivacy("member"); - if (o.ContainsKey("pronoun_privacy")) member.PronounPrivacy = o.Value("pronoun_privacy").ParsePrivacy("member"); + if (o.ContainsKey("visibility")) patch.Visibility = o.Value("visibility").ParsePrivacy("member"); + if (o.ContainsKey("name_privacy")) patch.NamePrivacy = o.Value("name_privacy").ParsePrivacy("member"); + if (o.ContainsKey("description_privacy")) patch.DescriptionPrivacy = o.Value("description_privacy").ParsePrivacy("member"); + if (o.ContainsKey("avatar_privacy")) patch.AvatarPrivacy = o.Value("avatar_privacy").ParsePrivacy("member"); + if (o.ContainsKey("birthday_privacy")) patch.BirthdayPrivacy = o.Value("birthday_privacy").ParsePrivacy("member"); + if (o.ContainsKey("pronoun_privacy")) patch.PronounPrivacy = o.Value("pronoun_privacy").ParsePrivacy("member"); // if (o.ContainsKey("color_privacy")) member.ColorPrivacy = o.Value("color_privacy").ParsePrivacy("member"); - if (o.ContainsKey("metadata_privacy")) member.MetadataPrivacy = o.Value("metadata_privacy").ParsePrivacy("member"); + if (o.ContainsKey("metadata_privacy")) patch.MetadataPrivacy = o.Value("metadata_privacy").ParsePrivacy("member"); } + + return patch; } private static string BoundsCheckField(this string input, int maxLength, string nameInError) diff --git a/PluralKit.API/Controllers/v1/MemberController.cs b/PluralKit.API/Controllers/v1/MemberController.cs index 6a893b4c..40875070 100644 --- a/PluralKit.API/Controllers/v1/MemberController.cs +++ b/PluralKit.API/Controllers/v1/MemberController.cs @@ -16,12 +16,14 @@ namespace PluralKit.API public class MemberController: ControllerBase { private IDataStore _data; + private IDatabase _db; private IAuthorizationService _auth; - public MemberController(IDataStore data, IAuthorizationService auth) + public MemberController(IDataStore data, IAuthorizationService auth, IDatabase db) { _data = data; _auth = auth; + _db = db; } [HttpGet("{hid}")] @@ -48,18 +50,18 @@ namespace PluralKit.API return BadRequest($"Member limit reached ({memberCount} / {Limits.MaxMemberCount})."); var member = await _data.CreateMember(system, properties.Value("name")); + MemberPatch patch; try { - member.ApplyJson(properties); + patch = JsonModelExt.ToMemberPatch(properties); } catch (JsonModelParseError e) { return BadRequest(e.Message); } - // TODO: retire SaveMember - await _data.SaveMember(member); - return Ok(member.ToJson(User.ContextFor(member))); + var newMember = await _db.Execute(conn => conn.UpdateMember(member.Id, patch)); + return Ok(member.ToJson(User.ContextFor(newMember))); } [HttpPatch("{hid}")] @@ -72,18 +74,18 @@ namespace PluralKit.API var res = await _auth.AuthorizeAsync(User, member, "EditMember"); if (!res.Succeeded) return Unauthorized($"Member '{hid}' is not part of your system."); + MemberPatch patch; try { - member.ApplyJson(changes); + patch = JsonModelExt.ToMemberPatch(changes); } catch (JsonModelParseError e) { return BadRequest(e.Message); } - // TODO: retire SaveMember - await _data.SaveMember(member); - return Ok(member.ToJson(User.ContextFor(member))); + var newMember = await _db.Execute(conn => conn.UpdateMember(member.Id, patch)); + return Ok(member.ToJson(User.ContextFor(newMember))); } [HttpDelete("{hid}")] diff --git a/PluralKit.Bot/Commands/MemberEdit.cs b/PluralKit.Bot/Commands/MemberEdit.cs index 3f8375d5..6706c4d4 100644 --- a/PluralKit.Bot/Commands/MemberEdit.cs +++ b/PluralKit.Bot/Commands/MemberEdit.cs @@ -100,10 +100,10 @@ namespace PluralKit.Bot var description = ctx.RemainderOrNull().NormalizeLineEndSpacing(); if (description.IsLongerThan(Limits.MaxDescriptionLength)) throw Errors.DescriptionTooLongError(description.Length); - target.Description = description; var patch = new MemberPatch {Description = Partial.Present(description)}; await _db.Execute(conn => conn.UpdateMember(target.Id, patch)); + await ctx.Reply($"{Emojis.Success} Member description changed."); } } diff --git a/PluralKit.Bot/Commands/MemberProxy.cs b/PluralKit.Bot/Commands/MemberProxy.cs index 2ab0e482..134e76a9 100644 --- a/PluralKit.Bot/Commands/MemberProxy.cs +++ b/PluralKit.Bot/Commands/MemberProxy.cs @@ -56,8 +56,6 @@ namespace PluralKit.Bot throw Errors.GenericCancelled(); } - target.ProxyTags = new ProxyTag[] { }; - var patch = new MemberPatch {ProxyTags = Partial.Present(new ProxyTag[0])}; await _db.Execute(conn => conn.UpdateMember(target.Id, patch)); diff --git a/PluralKit.Core/Models/PKMember.cs b/PluralKit.Core/Models/PKMember.cs index fd0026da..fa47dd06 100644 --- a/PluralKit.Core/Models/PKMember.cs +++ b/PluralKit.Core/Models/PKMember.cs @@ -9,27 +9,27 @@ namespace PluralKit.Core { public class PKMember { public MemberId Id { get; } - public string Hid { get; set; } - public SystemId System { get; set; } - public string Color { get; set; } - public string AvatarUrl { get; set; } - public string Name { get; set; } - public string DisplayName { get; set; } - public LocalDate? Birthday { get; set; } - public string Pronouns { get; set; } - public string Description { get; set; } - public ICollection ProxyTags { get; set; } - public bool KeepProxy { get; set; } - public Instant Created { get; set; } - public int MessageCount { get; set; } + public string Hid { get; } + public SystemId System { get; } + public string Color { get; } + public string AvatarUrl { get; } + public string Name { get; } + public string DisplayName { get; } + public LocalDate? Birthday { get; } + public string Pronouns { get; } + public string Description { get; } + public ICollection ProxyTags { get; } + public bool KeepProxy { get; } + public Instant Created { get; } + public int MessageCount { get; } - public PrivacyLevel MemberVisibility { get; set; } - public PrivacyLevel DescriptionPrivacy { get; set; } - public PrivacyLevel AvatarPrivacy { get; set; } - public PrivacyLevel NamePrivacy { get; set; } //ignore setting if no display name is set - public PrivacyLevel BirthdayPrivacy { get; set; } - public PrivacyLevel PronounPrivacy { get; set; } - public PrivacyLevel MetadataPrivacy { get; set; } + public PrivacyLevel MemberVisibility { get; } + public PrivacyLevel DescriptionPrivacy { get; } + public PrivacyLevel AvatarPrivacy { get; } + public PrivacyLevel NamePrivacy { get; } //ignore setting if no display name is set + public PrivacyLevel BirthdayPrivacy { get; } + public PrivacyLevel PronounPrivacy { get; } + public PrivacyLevel MetadataPrivacy { get; } // public PrivacyLevel ColorPrivacy { get; set; } /// Returns a formatted string representing the member's birthday, taking into account that a year of "0001" or "0004" is hidden diff --git a/PluralKit.Core/Models/Patch/MemberPatch.cs b/PluralKit.Core/Models/Patch/MemberPatch.cs index f1f63e56..aa1529f2 100644 --- a/PluralKit.Core/Models/Patch/MemberPatch.cs +++ b/PluralKit.Core/Models/Patch/MemberPatch.cs @@ -8,6 +8,7 @@ namespace PluralKit.Core { public Partial Name { get; set; } public Partial DisplayName { get; set; } + public Partial AvatarUrl { get; set; } public Partial Color { get; set; } public Partial Birthday { get; set; } public Partial Pronouns { get; set; } @@ -26,6 +27,7 @@ namespace PluralKit.Core public override UpdateQueryBuilder Apply(UpdateQueryBuilder b) => b .With("name", Name) .With("display_name", DisplayName) + .With("avatar_url", AvatarUrl) .With("color", Color) .With("birthday", Birthday) .With("pronouns", Pronouns) diff --git a/PluralKit.Core/Services/DataFileService.cs b/PluralKit.Core/Services/DataFileService.cs index ab4ab485..5e399ad8 100644 --- a/PluralKit.Core/Services/DataFileService.cs +++ b/PluralKit.Core/Services/DataFileService.cs @@ -72,12 +72,10 @@ namespace PluralKit.Core }; } - private PKMember ConvertMember(PKSystem system, DataFileMember fileMember) + private MemberPatch ToMemberPatch(DataFileMember fileMember) { - var newMember = new PKMember + var newMember = new MemberPatch { - Hid = fileMember.Id, - System = system.Id, Name = fileMember.Name, DisplayName = fileMember.DisplayName, Description = fileMember.Description, @@ -88,10 +86,10 @@ namespace PluralKit.Core }; if (fileMember.Prefix != null || fileMember.Suffix != null) - newMember.ProxyTags = new List {new ProxyTag(fileMember.Prefix, fileMember.Suffix)}; + newMember.ProxyTags = new[] {new ProxyTag(fileMember.Prefix, fileMember.Suffix)}; else // Ignore proxy tags where both prefix and suffix are set to null (would be invalid anyway) - newMember.ProxyTags = (fileMember.ProxyTags ?? new ProxyTag[] { }).Where(tag => !tag.IsEmpty).ToList(); + newMember.ProxyTags = (fileMember.ProxyTags ?? new ProxyTag[] { }).Where(tag => !tag.IsEmpty).ToArray(); if (fileMember.Birthday != null) { @@ -149,7 +147,7 @@ namespace PluralKit.Core _logger.Debug( "Importing member with identifier {FileId} to system {System} (is creating new member? {IsCreatingNewMember})", fileMember.Id, system.Id, isCreatingNewMember); - var newMember = await imp.AddMember(fileMember.Id, ConvertMember(system, fileMember)); + var newMember = await imp.AddMember(fileMember.Id, fileMember.Id, fileMember.Name, ToMemberPatch(fileMember)); if (isCreatingNewMember) result.AddedNames.Add(newMember.Name); diff --git a/PluralKit.Core/Utils/BulkImporter.cs b/PluralKit.Core/Utils/BulkImporter.cs index 32edad9c..bf7d764a 100644 --- a/PluralKit.Core/Utils/BulkImporter.cs +++ b/PluralKit.Core/Utils/BulkImporter.cs @@ -60,43 +60,45 @@ namespace PluralKit.Core /// /// If an existing member exists in this system that matches this member in either HID or name, it'll overlay the member information on top of this instead. /// An opaque identifier string that refers to this member regardless of source. Is used when importing switches. Value is irrelevant, but should be consistent with the same member later. - /// A member struct containing the data to apply to this member. Null fields will be ignored. + /// When trying to match the member to an existing member, will use a member with this HID if present in system. + /// When trying to match the member to an existing member, will use a member with this name if present in system. + /// A member patch struct containing the data to apply to this member /// The inserted member object, which may or may not share an ID or HID with the input member. - public async Task AddMember(string identifier, PKMember member) + public async Task AddMember(string identifier, string potentialHid, string potentialName, MemberPatch patch) { // See if we can find a member that matches this one // if not, roll a new hid and we'll insert one with that // (we can't trust the hid given in the member, it might let us overwrite another system's members) - var existingMember = FindExistingMemberInSystem(member.Hid, member.Name); + var existingMember = FindExistingMemberInSystem(potentialHid, potentialName); string newHid = existingMember?.Hid ?? await _conn.QuerySingleAsync("find_free_member_hid", commandType: CommandType.StoredProcedure); // Upsert member data and return the ID QueryBuilder qb = QueryBuilder.Upsert("members", "hid") .Constant("hid", "@Hid") - .Constant("system", "@System") - .Variable("name", "@Name") - .Variable("keep_proxy", "@KeepProxy"); + .Constant("system", "@System"); - if (member.DisplayName != null) qb.Variable("display_name", "@DisplayName"); - if (member.Description != null) qb.Variable("description", "@Description"); - if (member.Color != null) qb.Variable("color", "@Color"); - if (member.AvatarUrl != null) qb.Variable("avatar_url", "@AvatarUrl"); - if (member.ProxyTags != null) qb.Variable("proxy_tags", "@ProxyTags"); - if (member.Birthday != null) qb.Variable("birthday", "@Birthday"); + if (patch.Name.IsPresent) qb.Variable("name", "@Name"); + if (patch.DisplayName.IsPresent) qb.Variable("display_name", "@DisplayName"); + if (patch.Description.IsPresent) qb.Variable("description", "@Description"); + if (patch.Color.IsPresent) qb.Variable("color", "@Color"); + if (patch.AvatarUrl.IsPresent) qb.Variable("avatar_url", "@AvatarUrl"); + if (patch.ProxyTags.IsPresent) qb.Variable("proxy_tags", "@ProxyTags"); + if (patch.Birthday.IsPresent) qb.Variable("birthday", "@Birthday"); + if (patch.KeepProxy.IsPresent) qb.Variable("keep_proxy", "@KeepProxy"); var newMember = await _conn.QueryFirstAsync(qb.Build("returning *"), new { Hid = newHid, System = _systemId, - member.Name, - member.DisplayName, - member.Description, - member.Color, - member.AvatarUrl, - member.KeepProxy, - member.ProxyTags, - member.Birthday + Name = patch.Name.Value, + DisplayName = patch.DisplayName.Value, + Description = patch.Description.Value, + Color = patch.Color.Value, + AvatarUrl = patch.AvatarUrl.Value, + KeepProxy = patch.KeepProxy.Value, + ProxyTags = patch.ProxyTags.Value, + Birthday = patch.Birthday.Value }); // Log this member ID by the given identifier diff --git a/PluralKit.Core/Utils/Partial.cs b/PluralKit.Core/Utils/Partial.cs index 99eef13d..3ba2c0f6 100644 --- a/PluralKit.Core/Utils/Partial.cs +++ b/PluralKit.Core/Utils/Partial.cs @@ -30,6 +30,8 @@ namespace PluralKit.Core public IEnumerator GetEnumerator() => ToArray().GetEnumerator(); IEnumerator IEnumerable.GetEnumerator() => ToArray().GetEnumerator(); + + public static implicit operator Partial(T val) => Present(val); } public class PartialConverter: JsonConverter