diff --git a/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs b/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs index f03f01f2..0ce218de 100644 --- a/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs +++ b/dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs @@ -128,6 +128,67 @@ public void OnlyWhitelistedKeysTravel() Assert.Equal(new[] { "username", "since", "need", "visible" }, tick.Select(kv => kv.Key).ToArray()); } + /// + /// A mark carries the lane the desk was showing. It is rebuilt from the allow + /// list and cleaned like a roster-feed body, so a key the feed does not know, + /// a value it would refuse, and the paging fields never reach the backend. + /// + [Fact] + public void MarkLaneIsRebuiltFromTheAllowListAndNormalized() + { + var payload = Ok(CurationDeskWrites.Mark, + "{\"author\":\"bob\",\"permlink\":\"p\",\"state\":\"reviewed\",\"lane\":{" + + "\"app\":\"peakd\",\"sort\":\"unique\",\"new_authors\":\"1\",\"rep_min\":250," + + "\"window\":\"bogus\",\"community\":\"../etc\",\"cursor\":\"c1\",\"limit\":5,\"seed\":\"abcd1234\"," + + "\"admin\":true,\"username\":\"mallory\"}}"); + var lane = Assert.IsType(payload["lane"]); + Assert.Equal(new[] { "app", "sort", "rep_min", "new_authors" }, lane.Select(kv => kv.Key).ToArray()); + Assert.Equal("peakd", lane["app"]!.GetValue()); + Assert.Equal("unique", lane["sort"]!.GetValue()); + Assert.Equal(100, lane["rep_min"]!.GetValue()); + Assert.Equal(new[] { "username", "author", "permlink", "state", "lane" }, payload.Select(kv => kv.Key).ToArray()); + } + + /// + /// The order travels, because it decides whether a position is a watermark: a mark + /// on newest-first says nothing about the older posts. The seed never travels, and + /// the backend reads the sort without its feed parser's seed rule. + /// + [Theory] + [InlineData("random", true)] + [InlineData("newest", true)] + [InlineData("queue", true)] + [InlineData("unique", true)] + [InlineData("payout", false)] + public void MarkLaneCarriesAKnownSortAndNeverTheSeed(string sort, bool travels) + { + var payload = Ok(CurationDeskWrites.Mark, + $"{{\"author\":\"bob\",\"permlink\":\"p\",\"state\":\"reviewed\",\"lane\":{{\"sort\":\"{sort}\",\"seed\":\"abcd1234\",\"app\":\"peakd\"}}}}"); + var lane = Assert.IsType(payload["lane"]); + Assert.Equal(travels, lane.ContainsKey("sort")); + Assert.False(lane.ContainsKey("seed")); + Assert.Equal("peakd", lane["app"]!.GetValue()); + } + + [Fact] + public void MarkWithoutALaneStaysWithoutOne() + { + var payload = Ok(CurationDeskWrites.Mark, "{\"author\":\"bob\",\"permlink\":\"p\",\"state\":\"reviewed\"}"); + Assert.False(payload.ContainsKey("lane")); + // and an empty object is a real answer: the whole queue + var whole = Ok(CurationDeskWrites.Mark, "{\"author\":\"bob\",\"permlink\":\"p\",\"state\":\"reviewed\",\"lane\":{}}"); + Assert.Empty(Assert.IsType(whole["lane"])); + } + + [Theory] + [InlineData("{\"author\":\"bob\",\"permlink\":\"p\",\"state\":\"reviewed\",\"lane\":\"peakd\"}")] + [InlineData("{\"author\":\"bob\",\"permlink\":\"p\",\"state\":\"reviewed\",\"lane\":[\"peakd\"]}")] + [InlineData("{\"author\":\"bob\",\"permlink\":\"p\",\"state\":\"reviewed\",\"lane\":7}")] + public void MarkLaneMustBeAnObject(string body) + { + Assert.Equal("lane must be an object", Rejected(CurationDeskWrites.Mark, body)); + } + [Theory] [InlineData("{\"permlink\":\"p\",\"state\":\"reviewed\"}", "author required")] [InlineData("{\"author\":\"\",\"permlink\":\"p\",\"state\":\"reviewed\"}", "author required")] diff --git a/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs b/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs index 717b896b..04f73aad 100644 --- a/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs +++ b/dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs @@ -777,8 +777,24 @@ public sealed record Route(string UpstreamPath, string[] Keys, bool ForwardClien public static readonly Route Tick = new("curation/desk/tick", new[] { "since", "need", "visible" }); + /// + /// A mark carries the feed params the desk that made it was showing, as one + /// `lane` object, so the hand-off can say which queue a position was earned in. + /// It travels on the mark rather than living per curator on purpose: two desks + /// on different filters then stamp each mark with their own, with nothing to + /// race over. The object is rebuilt from an allow list and normalized exactly + /// like a roster-feed body, minus paging, before it goes upstream. + /// public static readonly Route Mark = new("curation/desk/marks", - new[] { "author", "permlink", "state", "reason", "note", "snooze_until" }); + new[] { "author", "permlink", "state", "reason", "note", "snooze_until", "lane" }); + + /// The roster-feed keys that describe WHICH posts, not how they are paged. + public static readonly string[] LaneKeys = + { + "view", "app", "community", "min_words", "sort", "window", "rep_min", "rep_max", "max_words", + "has_images", "new_authors", "recommended", "flagged", "hide_curated", "hide_reviewed", + "hide_snoozed", + }; public static readonly Route MarkClear = new("curation/desk/marks/clear", new[] { "author", "permlink" }); @@ -828,6 +844,11 @@ public static (JsonObject? Payload, string? Error) Build(Route route, string use NormalizeRosterFeed(payload, body); } + if (ReferenceEquals(route, Mark) && body.Field("lane") is JsonObject lane) + { + payload["lane"] = NormalizeLane(lane); + } + if (ReferenceEquals(route, Tick)) { // The backend caps both lists at this many ids; truncating here @@ -885,6 +906,38 @@ private static void NormalizeRosterFeed(JsonObject payload, JsonObject body) Clamp(payload, "max_words", 0, CurationDeskQuery.MaxWords); } + /// + /// The lane a mark was made in, rebuilt from the allow list and cleaned with the + /// same rules as a roster-feed body. Nothing the feed would refuse reaches the + /// backend, and nothing outside is copied at all. + /// + private static JsonObject NormalizeLane(JsonObject lane) + { + var clean = new JsonObject(); + foreach (var key in LaneKeys) + { + CopyIfPresent(clean, lane, key); + } + // The order travels, because it decides whether a position is a watermark + // at all: a mark on newest-first says nothing about the older posts. The seed + // does not travel, and the backend reads the sort off the lane on its own + // rather than through its feed parser, so random without a seed is fine here. + var sort = lane.Str("sort"); + if (sort == null || !RosterSorts.Contains(sort)) + { + clean.Remove("sort"); + } + KeepAllowed(clean, "view", RosterViews); + KeepAllowed(clean, "app", CurationDeskQuery.Apps); + KeepAllowed(clean, "window", CurationDeskQuery.Windows); + KeepMatching(clean, "community", CurationDeskQuery.IsCommunity); + Clamp(clean, "rep_min", 0, 100); + Clamp(clean, "rep_max", 0, 100); + Clamp(clean, "min_words", 0, CurationDeskQuery.MaxWords); + Clamp(clean, "max_words", 0, CurationDeskQuery.MaxWords); + return clean; + } + /// Drop a field whose value is not one of . private static void KeepAllowed(JsonObject payload, string key, IReadOnlySet allowed) { @@ -954,6 +1007,13 @@ private static void Truncate(JsonObject payload, string key, int max) { if (ReferenceEquals(route, Mark)) { + // The lane is optional, and absent is the honest answer for a desk that + // predates it; but a lane that is not an object is a client bug, not a + // lane, and the backend would only store NULL for it anyway. + if (body.TryGetPropertyValue("lane", out var lane) && lane is not JsonObject) + { + return "lane must be an object"; + } return RequireAuthorPermlink(body) ?? RequireOneOf(body, "state", MarkStates); } if (ReferenceEquals(route, MarkClear))