Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,67 @@ public void OnlyWhitelistedKeysTravel()
Assert.Equal(new[] { "username", "since", "need", "visible" }, tick.Select(kv => kv.Key).ToArray());
}

/// <summary>
/// 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.
/// </summary>
[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<JsonObject>(payload["lane"]);
Assert.Equal(new[] { "app", "sort", "rep_min", "new_authors" }, lane.Select(kv => kv.Key).ToArray());
Assert.Equal("peakd", lane["app"]!.GetValue<string>());
Assert.Equal("unique", lane["sort"]!.GetValue<string>());
Assert.Equal(100, lane["rep_min"]!.GetValue<int>());
Assert.Equal(new[] { "username", "author", "permlink", "state", "lane" }, payload.Select(kv => kv.Key).ToArray());
}

/// <summary>
/// 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.
/// </summary>
[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<JsonObject>(payload["lane"]);
Assert.Equal(travels, lane.ContainsKey("sort"));
Assert.False(lane.ContainsKey("seed"));
Assert.Equal("peakd", lane["app"]!.GetValue<string>());
}

[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<JsonObject>(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")]
Expand Down
62 changes: 61 additions & 1 deletion dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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" });

/// <summary>
/// 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.
/// </summary>
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" });

/// <summary>The roster-feed keys that describe WHICH posts, not how they are paged.</summary>
public static readonly string[] LaneKeys =
{
"view", "app", "community", "min_words", "sort", "window", "rep_min", "rep_max", "max_words",
Comment thread
coderabbitai[bot] marked this conversation as resolved.
"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" });

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -885,6 +906,38 @@ private static void NormalizeRosterFeed(JsonObject payload, JsonObject body)
Clamp(payload, "max_words", 0, CurationDeskQuery.MaxWords);
}

/// <summary>
/// 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 <see cref="LaneKeys"/> is copied at all.
/// </summary>
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;
}

/// <summary>Drop a field whose value is not one of <paramref name="allowed"/>.</summary>
private static void KeepAllowed(JsonObject payload, string key, IReadOnlySet<string> allowed)
{
Expand Down Expand Up @@ -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))
Expand Down
Loading