Stirling-Tools/Stirling-PDF
1
0
Fork 0
mirror of https://github.com/Stirling-Tools/Stirling-PDF.git synced 2026-09-03 05:10:16 +03:00
Code Issues Packages Projects Releases Wiki Activity

Remove untenanted tool workflows endpoint and tighten usage tracking

Browse Source
This commit is contained in:
Anthony Stirling
2026-08-30 10:38:36 +01:00
parent 530085bd74
commit 5523cc9559
7 changed files with 83 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/proprietary/src/main/java/stirling/software/proprietary/controller/api/ToolRecommendationController.java
+5 -26
View File
@@ -4,6 +4,7 @@ import java.util.List;
import java.util.Optional;
import java.util.regex.Pattern;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
@@ -20,7 +21,6 @@ import stirling.software.common.annotations.api.ProprietaryUiDataApi;
import stirling.software.common.service.UserServiceInterface;
import stirling.software.proprietary.service.ToolRecommendationService;
import stirling.software.proprietary.service.ToolRecommendationService.ToolRecommendation;
import stirling.software.proprietary.service.ToolRecommendationService.ToolWorkflow;
import stirling.software.proprietary.service.ToolUsageTrackingService;
/**
@@ -40,8 +40,6 @@ public class ToolRecommendationController {
public record RecommendationsResponse(List<ToolRecommendation> recommendations) {}
public record WorkflowsResponse(List<ToolWorkflow> workflows) {}
/**
* @param priorChains the tools already applied to each input document, oldest step first and
* excluding this run - one entry per input document, empty for a fresh upload.
@@ -72,29 +70,6 @@ public class ToolRecommendationController {
}
}
@GetMapping("/tool-recommendations/workflows")
@Operation(
summary = "Get repeated tool workflows",
description =
"Ordered tool sequences that get applied to the same document over and over,"
+ " most repeated first. Intended as the basis for suggesting"
+ " automations, so each entry says whether the pattern is the"
+ " caller's own, their team's, or the whole install's.")
public ResponseEntity<WorkflowsResponse> getWorkflows(
@RequestParam(value = "minLength", defaultValue = "2") int minLength,
@RequestParam(value = "limit", defaultValue = "6") int limit,
@RequestHeader(value = "X-Browser-Id", required = false) String browserId) {
try {
return ResponseEntity.ok(
new WorkflowsResponse(
recommendationService.getWorkflows(
resolvePrincipal(browserId), minLength, limit)));
} catch (Exception e) {
log.warn("Failed to load tool workflows: {}", e.getMessage());
return ResponseEntity.ok(new WorkflowsResponse(List.of()));
}
}
@PostMapping("/tool-recommendations/usage")
@Operation(
summary = "Record a completed tool run",
@@ -108,6 +83,10 @@ public class ToolRecommendationController {
if (request == null || !ToolUsageTrackingService.isValidToolKey(request.toolKey())) {
return ResponseEntity.badRequest().build();
}
if (!trackingService.isRecordingEnabled()) {
// 501 latches the client, so a declining install stops receiving the posts at all.
return ResponseEntity.status(HttpStatus.NOT_IMPLEMENTED).build();
}
trackingService.recordUsage(
resolvePrincipal(browserId), request.toolKey(), request.priorChains());
return ResponseEntity.noContent().build();
app/proprietary/src/main/java/stirling/software/proprietary/service/ToolUsageTrackingService.java
+6 -2
View File
@@ -7,7 +7,6 @@ import java.util.Comparator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.regex.Pattern;
import org.springframework.dao.DataIntegrityViolationException;
@@ -57,6 +56,11 @@ public class ToolUsageTrackingService {
&& properties.getSystem().isAnalyticsEnabled();
}
/** Lets the controller answer 501 so the browser stops posting on a declining install. */
public boolean isRecordingEnabled() {
return isUsageDataAllowed(applicationProperties);
}
/**
* Counts one completed run of {@code toolKey} against each input document's history.
*
@@ -177,7 +181,7 @@ public class ToolUsageTrackingService {
}
/** Daily retention sweep; both tables hold one row per principal, key and day. */
@Scheduled(fixedDelay = 1, initialDelay = 1, timeUnit = TimeUnit.DAYS)
@Scheduled(cron = "0 15 3 * * *")
public void cleanupOldStats() {
int retentionDays = applicationProperties.getToolRecommendations().getRetentionDays();
if (retentionDays <= 0) {
app/proprietary/src/test/java/stirling/software/proprietary/controller/api/ToolRecommendationControllerTest.java
+43 -22
View File
@@ -1,13 +1,18 @@
package stirling.software.proprietary.controller.api;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
import java.util.List;
import java.util.Optional;
@@ -20,15 +25,14 @@ import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.http.ResponseEntity;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
import stirling.software.common.service.UserServiceInterface;
import stirling.software.proprietary.controller.api.ToolRecommendationController.RecommendationsResponse;
import stirling.software.proprietary.controller.api.ToolRecommendationController.UsageRequest;
import stirling.software.proprietary.controller.api.ToolRecommendationController.WorkflowsResponse;
import stirling.software.proprietary.service.ToolRecommendationService;
import stirling.software.proprietary.service.ToolRecommendationService.ToolRecommendation;
import stirling.software.proprietary.service.ToolRecommendationService.ToolWorkflow;
import stirling.software.proprietary.service.ToolRecommendationService.WorkflowScope;
import stirling.software.proprietary.service.ToolUsageTrackingService;
@ExtendWith(MockitoExtension.class)
@@ -36,6 +40,8 @@ class ToolRecommendationControllerTest {
private static final String BROWSER_ID = "0f8fad5b-d9cb-469f-a165-70867728950e";
private static final String BASE_PATH = "/api/v1/proprietary/ui-data/tool-recommendations";
/** One input document that has already been through compress. */
private static final List<List<String>> CHAIN = List.of(List.of("compress"));
@@ -46,12 +52,16 @@ class ToolRecommendationControllerTest {
@Mock private UserServiceInterface userService;
private ToolRecommendationController controller;
private MockMvc mockMvc;
@BeforeEach
void setUp() {
controller =
new ToolRecommendationController(
trackingService, recommendationService, Optional.of(userService));
mockMvc = MockMvcBuilders.standaloneSetup(controller).build();
// Lenient: the 400 and 404 cases never reach the consent check.
lenient().when(trackingService.isRecordingEnabled()).thenReturn(true);
}
@Nested
@@ -137,36 +147,47 @@ class ToolRecommendationControllerTest {
verify(trackingService).recordUsage("alice", "ocr", JUNK_CHAIN);
}
@Test
@DisplayName("an install that declined tracking answers 501 so the client stops posting")
void declinedInstallReturns501() {
when(trackingService.isRecordingEnabled()).thenReturn(false);
ResponseEntity<Void> response =
controller.recordUsage(new UsageRequest("ocr", CHAIN), null);
assertThat(response.getStatusCode().value()).isEqualTo(501);
verify(trackingService, never()).recordUsage(anyString(), anyString(), any());
}
}
@Nested
@DisplayName("GET workflows")
class GetWorkflows {
@DisplayName("Cross-principal exposure")
class CrossPrincipalExposure {
/** The chain queries carry no tenant predicate, so no HTTP route may reach them. */
@Test
@DisplayName("returns the repeated workflows for the resolved principal")
void returnsWorkflows() {
when(userService.getCurrentUsername()).thenReturn("alice");
ToolWorkflow workflow =
new ToolWorkflow(List.of("compress", "ocr"), 4, WorkflowScope.USER);
when(recommendationService.getWorkflows("alice", 2, 6)).thenReturn(List.of(workflow));
@DisplayName("no route serves other principals' chains")
void workflowsRouteIsGone() throws Exception {
mockMvc.perform(get(BASE_PATH + "/workflows").param("minLength", "2"))
.andExpect(status().isNotFound());
ResponseEntity<WorkflowsResponse> response = controller.getWorkflows(2, 6, null);
assertThat(response.getBody().workflows()).containsExactly(workflow);
verifyNoInteractions(recommendationService);
}
@Test
@DisplayName("a failure degrades to an empty list rather than breaking the caller")
void failureDegrades() {
when(userService.getCurrentUsername()).thenReturn("alice");
when(recommendationService.getWorkflows(anyString(), anyInt(), anyInt()))
.thenThrow(new RuntimeException("db down"));
@DisplayName("a spoofed browser id cannot read the logged-in caller out of their own scope")
void browserIdCannotOverrideLoggedInPrincipal() throws Exception {
when(userService.getCurrentUsername()).thenReturn("bob");
when(recommendationService.getRecommendations(eq("bob"), isNull(), anyInt()))
.thenReturn(List.of());
ResponseEntity<WorkflowsResponse> response = controller.getWorkflows(2, 6, null);
mockMvc.perform(get(BASE_PATH).header("X-Browser-Id", BROWSER_ID))
.andExpect(status().isOk());
assertThat(response.getStatusCode().value()).isEqualTo(200);
assertThat(response.getBody().workflows()).isEmpty();
verify(recommendationService).getRecommendations("bob", null, 6);
verify(recommendationService, never())
.getRecommendations(eq("anon:" + BROWSER_ID), any(), anyInt());
}
}
frontend/editor/src/core/api/toolRecommendations.test.ts
+13 -28
View File
@@ -3,7 +3,6 @@ import { describe, it, expect, vi, beforeEach } from "vitest";
import apiClient from "@app/services/apiClient";
import {
fetchToolRecommendations,
fetchToolWorkflows,
recordToolUsage,
resetToolRecommendationsAvailabilityForTests,
} from "@app/api/toolRecommendations";
@@ -19,6 +18,10 @@ const http404 = Object.assign(new Error("not found"), {
response: { status: 404 },
});
const http501 = Object.assign(new Error("not implemented"), {
response: { status: 501 },
});
describe("toolRecommendations api", () => {
beforeEach(() => {
vi.clearAllMocks();
@@ -71,33 +74,6 @@ describe("toolRecommendations api", () => {
});
});
describe("fetchToolWorkflows", () => {
it("returns repeated workflows and passes the filters", async () => {
mockGet.mockResolvedValue({
data: {
workflows: [
{ tools: ["compress", "watermark"], count: 4, scope: "USER" },
],
},
});
const result = await fetchToolWorkflows(3, 10);
expect(result).toEqual([
{ tools: ["compress", "watermark"], count: 4, scope: "USER" },
]);
const url = mockGet.mock.calls[0][0] as string;
expect(url).toContain("minLength=3");
expect(url).toContain("limit=10");
});
it("returns null on failure", async () => {
mockGet.mockRejectedValue(new Error("network down"));
expect(await fetchToolWorkflows()).toBeNull();
});
});
describe("recordToolUsage", () => {
it("posts the tool and each input document's prior chain", async () => {
mockPost.mockResolvedValue({});
@@ -136,5 +112,14 @@ describe("toolRecommendations api", () => {
expect(mockPost).toHaveBeenCalledTimes(1);
});
it("stops posting after a 501 from an install that declined tracking", async () => {
mockPost.mockRejectedValue(http501);
await recordToolUsage("ocr");
await recordToolUsage("ocr");
expect(mockPost).toHaveBeenCalledTimes(1);
});
});
});
frontend/editor/src/core/api/toolRecommendations.ts
-34
View File
@@ -69,37 +69,3 @@ export async function recordToolUsage(
markUnavailableOn404(error);
}
}
/** Where a repeated workflow was observed. */
export type WorkflowScope = "USER" | "TEAM" | "GLOBAL";
export interface ToolWorkflowDto {
tools: string[];
count: number;
scope: WorkflowScope;
}
/**
* Tool sequences applied to the same document over and over - the basis for
* suggesting an automation. Null when the backend cannot serve them.
*/
export async function fetchToolWorkflows(
minLength = 2,
limit = 6,
): Promise<ToolWorkflowDto[] | null> {
if (backendUnavailable) return null;
try {
const params = new URLSearchParams({
minLength: String(minLength),
limit: String(limit),
});
const response = await apiClient.get<{ workflows: ToolWorkflowDto[] }>(
`${BASE_PATH}/workflows?${params}`,
{ suppressErrorToast: true, skipAuthRedirect: true },
);
return response.data?.workflows ?? [];
} catch (error) {
markUnavailableOn404(error);
return null;
}
}
frontend/editor/src/core/hooks/tools/shared/useToolOperation.ts
+7 -1
View File
@@ -445,6 +445,8 @@ export const useToolOperation = <TParams>(
// Set by both branches so the usage tracker can move each document's
// tool chain from the inputs onto the outputs that replaced them.
let producedFileIds: FileId[] = [];
// An input that failed stays in the workbench, so it must keep its chain.
let consumedInputIds: FileId[] = [];
if (isVersionOp) {
// Output is a modified version of the input — link it to the input's version chain.
@@ -493,6 +495,7 @@ export const useToolOperation = <TParams>(
const toConsumeInputIds = successSourceIds.filter((id) =>
inputFileIds.includes(id),
);
consumedInputIds = toConsumeInputIds;
console.debug("[useToolOperation] Consuming files (version)", {
inputCount: inputFileIds.length,
toConsume: toConsumeInputIds.length,
@@ -557,6 +560,7 @@ export const useToolOperation = <TParams>(
const toConsumeInputIds = successSourceIds.filter((id) =>
inputFileIds.includes(id),
);
consumedInputIds = toConsumeInputIds;
console.debug("[useToolOperation] Consuming files (independent)", {
inputCount: inputFileIds.length,
toConsume: toConsumeInputIds.length,
@@ -598,7 +602,9 @@ export const useToolOperation = <TParams>(
// the outputs that replaced it.
notifyToolCompleted({
toolId: config.operationType,
inputs: inputStirlingFileStubs,
inputs: inputStirlingFileStubs.filter((stub) =>
consumedInputIds.includes(stub.id),
),
outputFileIds: producedFileIds,
});
}
frontend/editor/src/core/services/toolUsageTracker.test.ts
+9
View File
@@ -109,6 +109,15 @@ describe("toolUsageTracker", () => {
expect(getDocumentToolChain(uploaded("p2"))).toEqual(["compress", "split"]);
});
it("leaves an input the tool never consumed with its chain intact", () => {
run("compress", [uploaded("a")], ["a2"]);
run("compress", [uploaded("b")], ["b2"]);
// Only a2 succeeded, so b2 stays in the workbench and keeps its lineage.
run("ocr", [uploaded("a2")], ["a3"]);
expect(getDocumentToolChain(uploaded("b2"))).toEqual(["compress"]);
});
it("prefers the tracked chain over a stale persisted history", () => {
run("compress", [uploaded("a")], ["a2"]);