diff --git a/.github/workflows/lua-tests.yml b/.github/workflows/lua-tests.yml new file mode 100644 index 0000000..4d4fe65 --- /dev/null +++ b/.github/workflows/lua-tests.yml @@ -0,0 +1,37 @@ +name: Lua tests + +on: + push: + pull_request: + branches: + - main + workflow_dispatch: + +permissions: + contents: read + +jobs: + test: + name: Lua 5.1 / Busted + runs-on: ubuntu-latest + timeout-minutes: 10 + + steps: + - name: Check out repository + uses: actions/checkout@v6 + + - name: Install Lua 5.1.5 + uses: leafo/gh-actions-lua@v13 + with: + luaVersion: "5.1.5" + + - name: Install LuaRocks + uses: leafo/gh-actions-luarocks@v6 + + - name: Install Busted + run: luarocks install busted 2.3.0-1 + + - name: Run tests + run: | + chmod +x tests/run_tests.sh + make test \ No newline at end of file diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..2b83a11 --- /dev/null +++ b/Makefile @@ -0,0 +1,10 @@ +.PHONY: test clean + +# Run the Busted unit test suite for plugin modules. +# Delegates to run_tests.sh (the authoritative runner) so flags are only defined once. +test: + ./tests/run_tests.sh + +# Remove generated/test artifacts +clean: + rm -f tests/spec/*.gc* 2>/dev/null || true diff --git a/lightroom-llama.lrplugin/Common.lua b/lightroom-llama.lrplugin/Common.lua index 068a419..9f61f68 100644 --- a/lightroom-llama.lrplugin/Common.lua +++ b/lightroom-llama.lrplugin/Common.lua @@ -1,587 +1,111 @@ --- Common.lua — Shared utilities for Lightroom Llama plugin ---- Loaded by LrLlama.lua, BatchLrLlama.lua, and ResetMetadata.lua. Provides thumbnail ---- export, Ollama API communication, model discovery, keyword management, and server ---- address validation. All public functions are exported via the return table at EOF. +--- Common.lua — Shared utilities for Lightroom Llama plugin (delegation layer). +--- Loads the focused modules and re-exports them under the original names used by +--- LrLlama.lua, BatchLrLlama.lua, and ResetMetadata.lua. This allows a zero-downtime +--- migration: entry points continue calling Common.X() while logic lives in dedicated +--- modules (OllamaClient, ThumbnailService, PromptBuilder, ResponseValidator, +--- MetadataService). Once verified, the delegation wrappers can be removed and +--- entry points updated to load modules directly. --- ---- SDK imports: LrHttp, LrLogger, LrFileUtils, LrStringUtils, LrTasks, LrPrefs +--- For testing, the exported `new` constructor accepts fake focused modules so that +--- delegation behavior can be verified without the Lightroom SDK. -local LrHttp = import 'LrHttp' -local LrPathUtils = import 'LrPathUtils' -local LrLogger = import 'LrLogger' -local LrFileUtils = import 'LrFileUtils' -local LrStringUtils = import 'LrStringUtils' -local LrTasks = import "LrTasks" local LrPrefs = import "LrPrefs" - -local logger = LrLogger('LrLlama') -logger:enable("logfile") -- Logs to ~/Documents/LrClassicLogs | tail -f LrLlama.log - ---- Default Ollama model name; change here to switch models. ----@type string -local model = "gemma4:latest" - ---- Fallback server address when prefs are missing, empty, or invalid. ----@type string -local defaultServerHost = "localhost:11434" - --------------------------------------------------------------------------------- --- Validates that a string is in "host:port" form (IP, hostname, or localhost) --- Returns (true, normalized_host) or (false, error_message) --------------------------------------------------------------------------------- - ---- Validate and normalize an Ollama server address. ---- Strips optional scheme (`http://`) and trailing slashes, then verifies the ---- string is `hostname:port`. Accepts localhost, IP addresses, and domain names. ----@param input string|nil User-supplied server address (may be empty) ----@return bool ok True when the address is valid ----@return string result The normalized host:port on success, error message on failure -local function validateServerHost(input) - if not input or input == "" then - return true, defaultServerHost -- treat empty as "use default" - end - -- Strip scheme and trailing slashes for normalization - local host = string.gsub(input, "^https?://", "") - host = string.gsub(host, "/+$", "") - - -- Must match something:digit (hostname or IP followed by colon and port) - local hostnamePart, portPart = string.match(host, "^([^:]+):(%d+)$") - if not hostnamePart or not portPart then - return false, "Server address must be in host:port format (e.g., localhost:11434)" - end - - local port = tonumber(portPart) - if port < 1 or port > 65535 then - return false, "Port number must be between 1 and 65535" - end - - -- hostnamePart should contain at least one valid character (alphanumeric, dot, hyphen) - if not string.match(hostnamePart, "^[%a%d%.%-]+$") then - return false, "Hostname contains invalid characters" - end - - return true, hostnamePart .. ":" .. portPart -end - ---- Build the base URL for Ollama API calls. ---- Reads `prefs.ollamaServerHost`, validates it, and falls back to ---- `defaultServerHost` if the stored value is malformed. Always returns a valid URL. ----@return string Base URL (e.g., "http://localhost:11434") -local function getOllamaBaseUrl() - local prefs = LrPrefs.prefsForPlugin() - local ok, host = validateServerHost(prefs.ollamaServerHost) - - if not ok then - logger:warn("Invalid server host in prefs: " .. tostring(prefs.ollamaServerHost)) - host = defaultServerHost - end - - return "http://" .. host -end - -JSON = (assert(loadfile(LrPathUtils.child(_PLUGIN.path, "JSON.lua"))))() - --------------------------------------------------------------------------------- --- Model discovery --------------------------------------------------------------------------------- - ---- Convert a plain list of model names into the `{title, value}` format that ---- LrView `popup_menu` requires for its `items` binding. ----@param modelNames table List of model strings (from fetchAvailableModels) ----@return table Table of `{title = m, value = m}` entries for popup_menu binding -local function makeModelItems(modelNames) - local items = {} - for _, m in ipairs(modelNames) do - table.insert(items, { title = m, value = m }) - end - return items -end - ---- Query Ollama's `/api/tags` endpoint for installed models. ---- Falls back to `{model}` (the default) on network errors, malformed JSON, ---- or an empty model list — so callers never receive nil. ----@return table List of model name strings; always non-empty. -local function fetchAvailableModels() - logger:info("Fetching available models from Ollama") - - local url = getOllamaBaseUrl() .. "/api/tags" - local response = LrHttp.get(url) - - if response then - local decodeSuccess, data = pcall(function() - return JSON:decode(response) - end) - - if decodeSuccess and data and type(data.models) == "table" then - local modelList = {} - - for _, availableModel in ipairs(data.models) do - local modelName = availableModel.name or availableModel.model - - if modelName and modelName ~= "" then - table.insert(modelList, modelName) - end - end - - if #modelList > 0 then - logger:info("Found " .. #modelList .. " models") - return modelList - end - - logger:warn("Ollama returned an empty model list") - elseif not decodeSuccess then - logger:warn( - "Failed to decode Ollama model response: " .. tostring(data) - ) - else - logger:warn("Ollama response did not contain a valid model list") - end - else - logger:warn("No response received from Ollama") - end - - logger:warn("Using default model: " .. model) - return { model } -end - ---- Persist the server host and rebuild the model dropdown. ---- MUST be called from within `LrTasks.startAsyncTask()` — LrBinding only ---- propagates property reassignments (modelItems, selectedModel) when done ---- off the UI thread. ----@param props table LrBinding property table containing serverHost, modelItems, selectedModel ----@param prefs table LrPrefs prefsForPlugin() instance ----@return bool ok True on success ----@return string message Human-readable status or error description -local function saveServerAndRefresh(props, prefs) - local ok, validatedHost = validateServerHost(props.serverHost) - if not ok then - return false, validatedHost -- caller displays error - end - - prefs.ollamaServerHost = validatedHost - - local availableModels = fetchAvailableModels() - if not availableModels or #availableModels == 0 then - return false, "No models found on server" - end - - -- Rebuild model list. This function is intended to be called from inside - -- LrTasks.startAsyncTask() so that LrBinding picks up the property - -- reassignment and propagates it to the dialog's popup_menu. - local oldSelection = props.selectedModel or "" - props.modelItems = makeModelItems(availableModels) - - -- Keep old selection if it still exists in the new list - local found = false - for _, m in ipairs(availableModels) do - if m == oldSelection then found = true; break; end - end - props.selectedModel = found and oldSelection or availableModels[1] - - return true, string.format("Loaded %d model(s)", #availableModels) -end - ---- Export a 512×512 JPEG thumbnail to a unique temp file. ---- **Side effect:** writes a file to the system temp directory. ---- Caller is responsible for deleting the returned file after use. ----@param photo LrPhoto Photo object from the catalog ----@return string|nil Absolute path on success, nil on failure -local function exportThumbnail(photo) - local tempPath = LrFileUtils.chooseUniqueFileName(LrPathUtils.getStandardFilePath('temp') .. "/thumbnail.jpg") - logger:info("Attempting to export thumbnail to: " .. tempPath) - - -- Validate that the temp directory exists and is accessible before proceeding - -- This prevents silent failures when the temp directory is missing or restricted - local tempDir = LrPathUtils.getStandardFilePath('temp') - if not LrFileUtils.exists(tempDir) then - logger:error("Temp directory does not exist: " .. tempDir) - return nil - end - - -- Track whether the thumbnail callback successfully wrote data - -- This flag is set inside the callback to communicate success back to the caller - local thumbnailSaved = false - -- Request a 512x512 JPEG thumbnail asynchronously - -- photo:requestJpegThumbnail() returns (success, result) and executes the callback - -- with the JPEG binary data if available - local success, result = photo:requestJpegThumbnail(512, 512, function(jpegData) - if jpegData then - -- Open temp file in binary write mode ("wb") to preserve JPEG data integrity - local tempFile = io.open(tempPath, "wb") - if tempFile then - tempFile:write(jpegData) - tempFile:close() - thumbnailSaved = true - logger:info("Thumbnail saved to " .. tempPath) - return true - else - logger:error("Could not open temp file for writing: " .. tempPath) - return false - end - else - logger:error("No JPEG data received from photo") - return false - end - end) - - -- Verify both the API call succeeded AND the callback wrote the file - if success and thumbnailSaved then - -- Final verification: ensure the file actually exists on disk - -- This catches edge cases where the write appeared successful but the file is missing - if LrFileUtils.exists(tempPath) then - logger:info("Thumbnail export successful: " .. tempPath) - return tempPath - else - logger:error("Thumbnail file was not created: " .. tempPath) - return nil - end - else - logger:warn("Failed to export thumbnail. Success: " .. tostring(success) .. ", Result: " .. tostring(result)) - return nil - end -end - --------------------------------------------------------------------------------- --- Base64 encoding --------------------------------------------------------------------------------- - ---- Read a JPEG file and return its Base64-encoded representation. ----@param imagePath string Absolute path to the JPEG file ----@return string|nil Base64 string on success, nil if file unreadable or empty -local function base64EncodeImage(imagePath) - logger:info("Attempting to encode image: " .. imagePath) - - -- Check if file exists - if not LrFileUtils.exists(imagePath) then - logger:error("Image file does not exist: " .. imagePath) - return nil - end - - local file = io.open(imagePath, "rb") - if not file then - logger:error("Could not open file for reading: " .. imagePath) - return nil - end - - local binaryData = file:read("*all") - file:close() - - if not binaryData or #binaryData == 0 then - logger:error("No data read from file: " .. imagePath) - return nil - end - - local base64Data = LrStringUtils.encodeBase64(binaryData) - if not base64Data then - logger:error("Failed to encode image to base64: " .. imagePath) - return nil - end - - logger:info("Successfully encoded image to base64. Size: " .. #binaryData .. " bytes") - return base64Data -end - --------------------------------------------------------------------------------- --- Default system prompt (the more detailed version from BatchLrLlama.lua) --------------------------------------------------------------------------------- - -local defaultSystemPrompt = [[# Image Metadata Generation Prompt - -You are an expert content curator specializing in creating compelling, accurate metadata for visual content. Your task is to analyze the provided image/video and generate a JSON object with three components: title, caption, and keywords. - -## Output Format -Return your response as a valid JSON object with this exact structure: -```json -{ - "title": "string", - "caption": "string", - "keywords": ["string", "string", "string"] -} -``` - -## Guidelines - -### Title Requirements -- **Length**: 5-12 words maximum -- **Style**: Write as a descriptive headline, not a sentence -- **Content**: Capture the main subject, action, and context -- **Focus**: Answer "what is happening" in the most compelling way -- **Avoid**: Generic terms, keyword stuffing, colons, redundant phrases -- **Include**: Specific details like location, time of day, or unique elements when relevant - -**Good examples:** -- "Mountain climber reaching summit during golden hour" -- "Children playing soccer in urban park" -- "Vintage red bicycle against brick wall" - -### Caption Requirements -- **Length**: 15-40 words -- **Style**: Complete sentences that expand on the title -- **Content**: Provide context, mood, or story behind the image -- **Focus**: Add emotional resonance or background information -- **Avoid**: Repeating the exact title wording -- **Include**: Atmosphere, setting details, or cultural context when relevant - -**Good example:** -*Title: "Street musician performing violin solo in subway station"* -*Caption: "A talented violinist captivates commuters with classical music during evening rush hour, creating a moment of beauty in the bustling underground transit hub."* - -### Keywords Requirements -- **Quantity**: 10-30 keywords (aim for 15-20 for optimal results) -- **Hierarchy**: Order from most specific to more general -- **Categories**: Include subjects, actions, emotions, locations, styles, colors, concepts -- **Format**: Single words or short phrases (2-3 words max) -- **Avoid**: Repeating title/caption words exactly, overly generic terms, technical camera specs - -**Keyword categories to consider:** -- Primary subjects (people, objects, animals) -- Actions and verbs -- Emotions and moods -- Locations and settings -- Colors and lighting -- Art styles or techniques -- Concepts and themes -- Seasonal or temporal elements - -## Quality Checklist -Before finalizing, ensure: -- [ ] Title is unique and descriptive without being generic -- [ ] Caption adds meaningful context beyond the title -- [ ] Keywords cover multiple relevant categories -- [ ] No unnecessary repetition across all three elements -- [ ] JSON format is valid and properly structured -- [ ] Content accurately reflects what's actually in the image - -## Example Output -```json -{ - "title": "Barista creating latte art in cozy downtown cafe", - "caption": "Skilled coffee artist carefully pours steamed milk to create an intricate leaf pattern, showcasing the craftsmanship behind specialty coffee culture in a warm, inviting neighborhood coffee shop.", - "keywords": ["barista", "latte art", "coffee shop", "cafe culture", "milk foam", "artisan", "beverage preparation", "downtown", "craftsmanship", "morning routine", "specialty coffee", "hospitality", "small business", "urban lifestyle", "food service"] -} -``` -]] +local LrPathUtils = import 'LrPathUtils' -------------------------------------------------------------------------------- --- API communication ---- Export a thumbnail, encode it as Base64, and POST to Ollama's `/api/generate`. ---- Retries thumbnail export up to 3 times. Cleans up the temp file after the ---- HTTP request regardless of success or failure. Validates the JSON response ---- has title (string), caption (string), and keywords (array of non-empty strings). ----@param photo LrPhoto Photo object from the catalog ----@param prompt string User-facing prompt text ----@param currentData table|nil Existing title/caption to prepend when useCurrentData is true ----@param useCurrentData boolean Whether to include current metadata in the prompt ----@param useSystemPrompt boolean Whether to send a system prompt alongside the user prompt ----@param selectedModel string|nil Model name; falls back to the default if nil ----@param systemPrompt string|nil Override for the built-in system prompt ----@return table|nil response Parsed JSON on success ({title, caption, keywords}) ----@return string|nil err Error message, or nil on success -local function sendDataToApi(photo, prompt, currentData, useCurrentData, useSystemPrompt, selectedModel, systemPrompt) - logger:info("Sending data to API") - - -- Try to export thumbnail with retry - local thumbnailPath = nil - for attempt = 1, 3 do - thumbnailPath = exportThumbnail(photo) - if thumbnailPath then - break - end - logger:warn("Thumbnail export attempt " .. attempt .. " failed, retrying...") - LrTasks.sleep(0.5) -- Wait 500ms before retry - end - - if not thumbnailPath then - return nil, "Failed to export thumbnail after 3 attempts" - end - - local encodedImage = base64EncodeImage(thumbnailPath) - if not encodedImage then - return nil, "Failed to encode image" - end - - local url = getOllamaBaseUrl() .. "/api/generate" - - -- Choose system prompt: caller-provided > default - local activeSystemPrompt = systemPrompt or defaultSystemPrompt - - local postData = { - model = selectedModel or model, - prompt = (useCurrentData and "Title: "..(currentData.title or ""):gsub('"', '\\"') .. " Caption: "..(currentData.caption or ""):gsub('"', '\\"') .. " " .. prompt) or prompt, - format = "json", - system = useSystemPrompt and activeSystemPrompt or nil, - images = {encodedImage}, - stream = false +--- Factory — build a Common compatibility object from dependency modules. +--- Production code uses the default instance returned at module load time; +--- tests call `common.new(deps)` with fake modules to verify delegation. +--- +--- All delegations use explicit wrapper functions (not direct reference +--- assignments) so that test harnesses can swap out fake implementations +--- between construction and invocation and observe correct forwarding. +---@param deps table Dependency map: +--- { ollamaClient, thumbnailService, metadataService, prefsService } +--- - ollamaClient: OllamaClient module (defaultModel, defaultServerHost, +--- validateServerHost, makeModelItems, saveServerAndRefresh, fetchModels, generate) +--- - thumbnailService: ThumbnailService module (export, encodeBase64) +--- - metadataService: MetadataService module +--- (addKeywordsWithParent, getLlmKeywordsFromPhoto, removeLlmKeywords, parseKeywordCsv) +--- - prefsService: LrPrefs module with prefsForPlugin() +---@return table Common compatibility object with all expected exports +local function createCommon(deps) + local ollamaClient = assert(deps.ollamaClient, "deps.ollamaClient is required") + local thumbnailService = assert(deps.thumbnailService, "deps.thumbnailService is required") + local metadataService = assert(deps.metadataService, "deps.metadataService is required") + local prefsService = assert(deps.prefsService, "deps.prefsService is required") + + return { + -- OllamaClient constants + model = ollamaClient.defaultModel, + defaultServerHost = ollamaClient.defaultServerHost, + + -- OllamaClient functions — explicit wrappers for testability. + -- Direct reference assignment would capture the function at construction + -- time, preventing tests from verifying delegation behavior. + validateServerHost = function(...) + return ollamaClient.validateServerHost(...) + end, + makeModelItems = function(...) + return ollamaClient.makeModelItems(...) + end, + saveServerAndRefresh = function(...) + return ollamaClient.saveServerAndRefresh(...) + end, + + -- fetchAvailableModels wraps client.fetchModels to provide no-arg compatibility + fetchAvailableModels = function() + return ollamaClient.fetchModels(prefsService.prefsForPlugin()) + end, + + -- ThumbnailService functions — explicit wrappers for testability. + exportThumbnail = function(...) + return thumbnailService.export(...) + end, + base64EncodeImage = function(...) + return thumbnailService.encodeBase64(...) + end, + + -- OllamaClient.generate is the decomposition of sendDataToApi; same 7-param signature. + sendDataToApi = function(photo, prompt, currentData, useCurrentData, + useSystemPrompt, selectedModel, systemPrompt) + return ollamaClient.generate(photo, prompt, currentData, useCurrentData, + useSystemPrompt, selectedModel, systemPrompt) + end, + + -- MetadataService functions — explicit wrappers for testability. + addKeywordsWithParent = function(...) + return metadataService.addKeywordsWithParent(...) + end, + getLlmKeywordsFromPhoto = function(...) + return metadataService.getLlmKeywordsFromPhoto(...) + end, + removeLlmKeywords = function(...) + return metadataService.removeLlmKeywords(...) + end, + parseKeywordCsv = function(...) + return metadataService.parseKeywordCsv(...) + end, } - - local jsonPayload = JSON:encode(postData) - - local response, headers = LrHttp.post(url, jsonPayload, {{ - field = "Content-Type", - value = "application/json" - }}) - - -- Clean up thumbnail file - LrFileUtils.delete(thumbnailPath) - - if response then - -- Decode outer Ollama API response - local decodeOk1, response_data = pcall(function() - return JSON:decode(response) - end) - - if not decodeOk1 - or type(response_data) ~= "table" - or type(response_data.response) ~= "string" then - logger:warn("Invalid API response structure: " .. tostring(response_data)) - return nil, "Invalid API response structure" - end - - local rawResponse = response_data.response - - -- Many Ollama models emit markdown-wrapped JSON ("```json\n{...}\n```"). - -- Strip the fences so JSON:decode receives bare JSON. Two patterns handle - -- both ```json and bare ``` variants. - rawResponse = string.gsub(rawResponse, "^%s*```%w+\n?", "") - rawResponse = string.gsub(rawResponse, "\n?```%s*$", "") - rawResponse = string.gsub(rawResponse, "^%s*```%s*\n?", "") - rawResponse = string.gsub(rawResponse, "\n?```%s*$", "") - - -- Decode the model-generated JSON - local decodeOk2, response_json = pcall(function() - return JSON:decode(rawResponse) - end) - - if not decodeOk2 or type(response_json) ~= "table" then - logger:warn("Invalid JSON in API response: " .. tostring(response_json)) - return nil, "Invalid JSON in API response" - end - - -- Validate required fields exist with correct types - if type(response_json.title) ~= "string" or - type(response_json.caption) ~= "string" or - type(response_json.keywords) ~= "table" then - logger:warn("API response missing required fields (title, caption, keywords)") - return nil, "API response missing required fields (title, caption, keywords)" - end - - -- Validate each keyword entry is a non-empty string - for _, kw in ipairs(response_json.keywords) do - if type(kw) ~= "string" or kw:match("^%s*$") then - logger:warn("API response contains an invalid keyword entry") - return nil, "API response contains an invalid keyword" - end - end - - return response_json, nil - else - return nil, "Failed to send data to the API" - end -end - --------------------------------------------------------------------------------- --- Keyword management --------------------------------------------------------------------------------- - ---- Add keywords under the `llm` parent keyword. ---- Creates (or retrieves) a top-level `llm` keyword, then adds each entry as a child ---- and associates it with the photo. Idempotent — safe to call on photos that ---- already have LLM keywords. ----@param catalog LrCatalog Active catalog (from LrApplication.activeCatalog()) ----@param photo LrPhoto Target photo ----@param keywords table Array of keyword strings -local function addKeywordsWithParent(catalog, photo, keywords) - if not keywords or type(keywords) ~= "table" then - return - end - - -- First create or get the parent 'llm' keyword - local llmKeyword = catalog:createKeyword("llm", nil, true, nil, true) - if not llmKeyword then - error("Failed to create or get 'llm' parent keyword") - end - - for _, keyword in ipairs(keywords) do - if keyword and keyword ~= "" then - -- Create child keyword under 'llm' parent - local childKeyword = catalog:createKeyword(keyword, nil, true, llmKeyword, true) - if childKeyword then - photo:addKeyword(childKeyword) - else - logger:warn("Failed to create keyword: " .. tostring(keyword)) - end - end - end end ---- Read existing LLM-generated keywords from a photo. ---- Filters all keywords to only those whose parent is `llm`. Wrapped in ---- `pcall` so that malformed keyword data doesn't crash the plugin. ----@param photo LrPhoto Target photo ----@return table Array of keyword name strings (may be empty) -local function getLlmKeywordsFromPhoto(photo) - local llmKeywords = {} - - -- Wrap in pcall to catch any errors - local success, result = pcall(function() - local allKeywords = photo:getRawMetadata("keywords") - - if allKeywords then - for _, keyword in ipairs(allKeywords) do - local parent = keyword:getParent() - if parent and parent:getName() == "llm" then - table.insert(llmKeywords, keyword:getName()) - end - end - end - end) - - if not success then - logger:warn("Error getting LLM keywords: " .. tostring(result)) - return {} -- Return empty array on error - end - - return llmKeywords -end - ---- Remove all `llm`-parented keywords from a photo. ---- Only removes the keyword-to-photo association — does not delete the keyword ---- definitions from the catalog (other photos may reference them). ----@param catalog LrCatalog Active catalog ----@param photo LrPhoto Target photo -local function removeLlmKeywords(catalog, photo) - local allKeywords = photo:getRawMetadata("keywords") - if not allKeywords then - return - end - - local keywordsToRemove = {} - for _, keyword in ipairs(allKeywords) do - local parent = keyword:getParent() - if parent and parent:getName() == "llm" then - table.insert(keywordsToRemove, keyword) - end - end - - for _, keyword in ipairs(keywordsToRemove) do - photo:removeKeyword(keyword) - end -end - --------------------------------------------------------------------------------- ---- Public API — imported by LrLlama.lua, BatchLrLlama.lua, ResetMetadata.lua --------------------------------------------------------------------------------- - -return { - model = model, - defaultServerHost = defaultServerHost, - validateServerHost = validateServerHost, - makeModelItems = makeModelItems, - fetchAvailableModels = fetchAvailableModels, - saveServerAndRefresh = saveServerAndRefresh, - exportThumbnail = exportThumbnail, - base64EncodeImage = base64EncodeImage, - sendDataToApi = sendDataToApi, - addKeywordsWithParent = addKeywordsWithParent, - getLlmKeywordsFromPhoto = getLlmKeywordsFromPhoto, - removeLlmKeywords = removeLlmKeywords, -} +-- Load focused modules for the default production instance. +local OllamaClient = (assert(loadfile(LrPathUtils.child(_PLUGIN.path, "OllamaClient.lua"))))() +local ThumbnailService = (assert(loadfile(LrPathUtils.child(_PLUGIN.path, "ThumbnailService.lua"))))() +local MetadataService = (assert(loadfile(LrPathUtils.child(_PLUGIN.path, "MetadataService.lua"))))() + +-- Create the default production compatibility object. +local common = createCommon({ + ollamaClient = OllamaClient, + thumbnailService = ThumbnailService, + metadataService = MetadataService, + prefsService = LrPrefs, +}) + +--- Create a new Common compatibility object with custom dependencies. +--- Used by tests to inject fake focused modules. Production callers ignore this. +---@param deps table Dependency map with ollamaClient, thumbnailService, metadataService, prefsService +---@return table New Common compatibility object +common.new = createCommon + +return common diff --git a/lightroom-llama.lrplugin/LlamaDialog.lua b/lightroom-llama.lrplugin/LlamaDialog.lua new file mode 100644 index 0000000..72857b8 --- /dev/null +++ b/lightroom-llama.lrplugin/LlamaDialog.lua @@ -0,0 +1,186 @@ +--- LlamaDialog.lua — Lightroom view adapter for single-photo metadata generation. +--- Owns all LrView / LrBinding / LrDialogs construction: property table, UI layout, +--- button actions, and modal presentation. Delegates workflow logic to the injected +--- controller so that business rules are testable without Lightroom. + +local LrDialogs = import "LrDialogs" +local LrView = import "LrView" +local LrTasks = import "LrTasks" +local LrFunctionContext = import "LrFunctionContext" +local LrBinding = import "LrBinding" +local LrColor = import "LrColor" +local LrPathUtils = import 'LrPathUtils' + +--- Map semantic status kinds to Lightroom display colors. +local STATUS_COLORS = { + success = LrColor(0.149, 0.616, 0.412), -- green + working = LrColor(0.439, 0.345, 0.745), -- purple + error = LrColor(0.8, 0.2, 0.2), -- red +} + +-------------------------------------------------------------------------------- +--- Build and show the modal dialog. +---@param deps table Dependency map: +--- { client, common, promptBuilder, prefs, catalog, photo, thumbnailPath, +--- statusMessages, writeMetadata } +--- `writeMetadata` wraps catalog:withWriteAccessDo for the save flow. +-------------------------------------------------------------------------------- +local function show(deps) + local ControllerModule = + assert(loadfile(LrPathUtils.child(_PLUGIN.path, "SinglePhotoController.lua")))() + local controller = ControllerModule.new(deps) + + LrFunctionContext.callWithContext("showLlamaDialog", function(context) + local props = LrBinding.makePropertyTable(context) + + -- Populate binding table from controller initial state. The same table + -- is passed back to controller actions so they mutate the live bindings. + local initialState = controller.initState() + for k, v in pairs(initialState) do + props[k] = v + end + + -- Create a view factory + local f = LrView.osFactory() + + -- Define the dialog contents (preserving exact current layout and sizing) + local c = f:view{ + bind_to_object = props, + f:row{f:column{ + f:picture{ + value = deps.thumbnailPath, + frame_width = 2, + width = 400, + height = 400 + }, + width = 400 + }, f:spacer{ width = 10 }, f:column{ + + -- Title field + f:column{f:static_text{ title = "Title:" }, + f:spacer{ f:label_spacing{} }, + f:edit_field{ value = LrView.bind("title"), width = 400 }}, + f:spacer{ height = 10 }, + + -- Caption field + f:static_text{ title = "Caption:", alignment = 'left' }, + f:spacer{ f:label_spacing{} }, + f:edit_field{ value = LrView.bind("caption"), width = 400, height = 100 }, + f:spacer{ height = 10 }, + + -- Keywords field + f:static_text{ title = "Keywords:", alignment = 'left' }, + f:spacer{ f:label_spacing{} }, + f:edit_field{ value = LrView.bind("keywords"), width = 400, height = 60 }, + f:spacer{ height = 10 }, + + f:separator{ width = 400 }, + f:spacer{ height = 10 }, + + -- Prompt field + f:static_text{ title = "Prompt:", alignment = 'left' }, + f:spacer{ f:label_spacing{} }, + f:edit_field{ value = LrView.bind("prompt"), width = 400, height = 60 }, + f:spacer{ height = 10 }, + + -- Checkboxes + f:checkbox{ title = "Use current title and caption", + value = LrView.bind("useCurrentData") }, + f:spacer{ height = 10 }, + f:checkbox{ title = "Use system prompt", + value = LrView.bind("useSystemPrompt") }, + f:spacer{ height = 10 }, + + f:separator{ width = 400 }, + f:spacer{ height = 10 }, + f:separator{ width = 400 }, + f:spacer{ height = 10 }, + + -- Server host + f:static_text{ title = "Ollama Server:", alignment = 'left' }, + f:spacer{ f:label_spacing{} }, + f:edit_field{ value = LrView.bind("serverHost"), width = 250 }, + f:static_text{ title = "(default: localhost:11434)", + alignment = 'left', text_color = LrColor(0.6, 0.6, 0.6) }, + f:spacer{ height = 10 }, + + -- Model dropdown + f:static_text{ title = "Model:", alignment = 'left' }, + f:spacer{ f:label_spacing{} }, + f:popup_menu{ + value = LrView.bind("selectedModel"), + items = LrView.bind("modelItems"), + width = 250, + }, + f:spacer{ height = 10 }, + + -- Status text with dynamic color + f:row{f:static_text{ fill_horizontal = 1 }, + f:static_text{ + alignment = 'right', + title = LrView.bind("status"), + width = 200, + font = "", + text_color = LrView.bind("statusColor") + }}, + f:spacer{ height = 10 }, + + -- Action buttons + f:row{ + f:push_button{ + title = "Generate", + action = function() + LrTasks.startAsyncTask(function() + controller.generate(props) + props.statusColor = STATUS_COLORS[props.statusKind] + or STATUS_COLORS.success + end) + end + }, + f:spacer{ width = 10 }, + f:push_button{ + title = "Save Server", + action = function() + LrTasks.startAsyncTask(function() + controller.saveServer(props) + props.statusColor = STATUS_COLORS[props.statusKind] + or STATUS_COLORS.success + end) + end + }}, + f:spacer{ height = 20 }, + + width = 400 + }} + } + + -- Show the dialog + local result = LrDialogs.presentModalDialog({ + title = "Lightroom Llama", + contents = c, + actionVerb = "Save" + }) + + if result == "ok" then + local saveResult = controller.saveMetadata(props) + + if not saveResult.ok then + -- Validation failure — show error; controller did NOT write prefs/data + LrDialogs.message( + "Invalid Server Address", + saveResult.message + .. "\n\nPlease enter it as host:port (e.g., localhost:11434 or 192.168.1.10:11434).", + "warning" + ) + else + LrDialogs.message( + "Metadata Saved", + "Title, caption, and keywords have been saved to the photo.", + "info" + ) + end + end + end) +end + +return { show = show } diff --git a/lightroom-llama.lrplugin/LrLlama.lua b/lightroom-llama.lrplugin/LrLlama.lua index 31846c6..70b21c2 100644 --- a/lightroom-llama.lrplugin/LrLlama.lua +++ b/lightroom-llama.lrplugin/LrLlama.lua @@ -1,355 +1,100 @@ ---- LrLlama.lua — Single-photo metadata generation. ---- Invoked from Library or Export menus. Exports a thumbnail, sends it to Ollama, ---- and presents the generated title, caption, and keywords in an editable dialog. +--- LrLlama.lua — Single-photo metadata generation entry point. +--- Invoked from Library or Export menus. Gets the active catalog and selected photo, +--- builds dependencies for the controller and dialog, and starts the plugin task. local LrApplication = import "LrApplication" -local LrDialogs = import "LrDialogs" -local LrView = import "LrView" -local LrTasks = import "LrTasks" -local LrFunctionContext = import "LrFunctionContext" -local LrBinding = import "LrBinding" -local LrColor = import "LrColor" -local LrPrefs = import "LrPrefs" -local LrPathUtils = import 'LrPathUtils' +local LrDialogs = import "LrDialogs" +local LrTasks = import "LrTasks" +local LrPrefs = import "LrPrefs" +local LrPathUtils = import 'LrPathUtils' --- Load shared utilities (includes logger, model constant, JSON loader, shared functions) +-- Common provides utility functions (parseKeywordCsv, addKeywordsWithParent, etc.) +-- and a delegation layer to OllamaClient for generate/fetchModels operations. local Common = (assert(loadfile(LrPathUtils.child(_PLUGIN.path, "Common.lua"))))() +-- PromptBuilder for the single-photo system prompt override. +local PromptBuilder = (assert(loadfile(LrPathUtils.child(_PLUGIN.path, "PromptBuilder.lua"))))() + +--- Build a client adapter that exposes the interface SinglePhotoController expects. +--- Common delegates to OllamaClient internally; this adapter maps method names and +--- adapts signatures so the controller has a stable, testable contract. +local function makeClientAdapter(common) + return { + --- Delegate to Common.sendDataToApi with matching 7-param signature. + generate = function(photo, userInstruction, currentData, useCurrentData, + useSystemPrompt, selectedModel, systemPromptOverride) + return common.sendDataToApi(photo, userInstruction, currentData, + useCurrentData, useSystemPrompt, + selectedModel, systemPromptOverride) + end, + + --- Delegate to Common.fetchAvailableModels (no-arg variant). + fetchModels = function() + return common.fetchAvailableModels() + end, + + -- Direct delegation — signatures match what the controller calls. + validateServerHost = common.validateServerHost, + saveServerAndRefresh = common.saveServerAndRefresh, + makeModelItems = common.makeModelItems, + + -- Constants from OllamaClient via Common. + defaultModel = common.model, + defaultServerHost = common.defaultServerHost, + } +end ---- Legacy system prompt used only by the single-photo dialog. ---- Simpler and more permissive than Common.defaultSystemPrompt — allows broader ---- keyword ranges and retains existing metadata verbatim. Passed to sendDataToApi ---- as the systemPrompt override so single-photo mode differs from batch mode. -local originalSystemPrompt = [[You are an AI tasked with creating a JSON object containing a `title`, a `caption`, and a list of `keywords` based on a given piece of content (such as an image or video). ]] .. -[[The content currently has the following metadata which you need to implement and improve upon. It is important to keep the title and caption as close to this as possible. - -Please follow these detailed guidelines for creating excellent metadata: - -1. **Title (Description):** - - Provide a unique, descriptive title for the content. - - The title should answer the Who, What, When, Where, and Why of the content. - - It should be written as a sentence or phrase, similar to a news headline, capturing the key details, mood, and emotions of the scene. - - Do not list keywords in the title. Avoid repetition of words and phrases. - - Include helpful details such as the angle, focus, and perspective if relevant. - - Do not include :. - - If given, use the current title as a starting point. - -2. **Caption:** - - Provide a more detailed description or context for the content. This can be a fuller explanation of the title, including any relevant background or emotional tone that helps convey the essence of the scene. - - If given, use the current caption as a starting point. - -3. **Keywords:** - - Provide a list of 7 to 50 keywords. - - Keywords should be specific and directly related to the content. - - Include broader topics, feelings, concepts, or associations represented by the content. - - Avoid using unrelated terms or repeating words or compound words. - - Do not include links, camera information, or trademarks unless required for editorial content. - -### JSON Format: -```json -{ - "title": "string", - "caption": "string", - "keywords": ["string"] -} -``` - -### Example: -```json -{ - "title": "A serene sunset over a peaceful beach with golden skies", - "caption": "A calm evening beach scene with a golden sunset reflecting on the ocean waves, creating a peaceful and tranquil mood. The horizon is clear with soft, pastel colors blending into the blue sky.", - "keywords": ["sunset", "beach", "calm", "ocean", "serene", "golden skies", "peaceful", "tranquil", "pastel colors", "horizon", "evening"] -} -``` - -Use this structure and guidelines to generate titles, captions, and keywords that are descriptive, unique, and accurate.]] - --- Note: saveServerAndRefresh moved to Common.lua — used by both dialogs - ---- Entry point. Invoked via `LrTasks.startAsyncTask(main)` — required by the ---- Lightroom SDK for menu-activated plugins. Shows a modal dialog that lets the ---- user generate and edit title, caption, and keywords for one photo. +--- Entry point. Obtains catalog and selected photo, exports a thumbnail, +--- constructs dependencies, and delegates to LlamaDialog for the UI flow. local function main() local catalog = LrApplication.activeCatalog() local selectedPhotos = catalog:getTargetPhotos() + if #selectedPhotos == 0 then - LrDialogs.message("No photo selected", "Please select a photo to view.", "critical") + LrDialogs.message( + "No photo selected", + "Please select a photo to view.", + "critical" + ) return end - -- Single-photo mode only processes the first selection; batch mode handles multiples. - local selectedPhoto = selectedPhotos[1] - local thumbnailPath = Common.exportThumbnail(selectedPhoto) + -- Single-photo mode only processes the first selection. + local photo = selectedPhotos[1] - -- LrBinding requires a live function context for property tables. Without it, - -- bound UI elements (edit fields, checkboxes, status text) won't update live. - LrFunctionContext.callWithContext("showLlamaDialog", function(context) - local props = LrBinding.makePropertyTable(context) - local prefs = LrPrefs.prefsForPlugin() - props.status = "Ready" - props.statusColor = LrColor(0.149, 0.616, 0.412) - props.prompt = "Caption this photo" - props.title = selectedPhoto:getFormattedMetadata('title') - props.caption = selectedPhoto:getFormattedMetadata('caption') - -- Initialize keywords with existing llm keywords - local existingLlmKeywords = Common.getLlmKeywordsFromPhoto(selectedPhoto) - props.keywords = table.concat(existingLlmKeywords, ", ") - props.response = "" - -- Pre-check so the "Use current title and caption" checkbox is only enabled - -- when there's actually existing metadata to include. - props.useCurrentData = props.title ~= "" or props.caption ~= "" - props.serverHost = prefs.ollamaServerHost or Common.defaultServerHost - props.useSystemPrompt = true + -- Export thumbnail (may fail; dialog will show nothing but won't crash) + local thumbnailPath = Common.exportThumbnail(photo) - -- Fetch available models from Ollama and populate the dropdown - local availableModels = Common.fetchAvailableModels() - props.modelItems = Common.makeModelItems(availableModels) - props.selectedModel = availableModels[1] -- default to first available model - - -- Create a view factory - local f = LrView.osFactory() - - -- Define the dialog contents - local c = f:view{ - bind_to_object = props, - f:row{f:column{ - f:picture{ - value = thumbnailPath, - frame_width = 2, - width = 400, - height = 400 - }, - width = 400 - }, f:spacer{ - width = 10 - }, f:column{ - f:column{f:static_text{ - title = "Title:" - }, f:spacer{f:label_spacing{}}, f:edit_field{ - value = LrView.bind("title"), -- Bind to the new response property - width = 400 - }}, - f:spacer{ - height = 10 - }, - f:static_text{ - title = "Caption:", - alignment = 'left' - }, - f:spacer{f:label_spacing{}}, - f:edit_field{ - value = LrView.bind("caption"), -- Bind to the new response property - width = 400, - height = 100 - }, - f:spacer{ - height = 10 - }, - f:static_text{ - title = "Keywords:", - alignment = 'left' - }, - f:spacer{f:label_spacing{}}, - f:edit_field{ - value = LrView.bind("keywords"), - width = 400, - height = 60 - }, - f:spacer{ - height = 10 - }, - f:separator{ - width = 400 - }, - f:spacer{ - height = 10 - }, - f:static_text{ - title = "Prompt:", - alignment = 'left' - }, - f:spacer{f:label_spacing{}}, - f:edit_field{ - value = LrView.bind("prompt"), - width = 400, - height = 60 - }, - f:spacer{ - height = 10 - }, - f:checkbox{ - title = "Use current title and caption", - value = LrView.bind("useCurrentData") - }, - f:spacer{ - height = 10 - }, - f:checkbox{ - title = "Use system prompt", - value = LrView.bind("useSystemPrompt") - }, - f:spacer{ - height = 10 - }, - f:separator{ - width = 400 - }, - f:spacer{ - height = 10 - }, - f:separator{ - width = 400 - }, - f:spacer{ - height = 10 - }, - f:static_text{ - title = "Ollama Server:", - alignment = 'left' - }, - f:spacer{f:label_spacing{}}, - f:edit_field{ - value = LrView.bind("serverHost"), - width = 250, - }, - f:static_text{ - title = "(default: localhost:11434)", - alignment = 'left', - text_color = LrColor(0.6, 0.6, 0.6) - }, - f:spacer{ - height = 10 - }, - f:static_text{ - title = "Model:", - alignment = 'left' - }, - f:spacer{f:label_spacing{}}, - f:popup_menu{ - value = LrView.bind("selectedModel"), - items = LrView.bind("modelItems"), - width = 250, - }, - f:spacer{ - height = 10 - }, - f:row{f:static_text{ - fill_horizontal = 1 - }, f:static_text{ - alignment = 'right', - title = LrView.bind("status"), - width = 200, - font = "", - text_color = LrView.bind("statusColor") - }}, - f:spacer{ - height = 10 - }, - f:row{f:push_button{ - title = "Generate", - action = function() - props.status = "The llama is thinking..." - props.statusColor = LrColor(0.439, 0.345, 0.745) - - -- API call must run off the UI thread. LrBinding properties can - -- be mutated from this async callback — they'll propagate live - -- to the dialog while we're still inside callWithContext above. - LrTasks.startAsyncTask(function() - local apiResponse, apiError = Common.sendDataToApi( - selectedPhoto, - props.prompt, - { title = props.title, caption = props.caption }, - props.useCurrentData, - props.useSystemPrompt, - props.selectedModel, - originalSystemPrompt - ) - if apiError then - props.status = "Error: " .. apiError - props.statusColor = LrColor(0.8, 0.2, 0.2) - return - end - props.response = apiResponse - props.title = apiResponse.title - props.caption = apiResponse.caption - -- Convert keywords array to comma-separated string for display - if apiResponse.keywords and type(apiResponse.keywords) == "table" then - props.keywords = table.concat(apiResponse.keywords, ", ") - else - props.keywords = "" - end - props.status = "Ready" - props.statusColor = LrColor(0.149, 0.616, 0.412) - end) - end - }, f:spacer{ - width = 10 - }, f:push_button{ - title = "Save Server", - -- saveServerAndRefresh must run async so LrBinding picks up modelItems changes - action = function() - props.status = "Refreshing models..." - props.statusColor = LrColor(0.439, 0.345, 0.745) - - LrTasks.startAsyncTask(function() - local ok, msg = Common.saveServerAndRefresh(props, prefs) - - if not ok then - props.status = "Error: " .. msg - props.statusColor = LrColor(0.8, 0.2, 0.2) - else - props.status = msg - props.statusColor = LrColor(0.149, 0.616, 0.412) - end - end) - end - }}, - f:spacer{ - height = 20 - }, - width = 400 - }} - } - - -- Show the dialog - local result = LrDialogs.presentModalDialog({ - title = "Lightroom Llama", - contents = c, - actionVerb = "Save" - }) - - - if result == "ok" then - -- Validate and save server host preference - local ok, validatedHost = Common.validateServerHost(props.serverHost) - if not ok then - LrDialogs.message( - "Invalid Server Address", - validatedHost .. "\n\nPlease enter it as host:port (e.g., localhost:11434 or 192.168.1.10:11434).", - "warning" - ) - else - prefs.ollamaServerHost = validatedHost + -- Build the catalog-write adapter. Ensures title, caption, and keyword + -- mutations happen inside a single catalog transaction. + local function writeMetadata(catalog, photo, metadata) + catalog:withWriteAccessDo("Save Llama metadata", function() + photo:setRawMetadata("title", metadata.title) + photo:setRawMetadata("caption", metadata.caption) + if metadata.keywords and #metadata.keywords > 0 then + Common.addKeywordsWithParent(catalog, photo, metadata.keywords) end + end) + end - -- All catalog mutations must run inside withWriteAccessDo — Lightroom - -- wraps the callback in a transaction and silently discards changes outside it. - catalog:withWriteAccessDo("Save Llama metadata", function() - selectedPhoto:setRawMetadata("title", props.title) - selectedPhoto:setRawMetadata("caption", props.caption) - -- Parse keywords from comma-separated string and add with llm parent - if props.keywords and props.keywords ~= "" then - local keywordList = {} - for keyword in string.gmatch(props.keywords, "([^,]+)") do - table.insert(keywordList, keyword:match("^%s*(.-)%s*$")) -- trim whitespace - end - Common.addKeywordsWithParent(catalog, selectedPhoto, keywordList) - end - end) - - LrDialogs.message("Metadata Saved", "Title, caption, and keywords have been saved to the photo.", "info") - end - end) + -- Load the dialog adapter (which in turn creates the controller). + local LlamaDialog = + assert(loadfile(LrPathUtils.child(_PLUGIN.path, "LlamaDialog.lua")))() + + LlamaDialog.show({ + client = makeClientAdapter(Common), + common = Common, + promptBuilder = PromptBuilder, + prefs = LrPrefs.prefsForPlugin(), + catalog = catalog, + photo = photo, + thumbnailPath = thumbnailPath, + statusMessages = { + ready = "Ready", + generating = "The llama is thinking...", + refreshing = "Refreshing models...", + }, + writeMetadata = writeMetadata, + }) end LrTasks.startAsyncTask(main) diff --git a/lightroom-llama.lrplugin/MetadataService.lua b/lightroom-llama.lrplugin/MetadataService.lua new file mode 100644 index 0000000..750ad2b --- /dev/null +++ b/lightroom-llama.lrplugin/MetadataService.lua @@ -0,0 +1,117 @@ +--- MetadataService.lua — Read and write photo metadata (title, caption, keywords). +--- Manages the `llm` parent keyword hierarchy: creating it, adding children, reading +--- existing keywords, and removing llm-parented keywords from photos. +--- SDK imports: LrLogger only. Catalog and photo objects are passed as parameters. + +local LrLogger = import 'LrLogger' + +local logger = LrLogger('LrLlama') + +local metadata = {} + +--- Add keywords under the `llm` parent keyword (idempotent). +--- Creates (or retrieves) a top-level `llm` keyword, then adds each entry as a child +--- and associates it with the photo. Safe to call on photos that already have LLM +--- keywords — duplicate keyword names are handled by Lightroom's createKeyword API. +---@param catalog LrCatalog Active catalog (from LrApplication.activeCatalog()) +---@param photo LrPhoto Target photo +---@param keywords table Array of keyword strings to add +function metadata.addKeywordsWithParent(catalog, photo, keywords) + if not keywords or type(keywords) ~= "table" then + return + end + + -- First create or get the parent 'llm' keyword + local llmKeyword = catalog:createKeyword("llm", nil, true, nil, true) + if not llmKeyword then + error("Failed to create or get 'llm' parent keyword") + end + + for _, keyword in ipairs(keywords) do + if keyword and keyword ~= "" then + -- Create child keyword under 'llm' parent + local childKeyword = catalog:createKeyword(keyword, nil, true, llmKeyword, true) + if childKeyword then + photo:addKeyword(childKeyword) + else + logger:warn("Failed to create keyword: " .. tostring(keyword)) + end + end + end +end + +--- Read existing llm-parented keywords from a photo. +--- Filters all keywords to only those whose parent is `llm`. Wrapped in +--- `pcall` so that malformed keyword data doesn't crash the plugin. +---@param photo LrPhoto Target photo +---@return table Keyword name strings (may be empty) +function metadata.getLlmKeywordsFromPhoto(photo) + local llmKeywords = {} + + -- Wrap in pcall to catch any errors + local success, result = pcall(function() + local allKeywords = photo:getRawMetadata("keywords") + + if allKeywords then + for _, keyword in ipairs(allKeywords) do + local parent = keyword:getParent() + if parent and parent:getName() == "llm" then + table.insert(llmKeywords, keyword:getName()) + end + end + end + end) + + if not success then + logger:warn("Error getting LLM keywords: " .. tostring(result)) + return {} -- Return empty array on error + end + + return llmKeywords +end + +--- Remove all llm-parented keywords from a photo. +--- Only removes the keyword-to-photo association — does not delete the keyword +--- definitions from the catalog (other photos may reference them). +---@param catalog LrCatalog Active catalog +---@param photo LrPhoto Target photo +function metadata.removeLlmKeywords(catalog, photo) + local allKeywords = photo:getRawMetadata("keywords") + if not allKeywords then + return + end + + local keywordsToRemove = {} + for _, keyword in ipairs(allKeywords) do + local parent = keyword:getParent() + if parent and parent:getName() == "llm" then + table.insert(keywordsToRemove, keyword) + end + end + + for _, keyword in ipairs(keywordsToRemove) do + photo:removeKeyword(keyword) + end +end + +--- Parse a comma-separated keyword string into a trimmed list. +--- Filters out empty entries. Used when reading keywords from the dialog's +--- edit field, where the user enters "sunset, beach, calm". +---@param csv string Comma-separated keywords (e.g., "sunset, beach, calm") +---@return table Trimmed keyword strings, empty entries filtered out +function metadata.parseKeywordCsv(csv) + if not csv or csv == "" then + return {} + end + + local result = {} + for keyword in string.gmatch(csv, "([^,]+)") do + local trimmed = keyword:match("^%s*(.-)%s*$") + if trimmed ~= "" then + table.insert(result, trimmed) + end + end + return result +end + +return metadata diff --git a/lightroom-llama.lrplugin/OllamaClient.lua b/lightroom-llama.lrplugin/OllamaClient.lua new file mode 100644 index 0000000..021a911 --- /dev/null +++ b/lightroom-llama.lrplugin/OllamaClient.lua @@ -0,0 +1,335 @@ +--- OllamaClient.lua — Communicate with a local Ollama server. +--- Handles base URL construction, model discovery, HTTP requests to the generate API, +--- and orchestrates the full pipeline: export thumbnail -> encode -> build prompt -> POST +--- -> validate response. +--- SDK imports: LrHttp, LrPrefs, LrLogger, LrPathUtils, LrTasks + +local LrHttp = import 'LrHttp' +local LrPathUtils = import 'LrPathUtils' +local LrLogger = import 'LrLogger' +local LrTasks = import "LrTasks" +local LrPrefs = import "LrPrefs" + +local logger = LrLogger('LrLlama') +logger:enable("logfile") -- Logs to ~/Documents/LrClassicLogs | tail -f LrLlama.log + +-- Load shared utilities +local JSON = (assert(loadfile(LrPathUtils.child(_PLUGIN.path, "JSON.lua"))))() +local ThumbnailService = (assert(loadfile(LrPathUtils.child(_PLUGIN.path, "ThumbnailService.lua"))))() +local PromptBuilder = (assert(loadfile(LrPathUtils.child(_PLUGIN.path, "PromptBuilder.lua"))))() +local ResponseValidator = (assert(loadfile(LrPathUtils.child(_PLUGIN.path, "ResponseValidator.lua"))))() + +-------------------------------------------------------------------------------- +--- Constructor — create a client with injectable dependencies. +--- callers need no changes while tests can provide lightweight fakes. +---@param deps table|nil Optional dependency overrides: +--- { http, prefs, tasks, json, thumbnailService, promptBuilder, responseValidator, logger } +---@return table client Fully initialised OllamaClient instance +-------------------------------------------------------------------------------- +local function createClient(deps) + deps = deps or {} + + local http = deps.http or LrHttp + local prefsService = deps.prefs or LrPrefs + local tasks = deps.tasks or LrTasks + local json = deps.json or JSON + local thumbnailService = deps.thumbnailService or ThumbnailService + local promptBuilder = deps.promptBuilder or PromptBuilder + local responseValidator = deps.responseValidator or ResponseValidator + local activeLogger = deps.logger or logger + + local client = {} + + -------------------------------------------------------------------------------- + -- Constants + -------------------------------------------------------------------------------- + + --- Default Ollama model name; change here to switch models. + client.defaultModel = "gemma4:latest" + + --- Fallback server address when prefs are missing, empty, or invalid. + client.defaultServerHost = "localhost:11434" + + -------------------------------------------------------------------------------- + -- Server configuration + -------------------------------------------------------------------------------- + + --- Validate and normalize an Ollama server address. + --- Strips optional scheme (`http://`) and trailing slashes, then verifies the + --- string is `hostname:port`. Accepts localhost, IP addresses, and domain names. + ---@param input string|nil User-supplied server address (may be empty) + ---@return bool ok True when the address is valid + ---@return string result The normalized host:port on success, error message on failure + function client.validateServerHost(input) + if not input or input == "" then + return true, client.defaultServerHost -- treat empty as "use default" + end + -- Strip scheme and trailing slashes for normalization + local host = string.gsub(input, "^https?://", "") + host = string.gsub(host, "/+$", "") + + -- Must match something:digit (hostname or IP followed by colon and port) + local hostnamePart, portPart = string.match(host, "^([^:]+):(%d+)$") + if not hostnamePart or not portPart then + return false, "Server address must be in host:port format (e.g., localhost:11434)" + end + + local port = tonumber(portPart) + if port < 1 or port > 65535 then + return false, "Port number must be between 1 and 65535" + end + + -- hostnamePart should contain at least one valid character (alphanumeric, dot, hyphen) + if not string.match(hostnamePart, "^[%a%d%.%-]+$") then + return false, "Hostname contains invalid characters" + end + + return true, hostnamePart .. ":" .. portPart + end + + --- Build the base URL from saved preferences. + --- Reads prefs.ollamaServerHost, validates, falls back to default if malformed. + ---@param prefs table LrPrefs.prefsForPlugin() instance (injected for testability) + ---@return string baseUrl Full URL like "http://localhost:11434" + function client.getBaseUrl(prefs) + local ok, host = client.validateServerHost(prefs.ollamaServerHost) + + if not ok then + activeLogger:warn("Invalid server host in prefs: " .. tostring(prefs.ollamaServerHost)) + host = client.defaultServerHost + end + + return "http://" .. host + end + + -------------------------------------------------------------------------------- + -- Model discovery + -------------------------------------------------------------------------------- + + --- Convert a plain list of model names into the `{title, value}` format that + --- LrView `popup_menu` requires for its `items` binding. + ---@param modelNames table List of model strings (from fetchModels) + ---@return table
Table of `{title = m, value = m}` entries for popup_menu binding + function client.makeModelItems(modelNames) + local items = {} + for _, m in ipairs(modelNames) do + table.insert(items, { title = m, value = m }) + end + return items + end + + --- Query Ollama's `/api/tags` endpoint for installed models. + --- Falls back to `{defaultModel}` on network errors, malformed JSON, or an empty + --- model list — so callers never receive nil. + ---@param prefs table LrPrefs instance + ---@return table List of model name strings; always non-empty. + function client.fetchModels(prefs) + activeLogger:info("Fetching available models from Ollama") + + local url = client.getBaseUrl(prefs) .. "/api/tags" + local response = http.get(url) + + if response then + local decodeSuccess, data = pcall(function() + return json:decode(response) + end) + + if decodeSuccess and data and type(data.models) == "table" then + local modelList = {} + + for _, availableModel in ipairs(data.models) do + local modelName = availableModel.name or availableModel.model + + if modelName and modelName ~= "" then + table.insert(modelList, modelName) + end + end + + if #modelList > 0 then + activeLogger:info("Found " .. #modelList .. " models") + return modelList + end + + activeLogger:warn("Ollama returned an empty model list") + elseif not decodeSuccess then + activeLogger:warn( + "Failed to decode Ollama model response: " .. tostring(data) + ) + else + activeLogger:warn("Ollama response did not contain a valid model list") + end + else + activeLogger:warn("No response received from Ollama") + end + + activeLogger:warn("Using default model: " .. client.defaultModel) + return { client.defaultModel } + end + + --- Persist the server host and rebuild the model dropdown. + --- MUST be called from within `LrTasks.startAsyncTask()` — LrBinding only + --- propagates property reassignments (modelItems, selectedModel) when done + --- off the UI thread. + ---@param props table LrBinding property table containing serverHost, modelItems, selectedModel + ---@param prefs table LrPrefs.prefsForPlugin() instance + ---@return bool ok True on success + ---@return string message Human-readable status or error description + function client.saveServerAndRefresh(props, prefs) + local ok, validatedHost = client.validateServerHost(props.serverHost) + if not ok then + return false, validatedHost -- caller displays error + end + + prefs.ollamaServerHost = validatedHost + + local availableModels = client.fetchModels(prefs) + if not availableModels or #availableModels == 0 then + return false, "No models found on server" + end + + -- Rebuild model list. This function is intended to be called from inside + -- LrTasks.startAsyncTask() so that LrBinding picks up the property + -- reassignment and propagates it to the dialog's popup_menu. + local oldSelection = props.selectedModel or "" + props.modelItems = client.makeModelItems(availableModels) + + -- Keep old selection if it still exists in the new list + local found = false + for _, m in ipairs(availableModels) do + if m == oldSelection then found = true; break; end + end + props.selectedModel = found and oldSelection or availableModels[1] + + return true, string.format("Loaded %d model(s)", #availableModels) + end + + -------------------------------------------------------------------------------- + -- Generate pipeline + -------------------------------------------------------------------------------- + + --- Export thumbnail with retry logic (up to 3 attempts, 500ms between failures). + ---@param photo LrPhoto Photo object from the catalog + ---@return string|nil Path on success, nil after all retries exhausted + local function exportWithRetries(photo) + for attempt = 1, 3 do + local path = thumbnailService.export(photo) + if path then + return path + end + activeLogger:warn("Thumbnail export attempt " .. attempt .. " failed, retrying...") + tasks.sleep(0.5) -- Wait 500ms before retry + end + return nil + end + + --- Best-effort helper: attempt to clean up the thumbnail and log any failure. + --- Returns true if cleanup succeeded, false if logged. + --- LrFileUtils.delete is a local-disk operation — it does not yield, so + --- pcall here is safe (avoids the "yield through protected boundary" issue + --- that plagues network I/O). + ---@param path string Path to delete + ---@return bool succeeded True if cleanup completed without error + local function tryCleanup(path) + local ok, err = pcall(function() thumbnailService.cleanup(path) end) + if not ok and activeLogger then + activeLogger:error( + "Thumbnail cleanup also failed: " .. tostring(err) + ) + end + return ok + end + + --- High-level generate: export thumbnail, encode, build prompt, POST, validate. + --- Orchestrates ThumbnailService + PromptBuilder + ResponseValidator internally. + --- This is the decomposition of the former Common.sendDataToApi mega-function. + --- Retries thumbnail export up to 3 times. Cleans up the temp file at every + --- exit point regardless of success or failure. + --- + --- Explicit cleanup (rather than wrapping SDK calls in pcall) avoids the + --- "yield through protected boundary" problem: Lightroom SDK functions like + --- http.post and LrStringUtils.encodeBase64 can yield internally, and pcall + --- cannot survive those yields — it returns early with a false error, silently + --- swallowing the real result. Explicit cleanup at each step is equally safe + --- for return-based failures and avoids the yield problem entirely. + --- Pure-Lua steps (prompt builder, JSON encoding) are still wrapped in pcall + --- because they cannot yield. + ---@param photo LrPhoto Photo object from the catalog + ---@param userInstruction string User-facing prompt text + ---@param currentData table|nil Existing title/caption to prepend when useCurrentData is true + ---@param useCurrentData boolean Whether to include current metadata in the prompt + ---@param useSystemPrompt boolean Whether to send a system prompt alongside the user prompt + ---@param selectedModel string|nil Model name; falls back to the default if nil + ---@param systemPromptOverride string|nil Override for the built-in system prompt + ---@return table|nil response Parsed JSON on success ({title, caption, keywords}) + ---@return string|nil err Error message, or nil on success + function client.generate(photo, userInstruction, currentData, useCurrentData, + useSystemPrompt, selectedModel, systemPromptOverride) + activeLogger:info("Sending data to API") + + -- Export thumbnail with retry; early return avoids cleanup when nothing created. + local thumbnailPath = exportWithRetries(photo) + if not thumbnailPath then + return nil, "Failed to export thumbnail after 3 attempts" + end + + -- Step 1: Encode image as Base64 (SDK — may yield, don't wrap in pcall). + local encodedImage = thumbnailService.encodeBase64(thumbnailPath) + if not encodedImage then + tryCleanup(thumbnailPath) + return nil, "Failed to encode image" + end + + -- Step 2: Build prompt, assemble body, JSON-encode (pure Lua — pcall-safe). + local jsonPayload + local ok, err = pcall(function() + local builtPrompt = promptBuilder.buildUserPrompt( + userInstruction, currentData, useCurrentData) + local requestBody = promptBuilder.assembleRequestBody( + builtPrompt, selectedModel or client.defaultModel, + useSystemPrompt, systemPromptOverride) + requestBody.images = {encodedImage} + jsonPayload = json:encode(requestBody) + end) + if not ok then + tryCleanup(thumbnailPath) + return nil, tostring(err) + end + + -- Step 3: HTTP POST (SDK — may yield, don't wrap in pcall). + local url = client.getBaseUrl(prefsService.prefsForPlugin()) .. "/api/generate" + local response = http.post(url, jsonPayload, {{ + field = "Content-Type", + value = "application/json" + }}) + if not response then + tryCleanup(thumbnailPath) + return nil, "Failed to send data to the API" + end + + -- Step 4: Cleanup thumbnail before validation — even on success. + -- LrFileUtils.delete is local-disk I/O (no yield), so pcall is safe. + -- If cleanup fails here, no point validating — return the cleanup error. + local cleanupOk, cleanupErr = pcall(function() + thumbnailService.cleanup(thumbnailPath) + end) + if not cleanupOk then + return nil, "Failed to clean up thumbnail: " .. tostring(cleanupErr) + end + + -- Step 5: Validate after cleanup — thumbnail removed regardless of outcome. + return responseValidator.validateAndParse(response) + end + + return client +end + +-- Create the default production client with real Lightroom SDK dependencies. +local defaultClient = createClient() + +--- Create a new OllamaClient with optional dependency overrides. +--- Tests inject fake deps; production callers may omit deps entirely. +---@param deps table|nil Optional dependency overrides +---@return table client New client instance +defaultClient.new = createClient + +return defaultClient diff --git a/lightroom-llama.lrplugin/PromptBuilder.lua b/lightroom-llama.lrplugin/PromptBuilder.lua new file mode 100644 index 0000000..ce66c4a --- /dev/null +++ b/lightroom-llama.lrplugin/PromptBuilder.lua @@ -0,0 +1,180 @@ +--- PromptBuilder.lua — Build prompts and request payloads for Ollama API calls. +--- Pure Lua module with no Lightroom SDK dependencies (no LrView, LrDialogs, etc.). +--- Owns all system prompt definitions and user-prompt assembly logic. +--- Loaded by OllamaClient.lua for the generate pipeline and by tests directly. + +local prompt = {} + +-------------------------------------------------------------------------------- +-- System prompts +-------------------------------------------------------------------------------- + +--- Default system prompt for batch/general mode (the detailed version). +--- Stricter guidelines: 5-12 word titles, 10-30 keywords, quality checklist. +prompt.defaultSystemPrompt = [[# Image Metadata Generation Prompt + +You are an expert content curator specializing in creating compelling, accurate metadata for visual content. Your task is to analyze the provided image/video and generate a JSON object with three components: title, caption, and keywords. + +## Output Format +Return your response as a valid JSON object with this exact structure: +```json +{ + "title": "string", + "caption": "string", + "keywords": ["string", "string", "string"] +} +``` + +## Guidelines + +### Title Requirements +- **Length**: 5-12 words maximum +- **Style**: Write as a descriptive headline, not a sentence +- **Content**: Capture the main subject, action, and context +- **Focus**: Answer "what is happening" in the most compelling way +- **Avoid**: Generic terms, keyword stuffing, colons, redundant phrases +- **Include**: Specific details like location, time of day, or unique elements when relevant + +**Good examples:** +- "Mountain climber reaching summit during golden hour" +- "Children playing soccer in urban park" +- "Vintage red bicycle against brick wall" + +### Caption Requirements +- **Length**: 15-40 words +- **Style**: Complete sentences that expand on the title +- **Content**: Provide context, mood, or story behind the image +- **Focus**: Add emotional resonance or background information +- **Avoid**: Repeating the exact title wording +- **Include**: Atmosphere, setting details, or cultural context when relevant + +**Good example:** +*Title: "Street musician performing violin solo in subway station"* +*Caption: "A talented violinist captivates commuters with classical music during evening rush hour, creating a moment of beauty in the bustling underground transit hub."* + +### Keywords Requirements +- **Quantity**: 10-30 keywords (aim for 15-20 for optimal results) +- **Hierarchy**: Order from most specific to more general +- **Categories**: Include subjects, actions, emotions, locations, styles, colors, concepts +- **Format**: Single words or short phrases (2-3 words max) +- **Avoid**: Repeating title/caption words exactly, overly generic terms, technical camera specs + +**Keyword categories to consider:** +- Primary subjects (people, objects, animals) +- Actions and verbs +- Emotions and moods +- Locations and settings +- Colors and lighting +- Art styles or techniques +- Concepts and themes +- Seasonal or temporal elements + +## Quality Checklist +Before finalizing, ensure: +- [ ] Title is unique and descriptive without being generic +- [ ] Caption adds meaningful context beyond the title +- [ ] Keywords cover multiple relevant categories +- [ ] No unnecessary repetition across all three elements +- [ ] JSON format is valid and properly structured +- [ ] Content accurately reflects what's actually in the image + +## Example Output +```json +{ + "title": "Barista creating latte art in cozy downtown cafe", + "caption": "Skilled coffee artist carefully pours steamed milk to create an intricate leaf pattern, showcasing the craftsmanship behind specialty coffee culture in a warm, inviting neighborhood coffee shop.", + "keywords": ["barista", "latte art", "coffee shop", "cafe culture", "milk foam", "artisan", "beverage preparation", "downtown", "craftsmanship", "morning routine", "specialty coffee", "hospitality", "small business", "urban lifestyle", "food service"] +} +``` +]] + +--- Legacy system prompt for single-photo dialog (more permissive). +--- Allows broader keyword ranges (7-50) and retains existing metadata verbatim. +prompt.singlePhotoSystemPrompt = [[You are an AI tasked with creating a JSON object containing a `title`, a `caption`, and a list of `keywords` based on a given piece of content (such as an image or video). ]] .. +[[The content currently has the following metadata which you need to implement and improve upon. It is important to keep the title and caption as close to this as possible. + +Please follow these detailed guidelines for creating excellent metadata: + +1. **Title (Description):** + - Provide a unique, descriptive title for the content. + - The title should answer the Who, What, When, Where, and Why of the content. + - It should be written as a sentence or phrase, similar to a news headline, capturing the key details, mood, and emotions of the scene. + - Do not list keywords in the title. Avoid repetition of words and phrases. + - Include helpful details such as the angle, focus, and perspective if relevant. + - Do not include :. + - If given, use the current title as a starting point. + +2. **Caption:** + - Provide a more detailed description or context for the content. This can be a fuller explanation of the title, including any relevant background or emotional tone that helps convey the essence of the scene. + - If given, use the current caption as a starting point. + +3. **Keywords:** + - Provide a list of 7 to 50 keywords. + - Keywords should be specific and directly related to the content. + - Include broader topics, feelings, concepts, or associations represented by the content. + - Avoid using unrelated terms or repeating words or compound words. + - Do not include links, camera information, or trademarks unless required for editorial content. + +### JSON Format: +```json +{ + "title": "string", + "caption": "string", + "keywords": ["string"] +} +``` + +### Example: +```json +{ + "title": "A serene sunset over a peaceful beach with golden skies", + "caption": "A calm evening beach scene with a golden sunset reflecting on the ocean waves, creating a peaceful and tranquil mood. The horizon is clear with soft, pastel colors blending into the blue sky.", + "keywords": ["sunset", "beach", "calm", "ocean", "serene", "golden skies", "peaceful", "tranquil", "pastel colors", "horizon", "evening"] +} +``` + +Use this structure and guidelines to generate titles, captions, and keywords that are descriptive, unique, and accurate.]] + +-------------------------------------------------------------------------------- +-- Prompt assembly +-------------------------------------------------------------------------------- + +--- Build the user prompt string, optionally prepending current metadata. +--- When `useCurrentData` is true, formats as `"Title: Caption: "` +--- to give the model context about existing metadata. Escapes double quotes in +--- title and caption to avoid breaking JSON when this string is embedded in a payload. +---@param userInstruction string The user's custom prompt text (e.g., "Caption this photo") +---@param currentData {title: string, caption: string}|nil Existing metadata +---@param useCurrentData boolean Whether to prepend existing title/caption +---@return string assembled Prompt text ready for the API +function prompt.buildUserPrompt(userInstruction, currentData, useCurrentData) + if not useCurrentData or not currentData then + return userInstruction + end + + local title = (currentData.title or ""):gsub('"', '\\"') + local caption = (currentData.caption or ""):gsub('"', '\\"') + return "Title: " .. title .. " Caption: " .. caption .. " " .. userInstruction +end + +--- Assemble the full Ollama /api/generate request body fields. +--- Returns a table suitable for JSON:encode before HTTP POST. +--- Does NOT include the `images` field — callers add that after receiving this table. +---@param userPrompt string The assembled user prompt (from buildUserPrompt) +---@param model string Model name to use +---@param useSystemPrompt boolean Whether to include system prompt in request +---@param systemPromptOverride string|nil Custom system prompt, or nil to use default +---@return table requestBody Ready-to-encode POST fields (model, prompt, format, system?, stream) +function prompt.assembleRequestBody(userPrompt, model, useSystemPrompt, systemPromptOverride) + local activeSystemPrompt = systemPromptOverride or prompt.defaultSystemPrompt + + return { + model = model, + prompt = userPrompt, + format = "json", + system = useSystemPrompt and activeSystemPrompt or nil, + stream = false, + } +end + +return prompt diff --git a/lightroom-llama.lrplugin/ResponseValidator.lua b/lightroom-llama.lrplugin/ResponseValidator.lua new file mode 100644 index 0000000..229073e --- /dev/null +++ b/lightroom-llama.lrplugin/ResponseValidator.lua @@ -0,0 +1,105 @@ +--- ResponseValidator.lua — Parse and validate Ollama API responses. +--- Pure Lua module with no Lightroom SDK dependencies. Strips markdown fences, +--- decodes the outer API envelope, and validates the inner JSON schema has +--- title (string), caption (string), and keywords (array of non-empty strings). +--- Loaded by OllamaClient.lua for the generate pipeline and by tests directly. + +local LrPathUtils = import 'LrPathUtils' + +local JSON = (assert(loadfile(LrPathUtils.child(_PLUGIN.path, "JSON.lua"))))() + +local validate = {} + +--- Strip markdown code fences from a string. +--- Handles ```json, ```, and bare backtick variants. Two passes ensure both +--- opening and closing fences are removed, including edge cases where the model +--- emits extra whitespace around the blocks. +---@param raw string Raw model response text +---@return string cleaned Text with fences removed +function validate.stripMarkdownFences(raw) + local result = string.gsub(raw, "^%s*```%w+\n?", "") + result = string.gsub(result, "\n?```%s*$", "") + result = string.gsub(result, "^%s*```%s*\n?", "") + result = string.gsub(result, "\n?```%s*$", "") + return result +end + +--- Decode and validate an Ollama API envelope response. +--- Verifies the outer `{response: "..."}` structure exists. The envelope is the +--- direct JSON output of the `/api/generate` endpoint, which wraps the model's +--- text in a `response` field. +---@param apiResponseString string The raw HTTP response body from /api/generate +---@return table|nil parsed Envelope data (has .response field), or nil +---@return string|nil err Error message or nil +function validate.parseApiEnvelope(apiResponseString) + local decodeOk, data = pcall(function() + return JSON:decode(apiResponseString) + end) + + if not decodeOk + or type(data) ~= "table" + or type(data.response) ~= "string" then + return nil, "Invalid API response structure" + end + + return data, nil +end + +--- Validate that a decoded JSON object has the required metadata schema. +--- Checks for title (string), caption (string), keywords (array of non-empty strings). +---@param data table Decoded JSON object from the model +---@return bool ok True if schema is valid +---@return string|nil err Describes which field failed, or nil +function validate.checkMetadataSchema(data) + if type(data.title) ~= "string" + or type(data.caption) ~= "string" + or type(data.keywords) ~= "table" then + return false, "API response missing required fields (title, caption, keywords)" + end + + for _, kw in ipairs(data.keywords) do + if type(kw) ~= "string" or kw:match("^%s*$") then + return false, "API response contains an invalid keyword" + end + end + + return true, nil +end + +--- Full pipeline: parse envelope, strip fences, decode inner JSON, validate schema. +--- Accepts the raw HTTP response string and returns parsed metadata or an error. +---@param rawHttpResponse string Raw POST /api/generate response body +---@return table|nil metadata {title, caption, keywords} on success +---@return string|nil err Error message, or nil +function validate.validateAndParse(rawHttpResponse) + -- Parse outer Ollama API envelope + local envelope, err = validate.parseApiEnvelope(rawHttpResponse) + if not envelope then + return nil, err + end + + local rawResponse = envelope.response + + -- Many Ollama models emit markdown-wrapped JSON ("```json\n{...}\n```"). + -- Strip the fences so JSON:decode receives bare JSON. + rawResponse = validate.stripMarkdownFences(rawResponse) + + -- Decode the model-generated JSON + local decodeOk, responseJson = pcall(function() + return JSON:decode(rawResponse) + end) + + if not decodeOk or type(responseJson) ~= "table" then + return nil, "Invalid JSON in API response" + end + + -- Validate required fields exist with correct types + local schemaOk, schemaErr = validate.checkMetadataSchema(responseJson) + if not schemaOk then + return nil, schemaErr + end + + return responseJson, nil +end + +return validate diff --git a/lightroom-llama.lrplugin/SinglePhotoController.lua b/lightroom-llama.lrplugin/SinglePhotoController.lua new file mode 100644 index 0000000..99269f0 --- /dev/null +++ b/lightroom-llama.lrplugin/SinglePhotoController.lua @@ -0,0 +1,223 @@ +--- SinglePhotoController.lua — Testable workflow logic for single-photo metadata +--- generation. Owns state transitions for generate, save-server, and save-metadata +--- actions without any Lightroom SDK dependencies (no LrView, LrBinding, LrColor, +--- LrDialogs, or LrFunctionContext). +--- +--- Inject dependencies via the constructor; each operation returns structured +--- results ({ok, message}) and mutates the state table in place so that callers +--- using the same table as an LrBinding property table see live updates. + +local controller = {} + +-------------------------------------------------------------------------------- +--- Constructor — validate dependencies and return a controller instance. +---@param deps table Dependency map: +--- { client, common, promptBuilder, prefs, catalog, photo, statusMessages, writeMetadata } +--- - client: OllamaClient instance (generate, fetchModels, validateServerHost, +--- saveServerAndRefresh, makeModelItems, defaultModel, defaultServerHost) +--- - common: Utility functions { parseKeywordCsv, getLlmKeywordsFromPhoto } +--- - promptBuilder: PromptBuilder module with singlePhotoSystemPrompt +--- - prefs: LrPrefs.prefsForPlugin() table for persistence +--- - catalog: LrCatalog active catalog +--- - photo: LrPhoto selected photo +--- - statusMessages: { ready, generating, refreshing } status strings +--- - writeMetadata: function(catalog, photo) callback for catalog writes +--- (wraps withWriteAccessDo in production; faked in tests) +---@return table controller Instance with initState, generate, saveServer, saveMetadata +function controller.new(deps) + assert(deps.client, "deps.client is required") + assert(deps.common, "deps.common is required") + assert(deps.prefs, "deps.prefs is required") + assert(deps.catalog, "deps.catalog is required") + assert(deps.photo, "deps.photo is required") + + local promptBuilder = deps.promptBuilder + local statusMessages = deps.statusMessages or {} + + return { + initState = function() return controller.initState(deps, promptBuilder, statusMessages) end, + generate = function(state) return controller.generate(deps, state) end, + saveServer = function(state) return controller.saveServer(deps, state) end, + saveMetadata = function(state) return controller.saveMetadata(deps, state) end, + } +end + +-------------------------------------------------------------------------------- +-- Initial state creation +-------------------------------------------------------------------------------- + +--- Build the initial state table by reading photo metadata, loading preferences, +--- and fetching available models. +local function initState(deps, promptBuilder, statusMessages) + local client = deps.client + local common = deps.common + local prefs = deps.prefs + local photo = deps.photo + + local state = {} + + -- Read existing photo metadata + state.title = photo:getFormattedMetadata("title") or "" + state.caption = photo:getFormattedMetadata("caption") or "" + + -- Read existing LLM keywords and join with commas + local existingKw = common.getLlmKeywordsFromPhoto(photo) + state.keywords = table.concat(existingKw, ", ") + + -- Enable "use current data" when there's metadata to work from + state.useCurrentData = (state.title ~= "" or state.caption ~= "") + + -- Server host: saved preference or default + state.serverHost = prefs.ollamaServerHost or client.defaultServerHost + state.useSystemPrompt = true + + -- Fetch models and populate dropdown items. The adapter may or may not accept + -- a prefs argument; call it with no args (prefs are internal to the adapter). + local models = client.fetchModels() + if not models or #models == 0 then + models = { client.defaultModel } + end + state.modelItems = client.makeModelItems(models) + state.selectedModel = models[1] + + -- Prompt and status defaults + state.prompt = "Caption this photo" + state.response = nil + state.status = statusMessages.ready or "Ready" + state.statusKind = "success" + + return state +end + +controller.initState = initState + +-------------------------------------------------------------------------------- +-- Generate action +-------------------------------------------------------------------------------- + +--- Call the API to generate metadata for the current photo. +--- Sets status to working before the call, success/error after. +--- On failure, preserves existing editable fields (title, caption, keywords). +---@param state table Mutable state table (also used as LrBinding property table) +---@return table result { ok: bool, message?: string } +local function generate(deps, state) + local client = deps.client + local statusMessages = deps.statusMessages or {} + + -- Transition to working status + state.status = statusMessages.generating or "The llama is thinking..." + state.statusKind = "working" + + -- Snapshot current editable fields so we can restore on failure + local savedTitle = state.title + local savedCaption = state.caption + local savedKeywords = state.keywords + + local apiResponse, apiError = deps.client.generate( + deps.photo, + state.prompt, + { title = state.title, caption = state.caption }, + state.useCurrentData, + state.useSystemPrompt, + state.selectedModel, + deps.promptBuilder and deps.promptBuilder.singlePhotoSystemPrompt or nil + ) + + if apiError then + -- Restore editable fields on failure + state.title = savedTitle + state.caption = savedCaption + state.keywords = savedKeywords + state.status = "Error: " .. apiError + state.statusKind = "error" + return { ok = false, message = "Error: " .. apiError } + end + + -- Update from API response on success + state.response = apiResponse + state.title = apiResponse.title + state.caption = apiResponse.caption + if apiResponse.keywords and type(apiResponse.keywords) == "table" then + state.keywords = table.concat(apiResponse.keywords, ", ") + else + state.keywords = "" + end + state.status = statusMessages.ready or "Ready" + state.statusKind = "success" + return { ok = true } +end + +controller.generate = generate + +-------------------------------------------------------------------------------- +-- Save Server action +-------------------------------------------------------------------------------- + +--- Validate and persist the server host, then refresh the model list. +--- Callers are responsible for running this from an async task (LrBinding +--- propagation requires off-UI-thread property mutations). +---@param state table Mutable state table +---@return table result { ok: bool, message?: string } +local function saveServer(deps, state) + local statusMessages = deps.statusMessages or {} + + state.status = statusMessages.refreshing or "Refreshing models..." + state.statusKind = "working" + + local ok, msg = deps.client.saveServerAndRefresh(state, deps.prefs) + + if not ok then + state.status = "Error: " .. (msg or "Failed to save server") + state.statusKind = "error" + return { ok = false, message = state.status } + end + + state.status = msg + state.statusKind = "success" + return { ok = true, message = msg } +end + +controller.saveServer = saveServer + +-------------------------------------------------------------------------------- +-- Save Metadata action +-------------------------------------------------------------------------------- + +--- Validate server host, persist preferences, and write metadata through the +--- injected write function. Returns a structured result so callers (and tests) +--- can distinguish validation failure from successful saves. +---@param state table Mutable state table +---@return table result { ok: bool, message?: string, errorKind?: string } +local function saveMetadata(deps, state) + -- Validate and normalize server host + local ok, validatedHost = deps.client.validateServerHost(state.serverHost) + if not ok then + return { + ok = false, + errorKind = "invalid_server", + message = validatedHost, + } + end + + -- Save normalized host to preferences (only when valid) + deps.prefs.ollamaServerHost = validatedHost + + -- Parse keywords using the tested CSV parser + local keywordList = {} + if state.keywords and state.keywords ~= "" then + keywordList = deps.common.parseKeywordCsv(state.keywords) + end + + -- Execute catalog writes through injected adapter + deps.writeMetadata(deps.catalog, deps.photo, { + title = state.title, + caption = state.caption, + keywords = keywordList, + }) + + return { ok = true } +end + +controller.saveMetadata = saveMetadata + +return controller diff --git a/lightroom-llama.lrplugin/ThumbnailService.lua b/lightroom-llama.lrplugin/ThumbnailService.lua new file mode 100644 index 0000000..71b6bad --- /dev/null +++ b/lightroom-llama.lrplugin/ThumbnailService.lua @@ -0,0 +1,122 @@ +--- ThumbnailService.lua — Export, encode, and clean up photo thumbnails. +--- Handles the full thumbnail lifecycle: request JPEG from Lightroom, write to temp, +--- Base64-encode for API payload, and delete after use. +--- SDK imports: LrFileUtils, LrPathUtils, LrStringUtils, LrLogger + +local LrFileUtils = import 'LrFileUtils' +local LrPathUtils = import 'LrPathUtils' +local LrStringUtils = import 'LrStringUtils' +local LrLogger = import 'LrLogger' + +local logger = LrLogger('LrLlama') + +local thumbnail = {} + +--- Export a 512×512 JPEG thumbnail to a unique temp file. +--- **Side effect:** writes a file to the system temp directory. +--- Caller is responsible for deleting the returned file after use via `cleanup()`. +---@param photo LrPhoto Photo object from the catalog +---@return string|nil Absolute path on success, nil on failure +function thumbnail.export(photo) + local tempPath = LrFileUtils.chooseUniqueFileName(LrPathUtils.getStandardFilePath('temp') .. "/thumbnail.jpg") + logger:info("Attempting to export thumbnail to: " .. tempPath) + + -- Validate that the temp directory exists and is accessible before proceeding + -- This prevents silent failures when the temp directory is missing or restricted + local tempDir = LrPathUtils.getStandardFilePath('temp') + if not LrFileUtils.exists(tempDir) then + logger:error("Temp directory does not exist: " .. tempDir) + return nil + end + + -- Track whether the thumbnail callback successfully wrote data + -- This flag is set inside the callback to communicate success back to the caller + local thumbnailSaved = false + + -- Request a 512x512 JPEG thumbnail asynchronously + -- photo:requestJpegThumbnail() returns (success, result) and executes the callback + -- with the JPEG binary data if available + local success, result = photo:requestJpegThumbnail(512, 512, function(jpegData) + if jpegData then + -- Open temp file in binary write mode ("wb") to preserve JPEG data integrity + local tempFile = io.open(tempPath, "wb") + if tempFile then + tempFile:write(jpegData) + tempFile:close() + thumbnailSaved = true + logger:info("Thumbnail saved to " .. tempPath) + return true + else + logger:error("Could not open temp file for writing: " .. tempPath) + return false + end + else + logger:error("No JPEG data received from photo") + return false + end + end) + + -- Verify both the API call succeeded AND the callback wrote the file + if success and thumbnailSaved then + -- Final verification: ensure the file actually exists on disk + -- This catches edge cases where the write appeared successful but the file is missing + if LrFileUtils.exists(tempPath) then + logger:info("Thumbnail export successful: " .. tempPath) + return tempPath + else + logger:error("Thumbnail file was not created: " .. tempPath) + return nil + end + else + logger:warn("Failed to export thumbnail. Success: " .. tostring(success) .. ", Result: " .. tostring(result)) + return nil + end +end + +--- Read a JPEG file and return its Base64-encoded representation. +---@param imagePath string Absolute path to the JPEG file +---@return string|nil Base64 string on success, nil if file unreadable or empty +function thumbnail.encodeBase64(imagePath) + logger:info("Attempting to encode image: " .. imagePath) + + -- Check if file exists + if not LrFileUtils.exists(imagePath) then + logger:error("Image file does not exist: " .. imagePath) + return nil + end + + local file = io.open(imagePath, "rb") + if not file then + logger:error("Could not open file for reading: " .. imagePath) + return nil + end + + local binaryData = file:read("*all") + file:close() + + if not binaryData or #binaryData == 0 then + logger:error("No data read from file: " .. imagePath) + return nil + end + + local base64Data = LrStringUtils.encodeBase64(binaryData) + if not base64Data then + logger:error("Failed to encode image to base64: " .. imagePath) + return nil + end + + logger:info("Successfully encoded image to base64. Size: " .. #binaryData .. " bytes") + return base64Data +end + +--- Clean up a temp thumbnail file (idempotent — no error if already missing). +--- Call after the API request completes, regardless of success or failure. +---@param imagePath string Path to delete +function thumbnail.cleanup(imagePath) + if imagePath and LrFileUtils.exists(imagePath) then + LrFileUtils.delete(imagePath) + logger:info("Cleaned up temp thumbnail: " .. imagePath) + end +end + +return thumbnail diff --git a/tests/busted.json b/tests/busted.json new file mode 100644 index 0000000..8150aee --- /dev/null +++ b/tests/busted.json @@ -0,0 +1,4 @@ +{ + "helper": [ "helpers/mock_sdk.lua" ], + "lua_sources": [ "." ] +} diff --git a/tests/helpers/mock_sdk.lua b/tests/helpers/mock_sdk.lua new file mode 100644 index 0000000..6872dfe --- /dev/null +++ b/tests/helpers/mock_sdk.lua @@ -0,0 +1,56 @@ +--- mock_sdk.lua — Minimal Lightroom SDK stubs for unit testing modules +--- that import LrPathUtils, LrLogger, LrPrefs, etc. at load time. +--- +--- Runs automatically before each spec via busted.json "helpers" config. + +-- Resolve the .lrplugin root relative to this helper file's location so tests +-- work on any developer's machine regardless of where the repo is cloned. +-- The structure is: /tests/helpers/ (this file) +-- /lightroom-llama.lrplugin/ (target) +local helperDir = string.match(debug.getinfo(1, "S").source, "^@(.*/)") or "" +PLUGIN_PATH = helperDir .. "../..//lightroom-llama.lrplugin/" + +-- _PLUGIN is a global Lightroom injects. Point it at the real plugin dir so +-- loadfile() can find JSON.lua and sibling modules. +_G._PLUGIN = { path = PLUGIN_PATH } + +-- import() is the Lightroom SDK class loader. Return a stub for each class +--- that a module might request. +import = function(className) + if className == "LrPathUtils" then + return { + child = function(parent, name) + return parent .. "/" .. name + end, + } + end + + if className == "LrLogger" then + -- LrLogger is called as a constructor: LrLogger('name') → logger instance + return setmetatable({}, { + __call = function(_, _) + return { + info = function() end, + warn = function() end, + error = function() end, + enable = function() end, + } + end, + }) + end + + if className == "LrPrefs" then + return { prefsForPlugin = function() return {} end } + end + + if className == "LrHttp" then + return {} + end + + if className == "LrTasks" then + return { sleep = function(_) end } + end + + -- Unknown classes get a blank table so loadfile() keeps working. + return {} +end diff --git a/tests/run_tests.sh b/tests/run_tests.sh new file mode 100755 index 0000000..4e47a77 --- /dev/null +++ b/tests/run_tests.sh @@ -0,0 +1,13 @@ +#!/usr/bin/env bash +# Run the Busted test suite for Lightroom Llama plugin modules. +# Usage: ./tests/run_tests.sh [spec file or directory] +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +ROOT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" + +cd "$SCRIPT_DIR" + +busted \ + -e "dofile('helpers/mock_sdk.lua')" \ + "${1:-spec/}" diff --git a/tests/spec/common_delegation_spec.lua b/tests/spec/common_delegation_spec.lua new file mode 100644 index 0000000..6e3261d --- /dev/null +++ b/tests/spec/common_delegation_spec.lua @@ -0,0 +1,519 @@ +--- common_delegation_spec.lua — Behavioral delegation regression tests for +--- Common.lua compatibility layer. +--- +--- Every exported function is tested with fake focused modules to prove that +--- the compatibility layer forwards arguments correctly, propagates return +--- values unchanged, and doesn't silently swallow errors. Because loadfile() +--- creates new function objects each call, these tests use behavioral +--- equivalence (spy recording) rather than pointer identity. + +local path = PLUGIN_PATH + +-------------------------------------------------------------------------------- +--- Helper: pack arguments so nil intermediates don't get lost by # operator. +local function pack(...) + return { n = select("#", ...), ... } +end + +-------------------------------------------------------------------------------- +-- Suite 1: Production load test — verify default Common loads and has all keys +-------------------------------------------------------------------------------- +describe("Common.lua production load", function() + local Common + + before_each(function() + _PLUGIN = { path = path } + Common = assert(loadfile(path .. "Common.lua"))() + end) + + it("exports correct default constant values", function() + assert.are_same("gemma4:latest", Common.model) + assert.are_same("localhost:11434", Common.defaultServerHost) + end) + + it("has all expected functions", function() + local expectedFunctions = { + "validateServerHost", + "makeModelItems", + "saveServerAndRefresh", + "fetchAvailableModels", + "exportThumbnail", + "base64EncodeImage", + "sendDataToApi", + "addKeywordsWithParent", + "getLlmKeywordsFromPhoto", + "removeLlmKeywords", + "parseKeywordCsv", + } + for _, name in ipairs(expectedFunctions) do + assert.is_function( + Common[name], + string.format("Common must export '%s' function", name) + ) + end + end) + + it("has constructor for test dependency injection", function() + assert.is_function(Common.new, "Common must export 'new' constructor") + end) +end) + +-------------------------------------------------------------------------------- +-- Suite 2: Behavioral delegation with fake modules +-------------------------------------------------------------------------------- +describe("Common.lua delegation behavior", function() + + -- Build a fresh set of fakes + common instance before each test. + local fakeOllamaClient + local fakeThumbnailService + local fakeMetadataService + local fakePrefsService + local expectedPrefs + local common + + before_each(function() + expectedPrefs = { + ollamaServerHost = "remote-server:11434", + _prefsMarker = {}, -- unique identity marker + } + + fakeOllamaClient = { + defaultModel = "test-model:42", + defaultServerHost = "test-host:1234", + validateServerHost = function() return true, "ok" end, + makeModelItems = function() return {} end, + saveServerAndRefresh = function() return true, "refreshed" end, + fetchModels = function() return {} end, + generate = function() return {}, nil end, + } + + fakeThumbnailService = { + export = function() return "/fake/thumbnail.jpg" end, + encodeBase64 = function() return "fake-base64-data" end, + } + + fakeMetadataService = { + addKeywordsWithParent = function() end, + getLlmKeywordsFromPhoto = function() return {} end, + removeLlmKeywords = function() end, + parseKeywordCsv = function() return {} end, + } + + fakePrefsService = { + prefsForPlugin = function() return expectedPrefs end, + } + + -- Load Common.lua fresh to get the constructor. We only need the `new` + -- method; the production instance loads fine under mock_sdk but we want + -- isolated fakes for behavioral tests. + _PLUGIN = { path = path } + local CommonModule = assert(loadfile(path .. "Common.lua"))() + common = CommonModule.new({ + ollamaClient = fakeOllamaClient, + thumbnailService = fakeThumbnailService, + metadataService = fakeMetadataService, + prefsService = fakePrefsService, + }) + end) + + ------------------------------------------------------------------------ + -- 1. Constants + ------------------------------------------------------------------------ + describe("constants", function() + it("exposes model from ollamaClient.defaultModel", function() + assert.are_same("test-model:42", common.model) + end) + + it("exposes defaultServerHost from ollamaClient.defaultServerHost", function() + assert.are_same("test-host:1234", common.defaultServerHost) + end) + end) + + ------------------------------------------------------------------------ + -- 2. Direct OllamaClient aliases — validateServerHost + ------------------------------------------------------------------------ + describe("validateServerHost delegation", function() + it("forwards all arguments and propagates all return values", function() + local received + fakeOllamaClient.validateServerHost = function(...) + received = pack(...) + return "ret-1", "ret-2", "ret-3" + end + + local a, b, c = common.validateServerHost("host:1234", "extra") + assert.are_same(2, received.n) + assert.are_same("host:1234", received[1]) + assert.are_same("extra", received[2]) + assert.are_same("ret-1", a) + assert.are_same("ret-2", b) + assert.are_same("ret-3", c) + end) + end) + + ------------------------------------------------------------------------ + -- 2b. Direct OllamaClient aliases — makeModelItems + ------------------------------------------------------------------------ + describe("makeModelItems delegation", function() + it("forwards arguments and propagates return value", function() + local received + local expectedItems = { + { title = "m1", value = "m1" }, + { title = "m2", value = "m2" }, + } + fakeOllamaClient.makeModelItems = function(models) + received = models + return expectedItems + end + + local models = { "m1", "m2" } + local result = common.makeModelItems(models) + assert.is_true(received == models, "same table passed through") + assert.are_same(expectedItems, result) + end) + end) + + ------------------------------------------------------------------------ + -- 2c. Direct OllamaClient aliases — saveServerAndRefresh + ------------------------------------------------------------------------ + describe("saveServerAndRefresh delegation", function() + it("forwards all arguments and propagates all return values", function() + local received + fakeOllamaClient.saveServerAndRefresh = function(...) + received = pack(...) + return true, "models-reloaded" + end + + local props = { serverHost = "localhost:11434", _marker = "props" } + local prefs = { ollamaServerHost = "localhost:11434", _marker = "prefs" } + + local a, b = common.saveServerAndRefresh(props, prefs) + assert.are_same(2, received.n) + assert.is_true(received[1] == props, "same props object") + assert.is_true(received[2] == prefs, "same prefs object") + assert.is_true(a) + assert.are_same("models-reloaded", b) + end) + end) + + ------------------------------------------------------------------------ + -- 3. fetchAvailableModels wrapper + ------------------------------------------------------------------------ + describe("fetchAvailableModels delegation", function() + it("calls prefsForPlugin and passes the exact result to fetchModels", function() + local receivedPrefs + fakeOllamaClient.fetchModels = function(prefs) + receivedPrefs = prefs + return { "model-a", "model-b" } + end + + local models = common.fetchAvailableModels() + -- Identity check: same object, not just equal contents + assert.is_true( + receivedPrefs == expectedPrefs, + "fetchAvailableModels must pass the exact prefs object from prefsForPlugin()" + ) + assert.are_same({ "model-a", "model-b" }, models) + end) + + it("passes no extra arguments to fetchModels", function() + local argCount = 0 + fakeOllamaClient.fetchModels = function(...) + argCount = select("#", ...) + end + + common.fetchAvailableModels() + assert.are_same(1, argCount, "fetchModels should receive exactly 1 argument (prefs)") + end) + + it("propagates all return values from fetchModels", function() + fakeOllamaClient.fetchModels = function() + return { "only-one" }, "second-return" + end + + local a, b = common.fetchAvailableModels() + assert.are_same({ "only-one" }, a) + assert.are_same("second-return", b) + end) + + it("surfaces errors from fetchModels instead of swallowing them", function() + fakeOllamaClient.fetchModels = function() + error("fetch-models-failure") + end + + local ok, err = pcall(common.fetchAvailableModels) + assert.is_false(ok, "should propagate errors from fetchModels") + assert.truthy( + string.find(err, "fetch-models-failure", 1, true), + string.format("error message should mention the original failure, got: %s", tostring(err)) + ) + end) + end) + + ------------------------------------------------------------------------ + -- 4. sendDataToApi wrapper — highest priority + ------------------------------------------------------------------------ + describe("sendDataToApi delegation", function() + it("forwards all 7 arguments in correct order", function() + local received + fakeOllamaClient.generate = function(...) + received = pack(...) + return { title = "T", caption = "C", keywords = { "k1" } }, nil + end + + local photo = { _type = "photo" } + local prompt = "PROMPT-ARGUMENT" + local currentData = { title = "CURRENT-TITLE", caption = "CURRENT-CAPTION" } + local useCurrent = true + local useSystem = false + local model = "MODEL-ARGUMENT" + local sysPrompt = "SYSTEM-PROMPT-ARGUMENT" + + common.sendDataToApi(photo, prompt, currentData, useCurrent, + useSystem, model, sysPrompt) + + assert.are_same(7, received.n, "generate must receive all 7 arguments") + assert.is_true(received[1] == photo, "arg 1: photo identity") + assert.are_same(prompt, received[2], "arg 2: prompt value") + assert.is_true(received[3] == currentData, "arg 3: currentData identity") + assert.is_true(received[4] == useCurrent, "arg 4: useCurrentData boolean") + assert.is_false(received[5], "arg 5: useSystemPrompt boolean") + assert.are_same(model, received[6], "arg 6: selectedModel value") + assert.are_same(sysPrompt, received[7], "arg 7: systemPrompt value") + end) + + it("preserves nil middle arguments without shifting later args", function() + local received + fakeOllamaClient.generate = function(...) + received = pack(...) + return {}, nil + end + + common.sendDataToApi( + { _p = "photo" }, -- 1: photo + "prompt", -- 2: prompt + nil, -- 3: currentData is nil + true, -- 4: useCurrentData + false, -- 5: useSystemPrompt + nil, -- 6: selectedModel is nil + "sys-prompt" -- 7: systemPrompt + ) + + assert.are_same(7, received.n, "must forward exactly 7 positions even with nil intermediates") + assert.is_true(type(received[1]) == "table", "arg 1 should be the photo table") + assert.are_same("prompt", received[2], "arg 2 should be 'prompt'") + assert.is_nil(received[3], "arg 3 should be nil (currentData)") + assert.is_true(received[4], "arg 4 should be true (useCurrentData)") + assert.is_false(received[5], "arg 5 should be false (useSystemPrompt)") + assert.is_nil(received[6], "arg 6 should be nil (selectedModel)") + assert.are_same("sys-prompt", received[7], "arg 7 should still be system prompt") + end) + + it("propagates both return values on success", function() + local response = { + title = "Generated title", + caption = "Generated caption", + keywords = { "one", "two" }, + } + fakeOllamaClient.generate = function() + return response, nil + end + + local a, b = common.sendDataToApi(nil, nil, nil, nil, nil, nil, nil) + assert.is_true(a == response, "response table identity preserved") + assert.is_nil(b, "error should be nil on success") + end) + + it("propagates both return values on failure", function() + fakeOllamaClient.generate = function() + return nil, "generation failed" + end + + local a, b = common.sendDataToApi(nil, nil, nil, nil, nil, nil, nil) + assert.is_nil(a, "response should be nil on failure") + assert.are_same("generation failed", b) + end) + + it("surfaces errors from generate instead of swallowing", function() + fakeOllamaClient.generate = function() + error("generate-wrapper-failure") + end + + local ok, err = pcall(common.sendDataToApi, nil, nil, nil, nil, nil, nil, nil) + assert.is_false(ok, "should propagate errors from generate") + assert.truthy( + string.find(err, "generate-wrapper-failure", 1, true), + string.format("error message should mention the original failure, got: %s", tostring(err)) + ) + end) + end) + + ------------------------------------------------------------------------ + -- 5. ThumbnailService delegation + ------------------------------------------------------------------------ + describe("exportThumbnail delegation", function() + it("forwards photo and propagates return value", function() + local received + fakeThumbnailService.export = function(photo) + received = photo + return "/fake/thumbnail-123.jpg" + end + + local photo = { _type = "photo" } + local result = common.exportThumbnail(photo) + assert.is_true(received == photo, "same photo object forwarded") + assert.are_same("/fake/thumbnail-123.jpg", result) + end) + + it("propagates multiple return values", function() + local received + fakeThumbnailService.export = function(p) + received = p + return "/path.jpg", "extra-return" + end + + local a, b = common.exportThumbnail({ _m = 1 }) + assert.is_true(received == { _m = 1 } or received._m == 1) + assert.are_same("/path.jpg", a) + assert.are_same("extra-return", b) + end) + end) + + describe("base64EncodeImage delegation", function() + it("forwards path and propagates return value", function() + local received + fakeThumbnailService.encodeBase64 = function(imagePath) + received = imagePath + return "encoded-data-xyz" + end + + local result = common.base64EncodeImage("/some/image.jpg") + assert.are_same("/some/image.jpg", received) + assert.are_same("encoded-data-xyz", result) + end) + + it("propagates multiple return values", function() + local received + fakeThumbnailService.encodeBase64 = function(path) + received = path + return "data", "second" + end + + local a, b = common.base64EncodeImage("/path.jpg") + assert.are_same("/path.jpg", received) + assert.are_same("data", a) + assert.are_same("second", b) + end) + end) + + ------------------------------------------------------------------------ + -- 6. MetadataService delegation + ------------------------------------------------------------------------ + describe("addKeywordsWithParent delegation", function() + it("forwards all arguments with correct identity", function() + local receivedCatalog, receivedPhoto, receivedKw + fakeMetadataService.addKeywordsWithParent = function(catalog, photo, keywords) + receivedCatalog = catalog + receivedPhoto = photo + receivedKw = keywords + end + + local catalog = { _type = "catalog" } + local photo = { _type = "photo" } + local keywords = { "sunset", "portrait" } + + common.addKeywordsWithParent(catalog, photo, keywords) + assert.is_true(receivedCatalog == catalog, "same catalog object") + assert.is_true(receivedPhoto == photo, "same photo object") + assert.is_true(receivedKw == keywords, "same keywords table") + end) + end) + + describe("getLlmKeywordsFromPhoto delegation", function() + it("forwards photo and propagates return value", function() + local received + local expectedResult = { "cat", "outdoor" } + fakeMetadataService.getLlmKeywordsFromPhoto = function(photo) + received = photo + return expectedResult + end + + local photo = { _type = "photo" } + local result = common.getLlmKeywordsFromPhoto(photo) + assert.is_true(received == photo, "same photo object") + assert.are_same(expectedResult, result) + end) + end) + + describe("removeLlmKeywords delegation", function() + it("forwards catalog and photo with correct identity", function() + local receivedCatalog, receivedPhoto + fakeMetadataService.removeLlmKeywords = function(catalog, photo) + receivedCatalog = catalog + receivedPhoto = photo + end + + local catalog = { _type = "catalog" } + local photo = { _type = "photo" } + + common.removeLlmKeywords(catalog, photo) + assert.is_true(receivedCatalog == catalog, "same catalog object") + assert.is_true(receivedPhoto == photo, "same photo object") + end) + end) + + describe("parseKeywordCsv delegation", function() + it("delegates to MetadataService instead of duplicating parser logic", function() + local receivedCsv + fakeMetadataService.parseKeywordCsv = function(value) + receivedCsv = value + return { "delegated-result" } + end + + local result = common.parseKeywordCsv("sunset, beach, portrait") + assert.are_same("sunset, beach, portrait", receivedCsv) + assert.are_same({ "delegated-result" }, result) + end) + + it("propagates nil input without error", function() + local receivedCsv + fakeMetadataService.parseKeywordCsv = function(value) + receivedCsv = value + return {} + end + + common.parseKeywordCsv(nil) + assert.is_nil(receivedCsv, "nil should be forwarded to the delegate") + end) + end) + + ------------------------------------------------------------------------ + -- 7. API surface regression — all expected keys present + ------------------------------------------------------------------------ + describe("complete API surface", function() + it("has every expected compatibility key", function() + local expectedFunctions = { + "validateServerHost", + "makeModelItems", + "saveServerAndRefresh", + "fetchAvailableModels", + "exportThumbnail", + "base64EncodeImage", + "sendDataToApi", + "addKeywordsWithParent", + "getLlmKeywordsFromPhoto", + "removeLlmKeywords", + "parseKeywordCsv", + } + for _, name in ipairs(expectedFunctions) do + assert.is_function( + common[name], + string.format("common must have function '%s'", name) + ) + end + + -- Constants + assert.is_not_nil(common.model, "common must have 'model' constant") + assert.is_not_nil(common.defaultServerHost, "common must have 'defaultServerHost' constant") + end) + end) +end) diff --git a/tests/spec/lrllama_entrypoint_spec.lua b/tests/spec/lrllama_entrypoint_spec.lua new file mode 100644 index 0000000..906eb35 --- /dev/null +++ b/tests/spec/lrllama_entrypoint_spec.lua @@ -0,0 +1,382 @@ +--- lrllama_entrypoint_spec.lua — Smoke tests for LrLlama.lua entry point. +--- Because the file imports Lightroom SDK modules at top level (LrApplication, +--- LrDialogs, etc.), it cannot be loaded with loadfile() outside Lightroom. +--- These tests verify structure via source inspection and validate the client +--- adapter interface with a fake Common module. + +local path = PLUGIN_PATH + +describe("LrLlama.lua entrypoint", function() + local source + + before_each(function() + source = assert(io.open(path .. "LrLlama.lua", "r")):read("*a") + end) + + --------------------------------------------------------------- + -- A. MODULE LOADING + --------------------------------------------------------------- + describe("loads dependency modules", function() + it("loads Common.lua via loadfile", function() + assert.truthy( + string.find(source, 'loadfile.*Common%.lua'), + "Entrypoint must load Common.lua" + ) + end) + + it("loads PromptBuilder.lua via loadfile", function() + assert.truthy( + string.find(source, 'loadfile.*PromptBuilder%.lua'), + "Entrypoint must load PromptBuilder.lua" + ) + end) + + it("loads LlamaDialog.lua via loadfile", function() + assert.truthy( + string.find(source, 'loadfile.*LlamaDialog%.lua'), + "Entrypoint must load LlamaDialog.lua" + ) + end) + end) + + --------------------------------------------------------------- + -- B. ASYNC EXECUTION + --------------------------------------------------------------- + describe("async execution", function() + it("runs main in LrTasks.startAsyncTask", function() + assert.truthy( + string.find(source, "LrTasks%.startAsyncTask"), + "Entrypoint must start async task" + ) + end) + end) + + --------------------------------------------------------------- + -- C. NO-PHOTO SAFEGUARD + --------------------------------------------------------------- + describe("no-photo safeguard", function() + it("gets target photos from catalog", function() + assert.truthy( + string.find(source, "getTargetPhotos"), + "Entrypoint must query selected photos" + ) + end) + + it("checks for empty selection", function() + assert.truthy( + string.find(source, "#selectedPhotos == 0"), + "Entrypoint must guard against no selection" + ) + end) + + it("shows message when no photo selected", function() + assert.truthy( + string.find(source, "LrDialogs%.message"), + "Entrypoint must show error message for empty selection" + ) + end) + end) + + --------------------------------------------------------------- + -- D. CLIENT ADAPTER INTERFACE + --------------------------------------------------------------- + describe("client adapter", function() + it("defines makeClientAdapter function", function() + assert.truthy( + string.find(source, "function makeClientAdapter"), + "Entrypoint must define makeClientAdapter" + ) + end) + + it("adapter delegates generate to Common.sendDataToApi", function() + assert.truthy( + string.find(source, "common%.sendDataToApi"), + "Adapter must delegate generate to Common.sendDataToApi" + ) + end) + + it("adapter delegates fetchModels to Common.fetchAvailableModels", function() + assert.truthy( + string.find(source, "common%.fetchAvailableModels"), + "Adapter must delegate fetchModels" + ) + end) + + it("adapter exposes validateServerHost", function() + assert.truthy( + string.find(source, "validateServerHost.*common%.validateServerHost"), + "Adapter must expose validateServerHost from Common" + ) + end) + + it("adapter exposes saveServerAndRefresh", function() + assert.truthy( + string.find(source, "saveServerAndRefresh.*common%.saveServerAndRefresh"), + "Adapter must expose saveServerAndRefresh from Common" + ) + end) + + it("adapter exposes makeModelItems", function() + assert.truthy( + string.find(source, "makeModelItems.*common%.makeModelItems"), + "Adapter must expose makeModelItems from Common" + ) + end) + + it("adapter exposes defaultModel constant", function() + assert.truthy( + string.find(source, "defaultModel.*=.*common%.model"), + "Adapter must expose defaultModel from Common.model" + ) + end) + + it("adapter exposes defaultServerHost constant", function() + assert.truthy( + string.find(source, "defaultServerHost.*=.*common%.defaultServerHost"), + "Adapter must expose defaultServerHost" + ) + end) + end) + + --------------------------------------------------------------- + -- E. DELEGATION TO LLAMADIALOG + --------------------------------------------------------------- + describe("delegation to LlamaDialog", function() + it("calls LlamaDialog.show with client adapter", function() + assert.truthy( + string.find(source, "LlamaDialog%.show"), + "Entrypoint must call LlamaDialog.show" + ) + end) + + it("passes client to dialog", function() + assert.truthy( + string.find(source, "client.*=.*makeClientAdapter"), + "Entrypoint must pass client adapter to dialog" + ) + end) + + it("passes common to dialog", function() + assert.truthy( + string.find(source, "= Common"), + "Entrypoint must pass common utilities to dialog" + ) + end) + + it("passes promptBuilder to dialog", function() + assert.truthy( + string.find(source, "= PromptBuilder"), + "Entrypoint must pass promptBuilder to dialog" + ) + end) + + it("passes prefs to dialog", function() + assert.truthy( + string.find(source, "LrPrefs.prefsForPlugin"), + "Entrypoint must pass prefs to dialog" + ) + end) + + it("passes catalog and photo to dialog", function() + assert.truthy(string.find(source, "catalog[[:%s]]*=")) + assert.truthy(string.find(source, "photo[[:%s]]*=")) + end) + + it("passes thumbnailPath to dialog", function() + assert.truthy( + string.find(source, "thumbnailPath"), + "Entrypoint must pass thumbnail path to dialog" + ) + end) + + it("passes statusMessages to dialog", function() + assert.truthy( + string.find(source, "statusMessages"), + "Entrypoint must pass status messages to dialog" + ) + end) + + it("passes writeMetadata callback to dialog", function() + assert.truthy( + string.find(source, "writeMetadata"), + "Entrypoint must pass writeMetadata callback" + ) + end) + end) + + --------------------------------------------------------------- + -- F. WRITEMETADATA CLOSURE + --------------------------------------------------------------- + describe("writeMetadata closure", function() + it("uses catalog withWriteAccessDo for transactional writes", function() + assert.truthy( + string.find(source, "withWriteAccessDo"), + "writeMetadata must use catalog transaction" + ) + end) + + it("sets title via setRawMetadata", function() + assert.truthy( + string.find(source, 'setRawMetadata.*title'), + "writeMetadata must set title" + ) + end) + + it("sets caption via setRawMetadata", function() + assert.truthy( + string.find(source, 'setRawMetadata.*caption'), + "writeMetadata must set caption" + ) + end) + + it("adds keywords via Common.addKeywordsWithParent", function() + assert.truthy( + string.find(source, "Common%.addKeywordsWithParent"), + "writeMetadata must use addKeywordsWithParent for keywords" + ) + end) + end) + + --------------------------------------------------------------- + -- G. NO INLINE BUSINESS LOGIC + --------------------------------------------------------------- + describe("no inline business logic", function() + it("has no inline keyword parsing loop", function() + assert.falsy( + string.find(source, "gmatch.*keywords.*%[%,", nil, true), + "Entrypoint must not contain inline gmatch keyword parsing" + ) + end) + + it("has no HTTP calls — delegates to Common/adapter", function() + assert.falsy( + string.find(source, "LrHttp"), + "Entrypoint must not make HTTP calls directly" + ) + end) + + it("has no LrView usage — delegates to LlamaDialog", function() + assert.falsy( + string.find(source, "LrView"), + "Entrypoint must not construct UI directly" + ) + end) + + it("has no binding construction — delegates to LlamaDialog", function() + assert.falsy( + string.find(source, "LrBinding"), + "Entrypoint must not create bindings directly" + ) + end) + end) +end) + +describe("makeClientAdapter interface shape", function() + -- Verify the adapter interface matches what SinglePhotoController expects. + -- Build a minimal fake Common and inline the adapter logic to ensure the + -- contract is stable. + + local function makeFakeCommon() + return { + sendDataToApi = function() return {}, nil end, + fetchAvailableModels = function() return {} end, + validateServerHost = function(host) return true, host end, + saveServerAndRefresh = function(state, prefs) return true, "ok" end, + makeModelItems = function(names) return names end, + model = "gemma4:latest", + defaultServerHost = "localhost:11434", + } + end + + local function makeClientAdapter(common) + return { + generate = function(photo, userInstruction, currentData, useCurrentData, + useSystemPrompt, selectedModel, systemPromptOverride) + return common.sendDataToApi(photo, userInstruction, currentData, + useCurrentData, useSystemPrompt, + selectedModel, systemPromptOverride) + end, + fetchModels = function() + return common.fetchAvailableModels() + end, + validateServerHost = common.validateServerHost, + saveServerAndRefresh = common.saveServerAndRefresh, + makeModelItems = common.makeModelItems, + defaultModel = common.model, + defaultServerHost = common.defaultServerHost, + } + end + + local adapter + local fakeCommon + + before_each(function() + fakeCommon = makeFakeCommon() + adapter = makeClientAdapter(fakeCommon) + end) + + it("has all required methods", function() + assert.is_function(adapter.generate) + assert.is_function(adapter.fetchModels) + assert.is_function(adapter.validateServerHost) + assert.is_function(adapter.saveServerAndRefresh) + assert.is_function(adapter.makeModelItems) + end) + + it("has all required constants", function() + assert.are_same("gemma4:latest", adapter.defaultModel) + assert.are_same("localhost:11434", adapter.defaultServerHost) + end) + + it("generate delegates to sendDataToApi with correct arg count", function() + local callArgs = nil + fakeCommon.sendDataToApi = function(p, u, c, uc, us, m, s) + callArgs = { p, u, c, uc, us, m, s } + return {}, nil + end + + adapter.generate("photo", "prompt", { title = "T" }, true, true, "model", "sys") + assert.are_same(7, #callArgs) + assert.are_same("photo", callArgs[1]) + assert.are_same("prompt", callArgs[2]) + end) + + it("fetchModels delegates to fetchAvailableModels", function() + local called = false + fakeCommon.fetchAvailableModels = function() + called = true + return { "m1" } + end + local result = adapter.fetchModels() + assert.is_true(called) + assert.are_same({ "m1" }, result) + end) + + it("validateServerHost is the Common function", function() + local ok, host = adapter.validateServerHost("host:1234") + assert.is_true(ok) + assert.are_same("host:1234", host) + end) + + it("saveServerAndRefresh passes through correctly", function() + local ok, msg = adapter.saveServerAndRefresh({}, {}) + assert.is_true(ok) + assert.are_same("ok", msg) + end) + + it("matches SinglePhotoController expected interface keys", function() + -- Mirror the dependency validation in SinglePhotoController.new() + -- to catch interface drift. + local expectedKeys = { + "generate", "fetchModels", "validateServerHost", + "saveServerAndRefresh", "makeModelItems", + "defaultModel", "defaultServerHost" + } + for _, key in ipairs(expectedKeys) do + assert.is_not_nil( + adapter[key], + string.format("Adapter must have '%s' for SinglePhotoController", key) + ) + end + end) +end) diff --git a/tests/spec/metadata_service_keywords_spec.lua b/tests/spec/metadata_service_keywords_spec.lua new file mode 100644 index 0000000..d2bca12 --- /dev/null +++ b/tests/spec/metadata_service_keywords_spec.lua @@ -0,0 +1,546 @@ +--- metadata_service_keywords_spec.lua — Unit tests for MetadataService catalog-bound +--- keyword functions: addKeywordsWithParent, getLlmKeywordsFromPhoto, removeLlmKeywords. +--- Uses plain-Lua fake objects; no real Lightroom SDK required. + +local path = PLUGIN_PATH + +-------------------------------------------------------------------------------- +--- Factory helpers for fake Lightroom keyword objects. +-------------------------------------------------------------------------------- +local function makeParent(name) + return { + getName = function() + return name + end, + } +end + +local function makeKeyword(name, parent) + return { + getName = function() + return name + end, + getParent = function() + return parent + end, + } +end + +-------------------------------------------------------------------------------- +--- Helper: build a fake catalog + photo with call recording. +--- Each call returns configurable values set by individual tests. +-------------------------------------------------------------------------------- +local function makeFakeCatalog() + local catalog = { + createKeyword_calls = {}, + -- Per-call return values: index into this array by call number + createKeyword_returns = {}, + } + -- MetadataService calls with colon notation: catalog:createKeyword(...) + -- so self is passed as first argument. Use dot-notation signature that + -- explicitly receives and ignores self. + catalog.createKeyword = function(_, name, _, scope, parent, system) + table.insert(catalog.createKeyword_calls, { + name = name, + scope = scope, + parent = parent, + system = system, + }) + local idx = #catalog.createKeyword_calls + return catalog.createKeyword_returns[idx] + end + return catalog +end + +local function makeFakePhoto() + local photo = { + addKeyword_calls = {}, + removeKeyword_calls = {}, + getRawMetadata_return = nil, + getRawMetadata_error = nil, + } + -- All called with colon notation: photo:xxx(...) → self is first arg + photo.addKeyword = function(_, keyword) + table.insert(photo.addKeyword_calls, keyword) + end + photo.removeKeyword = function(_, keyword) + table.insert(photo.removeKeyword_calls, keyword) + end + photo.getRawMetadata = function(_, key) + if photo.getRawMetadata_error then + error(photo.getRawMetadata_error) + end + return photo.getRawMetadata_return + end + return photo +end + +-------------------------------------------------------------------------------- + +describe("MetadataService.addKeywordsWithParent", function() + local metadata + local catalog + local photo + + before_each(function() + metadata = assert(loadfile(path .. "MetadataService.lua"))() + catalog = makeFakeCatalog() + photo = makeFakePhoto() + end) + + --------------------------------------------------------------- + --- Invalid input — early return without side effects + --------------------------------------------------------------- + describe("invalid input", function() + it("returns without calling catalog methods for nil", function() + metadata.addKeywordsWithParent(catalog, photo, nil) + assert.are_same(0, #catalog.createKeyword_calls) + assert.are_same(0, #photo.addKeyword_calls) + end) + + it("returns without calling catalog methods for a string", function() + metadata.addKeywordsWithParent(catalog, photo, "sunset") + assert.are_same(0, #catalog.createKeyword_calls) + end) + + it("returns without calling catalog methods for a number", function() + metadata.addKeywordsWithParent(catalog, photo, 42) + assert.are_same(0, #catalog.createKeyword_calls) + end) + + it("returns without calling catalog methods for a boolean", function() + metadata.addKeywordsWithParent(catalog, photo, true) + assert.are_same(0, #catalog.createKeyword_calls) + end) + end) + + --------------------------------------------------------------- + --- Empty table — documents current behavior (parent still created) + --------------------------------------------------------------- + describe("empty table", function() + before_each(function() + catalog.createKeyword_returns[1] = makeParent("llm") + end) + + it("creates the llm parent keyword", function() + metadata.addKeywordsWithParent(catalog, photo, {}) + assert.are_same(1, #catalog.createKeyword_calls) + assert.are_same("llm", catalog.createKeyword_calls[1].name) + end) + + it("does not create child keywords", function() + metadata.addKeywordsWithParent(catalog, photo, {}) + assert.are_same(1, #catalog.createKeyword_calls) + assert.are_same(0, #photo.addKeyword_calls) + end) + end) + + --------------------------------------------------------------- + --- Parent creation arguments + --------------------------------------------------------------- + describe("parent creation", function() + before_each(function() + catalog.createKeyword_returns[1] = makeParent("llm") + end) + + it("creates parent with correct arguments", function() + metadata.addKeywordsWithParent(catalog, photo, { "sunset" }) + local call = catalog.createKeyword_calls[1] + assert.are_same("llm", call.name) + assert.are_same(true, call.scope) + assert.are_same(nil, call.parent) + assert.are_same(true, call.system) + end) + end) + + --------------------------------------------------------------- + --- Success path — child creation and association + --------------------------------------------------------------- + describe("success path", function() + local llmParent + local sunsetChild + local portraitChild + + before_each(function() + llmParent = { marker = "llm-parent" } + sunsetChild = { marker = "sunset-child" } + portraitChild = { marker = "portrait-child" } + catalog.createKeyword_returns[1] = llmParent + catalog.createKeyword_returns[2] = sunsetChild + catalog.createKeyword_returns[3] = portraitChild + end) + + it("creates parent once and children under it", function() + metadata.addKeywordsWithParent(catalog, photo, { "sunset", "portrait" }) + assert.are_same(3, #catalog.createKeyword_calls) + -- Parent created with nil parent-arg + assert.are_same(nil, catalog.createKeyword_calls[1].parent) + -- Children created under exact llmParent object + assert.is_true(catalog.createKeyword_calls[2].parent == llmParent) + assert.is_true(catalog.createKeyword_calls[3].parent == llmParent) + end) + + it("adds each child keyword to the photo in order", function() + metadata.addKeywordsWithParent(catalog, photo, { "sunset", "portrait" }) + assert.are_same(2, #photo.addKeyword_calls) + assert.is_true(photo.addKeyword_calls[1] == sunsetChild) + assert.is_true(photo.addKeyword_calls[2] == portraitChild) + end) + + it("passes correct arguments for child creation", function() + metadata.addKeywordsWithParent(catalog, photo, { "sunset" }) + local childCall = catalog.createKeyword_calls[2] + assert.are_same("sunset", childCall.name) + assert.are_same(true, childCall.scope) + assert.is_true(childCall.parent == llmParent) + assert.are_same(true, childCall.system) + end) + end) + + --------------------------------------------------------------- + --- Empty and nil entries + --------------------------------------------------------------- + describe("empty and nil entries", function() + before_each(function() + catalog.createKeyword_returns[1] = makeParent("llm") + end) + + it("skips empty string entries", function() + local child1 = { marker = "child1" } + local child2 = { marker = "child2" } + catalog.createKeyword_returns[2] = child1 + catalog.createKeyword_returns[3] = child2 + metadata.addKeywordsWithParent(catalog, photo, { "sunset", "", "portrait" }) + -- 1 parent + 2 children (empty skipped) + assert.are_same(3, #catalog.createKeyword_calls) + assert.are_same(2, #photo.addKeyword_calls) + end) + + it("stops at nil hole due to ipairs semantics", function() + local child1 = { marker = "child1" } + catalog.createKeyword_returns[2] = child1 + -- ipairs stops at first nil, so "portrait" after nil is never reached + metadata.addKeywordsWithParent(catalog, photo, { "sunset", nil, "portrait" }) + -- 1 parent + 1 child (only "sunset" processed) + assert.are_same(2, #catalog.createKeyword_calls) + assert.are_same(1, #photo.addKeyword_calls) + end) + end) + + --------------------------------------------------------------- + --- Partial child creation failure + --------------------------------------------------------------- + describe("partial child failure", function() + before_each(function() + catalog.createKeyword_returns[1] = makeParent("llm") + -- First child creation fails (returns nil) + catalog.createKeyword_returns[2] = nil + -- Second child succeeds + catalog.createKeyword_returns[3] = { marker = "portrait-child" } + end) + + it("skips failed child but continues processing", function() + metadata.addKeywordsWithParent(catalog, photo, { "sunset", "portrait" }) + -- 1 parent + 2 children attempted + assert.are_same(3, #catalog.createKeyword_calls) + -- Only successful child added to photo + assert.are_same(1, #photo.addKeyword_calls) + end) + + it("does not call addKeyword for nil child", function() + metadata.addKeywordsWithParent(catalog, photo, { "sunset", "portrait" }) + -- The first addKeyword would be for sunset (which returned nil) — must not be called + assert.are_same(1, #photo.addKeyword_calls) + end) + end) + + --------------------------------------------------------------- + --- Parent creation failure + --------------------------------------------------------------- + describe("parent creation failure", function() + before_each(function() + catalog.createKeyword_returns[1] = nil + end) + + it("raises error when parent cannot be created", function() + assert.has_error( + function() + metadata.addKeywordsWithParent(catalog, photo, { "sunset" }) + end, + "Failed to create or get 'llm' parent keyword" + ) + end) + + it("does not create children when parent fails", function() + pcall(function() + metadata.addKeywordsWithParent(catalog, photo, { "sunset" }) + end) + -- Only the parent creation was attempted + assert.are_same(1, #catalog.createKeyword_calls) + assert.are_same(0, #photo.addKeyword_calls) + end) + end) +end) + +-------------------------------------------------------------------------------- + +describe("MetadataService.getLlmKeywordsFromPhoto", function() + local metadata + local photo + + before_each(function() + metadata = assert(loadfile(path .. "MetadataService.lua"))() + photo = makeFakePhoto() + end) + + --------------------------------------------------------------- + --- Keyword filtering + --------------------------------------------------------------- + describe("keyword filtering", function() + it("returns only llm-parented keywords in order", function() + local llmParent = makeParent("llm") + local peopleParent = makeParent("people") + photo.getRawMetadata_return = { + makeKeyword("dog", peopleParent), -- not llm + makeKeyword("sunset", llmParent), -- llm + makeKeyword("landscape"), -- no parent (top-level) + makeKeyword("portrait", llmParent), -- llm + } + local result = metadata.getLlmKeywordsFromPhoto(photo) + assert.are_same({ "sunset", "portrait" }, result) + end) + + it("returns empty table when no keywords", function() + photo.getRawMetadata_return = nil + assert.are_same({}, metadata.getLlmKeywordsFromPhoto(photo)) + end) + + it("returns empty table for empty keyword list", function() + photo.getRawMetadata_return = {} + assert.are_same({}, metadata.getLlmKeywordsFromPhoto(photo)) + end) + + it("requires exact parent name match — rejects LLM", function() + local llmParent = makeParent("llm") + local upperParent = makeParent("LLM") + photo.getRawMetadata_return = { + makeKeyword("good", llmParent), + makeKeyword("bad", upperParent), + } + assert.are_same({ "good" }, metadata.getLlmKeywordsFromPhoto(photo)) + end) + + it("requires exact parent name match — rejects llm-child and my-llm", function() + local llmParent = makeParent("llm") + local childParent = makeParent("llm-child") + local myParent = makeParent("my-llm") + photo.getRawMetadata_return = { + makeKeyword("keep", llmParent), + makeKeyword("drop1", childParent), + makeKeyword("drop2", myParent), + } + assert.are_same({ "keep" }, metadata.getLlmKeywordsFromPhoto(photo)) + end) + end) + + --------------------------------------------------------------- + --- pcall error protection — all-or-nothing behavior + --------------------------------------------------------------- + describe("pcall error protection", function() + it("returns empty table when getRawMetadata raises", function() + photo.getRawMetadata_error = "metadata unavailable" + assert.are_same({}, metadata.getLlmKeywordsFromPhoto(photo)) + end) + + it("returns empty table when getParent raises on any keyword", function() + local llmParent = makeParent("llm") + local badKeyword = { + getName = function() return "broken" end, + getParent = function() error("getParent failed") end, + } + photo.getRawMetadata_return = { + makeKeyword("sunset", llmParent), + badKeyword, + } + -- pcall wraps entire loop — returns {} on any error + assert.are_same( + {}, + metadata.getLlmKeywordsFromPhoto(photo), + "pcall catches getParent error and returns empty table" + ) + end) + + it("returns empty table when parent getName raises", function() + local badParent = { + getName = function() error("getName failed") end, + } + photo.getRawMetadata_return = { + makeKeyword("sunset", badParent), + } + assert.are_same( + {}, + metadata.getLlmKeywordsFromPhoto(photo), + "pcall catches parent:getName error" + ) + end) + + it("returns empty table when keyword getName raises after parent match", function() + -- This tests the case where parent matches "llm" but the keyword's + -- own getName fails. The pcall wraps the entire loop, so partial + -- results are discarded — all-or-nothing behavior. + local goodParent = makeParent("llm") + local badKeyword = { + getParent = function() return goodParent end, + getName = function() error("getName failed") end, + } + photo.getRawMetadata_return = { + makeKeyword("sunset", goodParent), + badKeyword, + } + assert.are_same( + {}, + metadata.getLlmKeywordsFromPhoto(photo), + "pcall discards partial results when keyword getName fails" + ) + end) + end) +end) + +-------------------------------------------------------------------------------- + +describe("MetadataService.removeLlmKeywords", function() + local metadata + local catalog + local photo + + before_each(function() + metadata = assert(loadfile(path .. "MetadataService.lua"))() + catalog = makeFakeCatalog() + photo = makeFakePhoto() + end) + + --------------------------------------------------------------- + --- Removal filtering + --------------------------------------------------------------- + describe("removal filtering", function() + it("removes only llm-parented keywords", function() + local llmParent = makeParent("llm") + local peopleParent = makeParent("people") + local sunsetKeyword = makeKeyword("sunset", llmParent) + local dogKeyword = makeKeyword("dog", peopleParent) + local landscapeKw = makeKeyword("landscape") -- top-level + local portraitKw = makeKeyword("portrait", llmParent) + + photo.getRawMetadata_return = { + sunsetKeyword, -- index 1 — remove + dogKeyword, -- index 2 — keep + landscapeKw, -- index 3 — keep (top-level) + portraitKw, -- index 4 — remove + } + + metadata.removeLlmKeywords(catalog, photo) + + assert.are_same(2, #photo.removeKeyword_calls) + assert.is_true(photo.removeKeyword_calls[1] == sunsetKeyword) + assert.is_true(photo.removeKeyword_calls[2] == portraitKw) + end) + + it("does not remove keywords with similar parent names", function() + local llmParent = makeParent("llm") + local upperParent = makeParent("LLM") + local childParent = makeParent("llm-child") + local myParent = makeParent("my-llm") + + photo.getRawMetadata_return = { + makeKeyword("keep1", llmParent), -- remove (exact match) + makeKeyword("keep2", upperParent), -- keep + makeKeyword("keep3", childParent), -- keep + makeKeyword("keep4", myParent), -- keep + } + + metadata.removeLlmKeywords(catalog, photo) + + assert.are_same(1, #photo.removeKeyword_calls) + end) + + it("returns early when no keyword metadata", function() + photo.getRawMetadata_return = nil + metadata.removeLlmKeywords(catalog, photo) + assert.are_same(0, #photo.removeKeyword_calls) + end) + + it("does nothing for empty keyword list", function() + photo.getRawMetadata_return = {} + metadata.removeLlmKeywords(catalog, photo) + assert.are_same(0, #photo.removeKeyword_calls) + end) + end) + + --------------------------------------------------------------- + --- Catalog argument behavior + --------------------------------------------------------------- + describe("catalog argument", function() + it("accepts catalog without calling any catalog methods", function() + local llmParent = makeParent("llm") + photo.getRawMetadata_return = { + makeKeyword("sunset", llmParent), + } + metadata.removeLlmKeywords(catalog, photo) + -- removeLlmKeywords accepts catalog but does not use it + assert.are_same(0, #catalog.createKeyword_calls) + end) + + it("does not delete keyword definitions from catalog", function() + -- Use a catalog that throws on any method call to verify nothing is called + local throwCatalog = { + createKeyword_calls = {}, + createKeyword = function() + error("catalog method should not be called") + end, + deleteKeyword = function() + error("deleteKeyword should not be called") + end, + } + local llmParent = makeParent("llm") + photo.getRawMetadata_return = { + makeKeyword("sunset", llmParent), + } + -- Must not raise — function only touches photo, not catalog + metadata.removeLlmKeywords(throwCatalog, photo) + assert.are_same(1, #photo.removeKeyword_calls) + end) + end) +end) + +-------------------------------------------------------------------------------- + +describe("Error semantics characterization", function() + --- Documents the asymmetry: getLlmKeywordsFromPhoto uses pcall (returns {} on + --- error), while removeLlmKeywords does not (propagates errors). This is + --- intentional behavior — do not change it without an explicit decision. + + local metadata + + before_each(function() + metadata = assert(loadfile(path .. "MetadataService.lua"))() + end) + + it("getLlmKeywordsFromPhoto returns {} on metadata error (pcall)", function() + local photo = makeFakePhoto() + photo.getRawMetadata_error = "I/O error reading metadata" + local result = metadata.getLlmKeywordsFromPhoto(photo) + assert.are_same({}, result) + end) + + it("removeLlmKeywords propagates metadata error (no pcall)", function() + local photo = makeFakePhoto() + photo.getRawMetadata_error = "I/O error reading metadata" + local catalog = makeFakeCatalog() + assert.has_error( + function() + metadata.removeLlmKeywords(catalog, photo) + end, + "I/O error reading metadata" + ) + end) +end) diff --git a/tests/spec/metadata_service_parse_spec.lua b/tests/spec/metadata_service_parse_spec.lua new file mode 100644 index 0000000..83e4019 --- /dev/null +++ b/tests/spec/metadata_service_parse_spec.lua @@ -0,0 +1,51 @@ +--- metadata_service_parse_spec.lua — Unit tests for MetadataService.parseKeywordCsv +--- Tests CSV parsing, trimming, and edge cases. Does not test catalog-bound +--- methods (addKeywordsWithParent, getLlmKeywordsFromPhoto, removeLlmKeywords) +--- since they require real Lightroom SDK objects. + +local path = PLUGIN_PATH + +describe("MetadataService.parseKeywordCsv", function() + local metadata + + before_each(function() + metadata = assert(loadfile(path .. "MetadataService.lua"))() + end) + + it("returns empty table for nil input", function() + assert.are_same({}, metadata.parseKeywordCsv(nil)) + end) + + it("returns empty table for empty string", function() + assert.are_same({}, metadata.parseKeywordCsv("")) + end) + + it("parses a single keyword", function() + assert.are_same({ "sunset" }, metadata.parseKeywordCsv("sunset")) + end) + + it("parses multiple keywords", function() + local got = metadata.parseKeywordCsv("beach, sunset, calm") + assert.are_same({ "beach", "sunset", "calm" }, got) + end) + + it("trims whitespace around each keyword", function() + local got = metadata.parseKeywordCsv(" ocean , hills , sky ") + assert.are_same({ "ocean", "hills", "sky" }, got) + end) + + it("filters out empty entries from consecutive commas", function() + local got = metadata.parseKeywordCsv("a,,b, ,c") + assert.are_same({ "a", "b", "c" }, got) + end) + + it("handles trailing comma", function() + local got = metadata.parseKeywordCsv("one, two,") + assert.are_same({ "one", "two" }, got) + end) + + it("handles leading comma", function() + local got = metadata.parseKeywordCsv(",first, second") + assert.are_same({ "first", "second" }, got) + end) +end) diff --git a/tests/spec/ollama_client_generate_spec.lua b/tests/spec/ollama_client_generate_spec.lua new file mode 100644 index 0000000..c19d802 --- /dev/null +++ b/tests/spec/ollama_client_generate_spec.lua @@ -0,0 +1,642 @@ +--- ollama_client_generate_spec.lua — Unit tests for OllamaClient.generate() pipeline. +--- Covers success path, retry behavior, error propagation, exception-safe cleanup, +--- event ordering, and error-preservation policy. + +local path = PLUGIN_PATH + +-------------------------------------------------------------------------------- +--- Helper: build fakes with event recording aligned to OllamaClient internals. +--- Dot-notation deps (http.post, thumbnailService.export) get no self. +--- Colon-notation deps (json.encode) receive self as first arg. +-------------------------------------------------------------------------------- +local function makeFakes() + local events = {} + + local http = { post_calls = {}, _response = nil } + http.post = function(url, body, headers) + table.insert(events, "post") + table.insert(http.post_calls, { url = url, body = body, headers = headers }) + return http._response + end + + local prefs = {} + prefs.prefsForPlugin = function() + return { ollamaServerHost = "localhost:11434" } + end + + local tasks = { sleep_calls = {} } + tasks.sleep = function(s) + table.insert(tasks.sleep_calls, s) + end + + -- JSON uses colon notation (json:encode) — self is first arg. + local json = { encode_calls = {}, _return = nil } + json.encode = function(_, tbl) + table.insert(events, "json-encode") + table.insert(json.encode_calls, tbl) + return json._return or "{}" + end + + -- thumbnailService uses dot notation — no implicit self. + local ts = { cleanup_calls = {}, export_calls = {}, _export_return = nil } + ts.export = function(photo) + table.insert(events, "export") + table.insert(ts.export_calls, photo) + return ts._export_return + end + ts.encodeBase64 = function(imagePath) + table.insert(events, "encode") + return ts._encode_return + end + ts.cleanup = function(imagePath) + table.insert(events, "cleanup") + table.insert(ts.cleanup_calls, imagePath) + end + + -- promptBuilder uses dot notation — no implicit self. + local pb = { + buildUserPrompt_calls = {}, + assembleRequestBody_calls = {}, + _buildReturn = "default prompt", + _assembleReturn = { model = "test", prompt = "p" }, + } + pb.buildUserPrompt = function(instruction, currentData, useCurrent) + table.insert(events, "build-prompt") + table.insert(pb.buildUserPrompt_calls, { + instruction = instruction, + currentData = currentData, + useCurrent = useCurrent, + }) + return pb._buildReturn + end + pb.assembleRequestBody = function(userPrompt, model, useSys, override) + table.insert(events, "assemble-body") + table.insert(pb.assembleRequestBody_calls, { + userPrompt = userPrompt, + model = model, + useSystemPrompt = useSys, + systemPromptOverride = override, + }) + return pb._assembleReturn + end + + -- responseValidator uses dot notation. + local rv = { calls = {}, _metadata = nil, _error = nil } + rv.validateAndParse = function(rawResponse) + table.insert(events, "validate") + table.insert(rv.calls, rawResponse) + return rv._metadata, rv._error + end + + -- logger — colon notation for info/warn/error (OllamaClient calls :info, :warn, :error). + local logger = { info_calls = {}, warn_calls = {}, error_calls = {}, enable = function() end } + logger.info = function(_self, msg) table.insert(logger.info_calls, msg) end + logger.warn = function(_self, msg) table.insert(logger.warn_calls, msg) end + logger.error = function(_self, msg) table.insert(logger.error_calls, msg) end + + return { + events = events, + http = http, + prefs = prefs, + tasks = tasks, + json = json, + thumbnailService = ts, + promptBuilder = pb, + responseValidator = rv, + logger = logger, + } +end + +-------------------------------------------------------------------------------- +--- Assert helpers +-------------------------------------------------------------------------------- +local function assertEventOrder(expected) + return function(f) + for i, name in ipairs(expected) do + assert.are_same(name, f.events[i], + string.format("Expected event[%d] = '%s', got '%s'", i, name, tostring(f.events[i]))) + end + end +end + +-------------------------------------------------------------------------------- +describe("OllamaClient — generate()", function() + + local Client + local c + local f + + -------------------------------------------------------------------- + -- 1. Happy path: full pipeline with event ordering + -------------------------------------------------------------------- + it("orchestrates the full pipeline on first attempt", function() + f = makeFakes() + f.thumbnailService._export_return = "/tmp/thumb.jpg" + f.thumbnailService._encode_return = "base64data" + f.http._response = '{"response":"{\"title\":\"T\",\"caption\":\"C\",\"keywords\":[\"k\"]}"}' + f.responseValidator._metadata = { title = "T", caption = "C", keywords = { "k" } } + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + local result, err = c.generate( + { uuid = "photo-1" }, + "Caption this", + nil, false, true, "gemma4:latest", nil + ) + + -- Event order: export → encode → build-prompt → assemble-body → json-encode + -- → post → cleanup → validate + assertEventOrder({ + "export", "encode", "build-prompt", "assemble-body", + "json-encode", "post", "cleanup", "validate" + })(f) + + -- Cleanup exactly once with correct path. + assert.are_same(1, #f.thumbnailService.cleanup_calls) + assert.are_same("/tmp/thumb.jpg", f.thumbnailService.cleanup_calls[1]) + + -- Result from validator returned unchanged. + assert.are_same("T", result.title) + assert.is_nil(err) + end) + + -------------------------------------------------------------------- + -- 2. Encoding returns nil → cleanup, no POST/validate + -------------------------------------------------------------------- + it("cleans up when encoding returns nil", function() + f = makeFakes() + f.thumbnailService._export_return = "/tmp/t.jpg" + f.thumbnailService._encode_return = nil + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + local result, err = c.generate( + { uuid = "p" }, "hi", nil, false, true, "gemma4:latest", nil + ) + + assert.is_nil(result) + assert.are_same("Failed to encode image", err) + assert.are_same(1, #f.thumbnailService.cleanup_calls) + assert.are_same("/tmp/t.jpg", f.thumbnailService.cleanup_calls[1]) + assert.are_same(0, #f.http.post_calls) + assert.are_same(0, #f.responseValidator.calls) + end) + + -------------------------------------------------------------------- + -- 3. Encoding throws → SDK crashes propagate (no pcall around + -- yielding SDK functions). Cleanup is a best-effort trade-off: + -- normal nil-returns trigger cleanup; true SDK crashes do not. + -------------------------------------------------------------------- + it("propagates when encoding SDK function throws", function() + f = makeFakes() + f.thumbnailService._export_return = "/tmp/t.jpg" + f.thumbnailService.encodeBase64 = function(_) + table.insert(f.events, "encode") + error("encode exploded") + end + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + -- SDK throws propagate because we avoid wrapping yielding SDK calls in pcall. + local ok, err = pcall(c.generate, c, + { uuid = "p" }, "hi", nil, false, true, "gemma4:latest", nil + ) + + assert.is_false(ok) + assert.is_not_nil(string.find(tostring(err), "encode exploded", 1, true)) + -- Cleanup cannot run when SDK functions throw — acceptable trade-off + -- that avoids the "yield through protected boundary" problem. + assert.are_same(0, #f.http.post_calls) + end) + + -------------------------------------------------------------------- + -- 4. Prompt builder throws → cleanup runs, error returned. + -------------------------------------------------------------------- + it("cleans up when prompt builder throws", function() + f = makeFakes() + f.thumbnailService._export_return = "/tmp/t.jpg" + f.thumbnailService._encode_return = "img" + f.promptBuilder.buildUserPrompt = function() + table.insert(f.events, "build-prompt") + error("prompt builder error") + end + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + local result, err = c.generate( + { uuid = "p" }, "hi", nil, false, true, "gemma4:latest", nil + ) + + assert.is_nil(result) + assert.is_not_nil(string.find(tostring(err), "prompt builder error", 1, true)) + -- Cleanup runs despite exception. + assert.are_same(1, #f.thumbnailService.cleanup_calls) + assert.are_same("/tmp/t.jpg", f.thumbnailService.cleanup_calls[1]) + end) + + -------------------------------------------------------------------- + -- 5. Request-body assembly throws → cleanup runs, error returned. + -------------------------------------------------------------------- + it("cleans up when request-body assembly throws", function() + f = makeFakes() + f.thumbnailService._export_return = "/tmp/t.jpg" + f.thumbnailService._encode_return = "img" + f.promptBuilder.assembleRequestBody = function() + table.insert(f.events, "assemble-body") + error("assembly error") + end + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + local result, err = c.generate( + { uuid = "p" }, "hi", nil, false, true, "gemma4:latest", nil + ) + + assert.is_nil(result) + assert.is_not_nil(string.find(tostring(err), "assembly error", 1, true)) + -- Cleanup runs despite exception. + assert.are_same(1, #f.thumbnailService.cleanup_calls) + assert.are_same("/tmp/t.jpg", f.thumbnailService.cleanup_calls[1]) + end) + + -------------------------------------------------------------------- + -- 6. JSON encoding throws → cleanup runs, error returned. + -------------------------------------------------------------------- + it("cleans up when JSON encoding throws", function() + f = makeFakes() + f.thumbnailService._export_return = "/tmp/t.jpg" + f.thumbnailService._encode_return = "img" + f.json.encode = function(_) + table.insert(f.events, "json-encode") + error("json encoding failed") + end + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + local result, err = c.generate( + { uuid = "p" }, "hi", nil, false, true, "gemma4:latest", nil + ) + + assert.is_nil(result) + assert.is_not_nil(string.find(tostring(err), "json encoding failed", 1, true)) + -- Cleanup runs despite exception. + assert.are_same(1, #f.thumbnailService.cleanup_calls) + assert.are_same("/tmp/t.jpg", f.thumbnailService.cleanup_calls[1]) + assert.are_same(0, #f.http.post_calls) + end) + + -------------------------------------------------------------------- + -- 7. HTTP POST returns nil → cleanup before error return + -------------------------------------------------------------------- + it("cleans up when http.post returns nil", function() + f = makeFakes() + f.thumbnailService._export_return = "/tmp/t.jpg" + f.thumbnailService._encode_return = "img" + f.http._response = nil + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + local result, err = c.generate( + { uuid = "p" }, "hi", nil, false, true, "gemma4:latest", nil + ) + + assert.is_nil(result) + assert.are_same("Failed to send data to the API", err) + -- Cleanup runs before error return. + assertEventOrder({ + "export", "encode", "build-prompt", "assemble-body", + "json-encode", "post", "cleanup" + })(f) + assert.are_same(1, #f.thumbnailService.cleanup_calls) + end) + + -------------------------------------------------------------------- + -- 8. HTTP POST throws → SDK crashes propagate (no pcall around + -- yielding SDK functions). Cleanup is best-effort for SDK crashes. + -------------------------------------------------------------------- + it("propagates when http.post throws", function() + f = makeFakes() + f.thumbnailService._export_return = "/tmp/t.jpg" + f.thumbnailService._encode_return = "img" + f.http.post = function(_, _, _) + table.insert(f.events, "post") + error("post exploded") + end + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + -- SDK throws propagate because we avoid wrapping yielding SDK calls in pcall. + local ok, err = pcall(c.generate, c, + { uuid = "p" }, "hi", nil, false, true, "gemma4:latest", nil + ) + + assert.is_false(ok) + assert.is_not_nil(string.find(tostring(err), "post exploded", 1, true)) + end) + + -------------------------------------------------------------------- + -- 9. Validator returns error → cleanup happened before validation + -------------------------------------------------------------------- + it("cleans up before validator and returns validation error", function() + f = makeFakes() + f.thumbnailService._export_return = "/tmp/t.jpg" + f.thumbnailService._encode_return = "img" + f.http._response = '{"response":"raw inner"}' + f.responseValidator._metadata = nil + f.responseValidator._error = "invalid metadata" + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + local result, err = c.generate( + { uuid = "p" }, "hi", nil, false, true, "gemma4:latest", nil + ) + + assert.is_nil(result) + assert.are_same("invalid metadata", err) + -- Cleanup before validate in event order. + local cleanupIdx = nil + local validateIdx = nil + for i, e in ipairs(f.events) do + if e == "cleanup" then cleanupIdx = i end + if e == "validate" then validateIdx = i end + end + assert.is_not_nil(cleanupIdx) + assert.is_not_nil(validateIdx) + assert.are_true(cleanupIdx < validateIdx, + "Cleanup must occur before validation") + end) + + -------------------------------------------------------------------- + -- 10. Validator throws → cleanup has occurred already (validation + -- is outside runWithCleanup, so exception propagates after cleanup). + -------------------------------------------------------------------- + it("cleans up before validator even if validator throws", function() + f = makeFakes() + f.thumbnailService._export_return = "/tmp/t.jpg" + f.thumbnailService._encode_return = "img" + f.http._response = '{"response":"raw"}' + f.responseValidator.validateAndParse = function(_) + table.insert(f.events, "validate") + error("validation exploded") + end + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + -- Validation is outside runWithCleanup, so a thrown exception + -- propagates. Cleanup already happened inside runWithCleanup. + local ok, err = pcall(c.generate, c, + { uuid = "p" }, "hi", nil, false, true, "gemma4:latest", nil + ) + + assert.is_false(ok) + assert.is_not_nil(string.find(tostring(err), "validation exploded", 1, true)) + -- Cleanup ran before validation was even attempted. + assert.are_same(1, #f.thumbnailService.cleanup_calls) + end) + + -------------------------------------------------------------------- + -- 11. Cleanup fails after successful generation → error returned, not thrown. + -------------------------------------------------------------------- + it("returns cleanup error when cleanup fails after successful generation", function() + f = makeFakes() + f.thumbnailService._export_return = "/tmp/t.jpg" + f.thumbnailService._encode_return = "img" + f.http._response = '{"response":"{\"title\":\"T\",\"caption\":\"C\",\"keywords\":[\"k\"]}"}' + f.responseValidator._metadata = { title = "T", caption = "C", keywords = { "k" } } + -- Make cleanup throw (but still record the call). + f.thumbnailService.cleanup = function(imagePath) + table.insert(f.events, "cleanup") + table.insert(f.thumbnailService.cleanup_calls, imagePath) + error("disk full") + end + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + local result, err = c.generate( + { uuid = "p" }, "hi", nil, false, true, "gemma4:latest", nil + ) + + -- pcall catches cleanup exception and returns it as error. + assert.is_nil(result) + assert.is_not_nil(string.find(tostring(err), "Failed to clean up thumbnail:", 1, true)) + assert.is_not_nil(string.find(tostring(err), "disk full", 1, true)) + assert.are_same(1, #f.thumbnailService.cleanup_calls) + -- Validation should not have been called (cleanup failed before validation). + assert.are_same(0, #f.responseValidator.calls) + end) + + -------------------------------------------------------------------- + -- 12. Both generation AND cleanup fail → original error primary, cleanup logged. + -------------------------------------------------------------------- + it("preserves generation error when both generation and cleanup fail", function() + f = makeFakes() + f.thumbnailService._export_return = "/tmp/t.jpg" + f.thumbnailService._encode_return = "img" + -- JSON encoding throws (generation fails) + f.json.encode = function(_) + table.insert(f.events, "json-encode") + error("json encoding failed") + end + -- Cleanup also throws (but still records the call). + f.thumbnailService.cleanup = function(imagePath) + table.insert(f.events, "cleanup") + table.insert(f.thumbnailService.cleanup_calls, imagePath) + error("disk full") + end + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + local result, err = c.generate( + { uuid = "p" }, "hi", nil, false, true, "gemma4:latest", nil + ) + + -- Original generation error is primary. + assert.is_nil(result) + assert.is_not_nil(string.find(tostring(err), "json encoding failed", 1, true), + "Expected original generation error to be returned") + -- Cleanup failure was logged, not surfaced as primary error. + assert.are_same( + 1, #f.logger.error_calls, + "Cleanup failure should be logged" + ) + assert.is_not_nil( + string.find(tostring(f.logger.error_calls[1]), "disk full", 1, true), + "Logged message should contain cleanup error" + ) + -- Cleanup was attempted exactly once. + assert.are_same(1, #f.thumbnailService.cleanup_calls) + -- POST should not have been called (JSON encoding failed first). + assert.are_same(0, #f.http.post_calls) + end) + + -------------------------------------------------------------------- + -- 13. Export fails all 3 attempts → no cleanup + -------------------------------------------------------------------- + it("does not clean up when export fails all attempts", function() + f = makeFakes() + f.thumbnailService.export = function(photo) + table.insert(f.events, "export") + table.insert(f.thumbnailService.export_calls, photo) + return nil + end + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + local result, err = c.generate( + { uuid = "p" }, "hi", nil, false, true, "gemma4:latest", nil + ) + + assert.is_nil(result) + assert.are_same("Failed to export thumbnail after 3 attempts", err) + -- Export called 3 times. + assert.are_same(3, #f.thumbnailService.export_calls) + -- No cleanup (nothing to clean up). + assert.are_same(0, #f.thumbnailService.cleanup_calls) + end) + + -------------------------------------------------------------------- + -- 14. Export succeeds on second attempt → cleanup runs once + -------------------------------------------------------------------- + it("cleans up when export succeeds on second attempt", function() + f = makeFakes() + local attempt = 0 + f.thumbnailService.export = function(photo) + table.insert(f.events, "export") + table.insert(f.thumbnailService.export_calls, photo) + attempt = attempt + 1 + if attempt < 2 then return nil end + return "/tmp/t.jpg" + end + f.thumbnailService._encode_return = "img" + f.http._response = '{}' + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + local result, err = c.generate( + { uuid = "p" }, "hi", nil, false, true, "gemma4:latest", nil + ) + + -- Export called twice. + assert.are_same(2, #f.thumbnailService.export_calls) + -- One sleep between attempts. + assert.are_same(1, #f.tasks.sleep_calls) + -- Cleanup runs once with correct path. + assert.are_same(1, #f.thumbnailService.cleanup_calls) + assert.are_same("/tmp/t.jpg", f.thumbnailService.cleanup_calls[1]) + end) + + -------------------------------------------------------------------- + -- 15. Export succeeds on third attempt → cleanup runs once + -------------------------------------------------------------------- + it("cleans up when export succeeds on third attempt", function() + f = makeFakes() + local attempt = 0 + f.thumbnailService.export = function(photo) + table.insert(f.events, "export") + table.insert(f.thumbnailService.export_calls, photo) + attempt = attempt + 1 + if attempt < 3 then return nil end + return "/tmp/t.jpg" + end + f.thumbnailService._encode_return = "img" + f.http._response = '{}' + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + local result, err = c.generate( + { uuid = "p" }, "hi", nil, false, true, "gemma4:latest", nil + ) + + -- Export called three times. + assert.are_same(3, #f.thumbnailService.export_calls) + -- Two sleeps between attempts. + assert.are_same(2, #f.tasks.sleep_calls) + -- Cleanup runs once. + assert.are_same(1, #f.thumbnailService.cleanup_calls) + assert.are_same("/tmp/t.jpg", f.thumbnailService.cleanup_calls[1]) + end) + + -------------------------------------------------------------------- + -- 16. No double cleanup — explicit assertion across scenarios + -------------------------------------------------------------------- + it("never calls cleanup more than once in any post-export scenario", function() + -- This meta-test re-runs several configurations and asserts + -- cleanup count == 1 for every scenario where export succeeded. + local scenarios = { + -- Name, setup function + { + "success", + function(fake) + fake.thumbnailService._encode_return = "img" + fake.http._response = '{"response":"{\"title\":\"T\",\"caption\":\"C\",\"keywords\":[\"k\"]}"}' + fake.responseValidator._metadata = { title = "T" } + end, + }, + { + "encode-nil", + function(fake) + fake.thumbnailService._encode_return = nil + end, + }, + { + "http-nil", + function(fake) + fake.thumbnailService._encode_return = "img" + fake.http._response = nil + end, + }, + { + "prompt-throw", + function(fake) + fake.thumbnailService._encode_return = "img" + fake.promptBuilder.buildUserPrompt = function() + table.insert(fake.events, "build-prompt") + error("prompt builder error") + end + end, + }, + { + "json-throw", + function(fake) + fake.thumbnailService._encode_return = "img" + fake.json.encode = function(_) + table.insert(fake.events, "json-encode") + error("json encoding failed") + end + end, + }, + } + + for _, scenario in ipairs(scenarios) do + local name, setup = scenario[1], scenario[2] + f = makeFakes() + f.thumbnailService._export_return = "/tmp/t.jpg" + setup(f) + + Client = assert(loadfile(path .. "OllamaClient.lua"))() + c = Client.new(f) + + c.generate({ uuid = "p" }, "hi", nil, false, true, "gemma4:latest", nil) + + assert.are_same( + 1, #f.thumbnailService.cleanup_calls, + string.format("[%s] cleanup called %d times (expected 1)", + name, #f.thumbnailService.cleanup_calls)) + end + end) +end) diff --git a/tests/spec/ollama_client_models_spec.lua b/tests/spec/ollama_client_models_spec.lua new file mode 100644 index 0000000..7848b7b --- /dev/null +++ b/tests/spec/ollama_client_models_spec.lua @@ -0,0 +1,496 @@ +--- ollama_client_models_spec.lua — Unit tests for OllamaClient model-related functions. +--- Covers getBaseUrl, fetchModels, makeModelItems, saveServerAndRefresh with injected fakes. + +local path = PLUGIN_PATH + +-------------------------------------------------------------------------------- +--- Helper: build a fresh set of fake dependencies each test. +-------------------------------------------------------------------------------- +local function makeFakes() + local calls = {} + + local http = { + get_calls = {}, + post_calls = {}, + get_response = nil, + post_response = nil, + } + http.get = function(url) + table.insert(http.get_calls, url) + return http.get_response + end + http.post = function(url, body, headers) + table.insert(http.post_calls, { url = url, body = body, headers = headers }) + return http.post_response + end + + local prefs = { ollamaServerHost = nil } + prefs.prefsForPlugin = function() + return { ollamaServerHost = prefs.ollamaServerHost } + end + + local tasks = { sleep_calls = {} } + tasks.sleep = function(seconds) + table.insert(tasks.sleep_calls, seconds) + end + + -- Jeffrey Friedl's JSON uses colon-style methods (self is first arg). + -- Our fake mirrors that so the SUT calls json:decode / json:encode. + local json = { + decode_calls = {}, + encode_calls = {}, + decode_return = nil, + decode_error = nil, + encode_return = nil, + } + json.decode = function(_, str) + table.insert(json.decode_calls, str) + if json.decode_error then + error(json.decode_error) + end + return json.decode_return + end + json.encode = function(_, tbl) + table.insert(json.encode_calls, tbl) + return json.encode_return or "{}" + end + + local thumbnailService = { + export_response = nil, + encodeBase64_response = nil, + cleanup_calls = {}, + export_calls = {}, + + export = function(_, photo) + table.insert(thumbnailService.export_calls, photo) + return thumbnailService.export_response + end, + + encodeBase64 = function(_, imagePaths) + return thumbnailService.encodeBase64_response + end, + + cleanup = function(imagePath) + table.insert(thumbnailService.cleanup_calls, imagePath) + end, + } + + local promptBuilder = { + buildUserPrompt_calls = {}, + assembleRequestBody_calls = {}, + buildUserPrompt_return = "default prompt", + assembleRequestBody_return = { model = "test", prompt = "p" }, + + buildUserPrompt = function(_, instruction, currentData, useCurrent) + table.insert(promptBuilder.buildUserPrompt_calls, { + instruction = instruction, + currentData = currentData, + useCurrent = useCurrent + }) + return promptBuilder.buildUserPrompt_return + end, + + assembleRequestBody = function(_, userPrompt, model, useSys, override) + table.insert(promptBuilder.assembleRequestBody_calls, { + userPrompt = userPrompt, + model = model, + useSystemPrompt = useSys, + systemPromptOverride = override + }) + return promptBuilder.assembleRequestBody_return + end, + } + + local responseValidator = { + validateAndParse_calls = {}, + validateAndParse_metadata = nil, + validateAndParse_error = nil, + + validateAndParse = function(rawResponse) + table.insert(responseValidator.validateAndParse_calls, rawResponse) + return responseValidator.validateAndParse_metadata, + responseValidator.validateAndParse_error + end, + } + + local logger = { + info_calls = {}, warn_calls = {}, error_calls = {}, + enable = function() end, + } + logger.info = function(msg) table.insert(logger.info_calls, msg) end + logger.warn = function(msg) table.insert(logger.warn_calls, msg) end + logger.error = function(msg) table.insert(logger.error_calls, msg) end + + calls.http = http + calls.prefs = prefs + calls.tasks = tasks + calls.json = json + calls.thumbnailService = thumbnailService + calls.promptBuilder = promptBuilder + calls.responseValidator = responseValidator + calls.logger = logger + + return calls +end + +-------------------------------------------------------------------------------- +describe("OllamaClient — models & configuration", function() + + -------------------------------------------------------------------- + -- getBaseUrl() + -------------------------------------------------------------------- + describe("getBaseUrl()", function() + it("returns default URL when prefs are empty", function() + local f = makeFakes() + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local url = c.getBaseUrl({ ollamaServerHost = nil }) + assert.are_same("http://localhost:11434", url) + end) + + it("returns default URL when prefs is nil", function() + local f = makeFakes() + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + -- validateServerHost treats nil as default + local url = c.getBaseUrl({ ollamaServerHost = nil }) + assert.are_same("http://localhost:11434", url) + end) + + it("uses saved valid host", function() + local f = makeFakes() + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local url = c.getBaseUrl({ ollamaServerHost = "192.168.1.5:9000" }) + assert.are_same("http://192.168.1.5:9000", url) + end) + + it("handles host with http:// prefix", function() + local f = makeFakes() + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local url = c.getBaseUrl({ ollamaServerHost = "http://myhost:11434/" }) + assert.are_same("http://myhost:11434", url) + end) + + it("falls back to default for invalid saved host", function() + local f = makeFakes() + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local url = c.getBaseUrl({ ollamaServerHost = "invalid-no-port" }) + assert.are_same("http://localhost:11434", url) + end) + + it("includes exactly one http:// prefix", function() + local f = makeFakes() + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local url = c.getBaseUrl({ ollamaServerHost = "localhost:11434" }) + local count = 0 + for _ in string.gmatch(url, "http://") do count = count + 1 end + assert.are_same(1, count) + end) + end) + + -------------------------------------------------------------------- + -- makeModelItems() + -------------------------------------------------------------------- + describe("makeModelItems()", function() + it("converts model names to title/value pairs", function() + local f = makeFakes() + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local items = c.makeModelItems({ "gemma4:latest", "llama3" }) + assert.are_same(2, #items) + assert.are_same("gemma4:latest", items[1].title) + assert.are_same("gemma4:latest", items[1].value) + assert.are_same("llama3", items[2].title) + assert.are_same("llama3", items[2].value) + end) + + it("returns empty table for empty input", function() + local f = makeFakes() + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local items = c.makeModelItems({}) + assert.are_same(0, #items) + end) + end) + + -------------------------------------------------------------------- + -- fetchModels() — success paths + -------------------------------------------------------------------- + describe("fetchModels() — success", function() + it("sends GET to /api/tags", function() + local f = makeFakes() + f.http.get_response = '{"models":[{"name":"gemma4:latest"}]}' + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + + local models = c.fetchModels({ ollamaServerHost = "localhost:11434" }) + + assert.are_same(1, #f.http.get_calls) + assert.are_same("http://localhost:11434/api/tags", f.http.get_calls[1]) + end) + + it("parses models using the name field", function() + local f = makeFakes() + f.json.decode_return = { + models = { { name = "gemma4:latest" }, { name = "llama3" } } + } + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + f.http.get_response = "{}" + local models = c.fetchModels({ ollamaServerHost = nil }) + assert.are_same(2, #models) + assert.are_same("gemma4:latest", models[1]) + assert.are_same("llama3", models[2]) + end) + + it("parses models using the legacy model field", function() + local f = makeFakes() + f.json.decode_return = { + models = { { model = "oldmodel" } } + } + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + f.http.get_response = "{}" + local models = c.fetchModels({ ollamaServerHost = nil }) + assert.are_same(1, #models) + assert.are_same("oldmodel", models[1]) + end) + + it("ignores entries with missing or empty names", function() + local f = makeFakes() + f.json.decode_return = { + models = { + { name = "good" }, + { name = "" }, + { name = nil }, + {}, -- no name field + } + } + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + f.http.get_response = "{}" + local models = c.fetchModels({ ollamaServerHost = nil }) + assert.are_same(1, #models) + assert.are_same("good", models[1]) + end) + + it("preserves response order", function() + local f = makeFakes() + f.json.decode_return = { + models = { + { name = "first" }, + { name = "second" }, + { name = "third" }, + } + } + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + f.http.get_response = "{}" + local models = c.fetchModels({ ollamaServerHost = nil }) + assert.are_same({ "first", "second", "third" }, models) + end) + end) + + -------------------------------------------------------------------- + -- fetchModels() — fallback paths + -------------------------------------------------------------------- + describe("fetchModels() — fallback to default", function() + it("returns default model on no HTTP response", function() + local f = makeFakes() + f.http.get_response = nil + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local models = c.fetchModels({ ollamaServerHost = nil }) + assert.are_same({ "gemma4:latest" }, models) + end) + + it("returns default model on malformed JSON", function() + local f = makeFakes() + f.json.decode_return = "not a table" -- string, not table + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + f.http.get_response = "garbage" + local models = c.fetchModels({ ollamaServerHost = nil }) + assert.are_same({ "gemma4:latest" }, models) + end) + + it("returns default model when models key is missing", function() + local f = makeFakes() + f.json.decode_return = { other = "data" } + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + f.http.get_response = '{"other":"data"}' + local models = c.fetchModels({ ollamaServerHost = nil }) + assert.are_same({ "gemma4:latest" }, models) + end) + + it("returns default model for empty model list", function() + local f = makeFakes() + f.json.decode_return = { models = {} } + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + f.http.get_response = '{"models":[]}' + local models = c.fetchModels({ ollamaServerHost = nil }) + assert.are_same({ "gemma4:latest" }, models) + end) + + it("handles JSON decoder exception without crashing", function() + local f = makeFakes() + f.json.decode_error = "attempt to index nil" + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + f.http.get_response = "bad json" + -- Should not throw; should return default + local models = c.fetchModels({ ollamaServerHost = nil }) + assert.are_same({ "gemma4:latest" }, models) + end) + end) + + -------------------------------------------------------------------- + -- saveServerAndRefresh() — invalid input + -------------------------------------------------------------------- + describe("saveServerAndRefresh() — validation", function() + it("returns failure for invalid server input", function() + local f = makeFakes() + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local ok, msg = c.saveServerAndRefresh({ serverHost = "noport" }, {}) + assert.is_false(ok) + assert.is_not_nil(msg) + end) + + it("does not modify prefs for invalid input", function() + local f = makeFakes() + f.prefs.ollamaServerHost = "original:1234" + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local prefs = { ollamaServerHost = "original:1234" } + c.saveServerAndRefresh({ serverHost = "bad" }, prefs) + -- prefs should be unchanged + assert.are_same("original:1234", prefs.ollamaServerHost) + end) + end) + + -------------------------------------------------------------------- + -- saveServerAndRefresh() — valid input + -------------------------------------------------------------------- + describe("saveServerAndRefresh() — success path", function() + it("normalizes and saves valid host", function() + local f = makeFakes() + f.json.decode_return = { models = { { name = "m1" } } } + f.http.get_response = '{"models":[{"name":"m1"}]}' + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local prefs = {} + local props = { serverHost = "http://myhost:9000/" } + + local ok, msg = c.saveServerAndRefresh(props, prefs) + + assert.is_true(ok) + assert.are_same("myhost:9000", prefs.ollamaServerHost) + end) + + it("builds model items from fetched models", function() + local f = makeFakes() + f.json.decode_return = { + models = { { name = "a" }, { name = "b" } } + } + f.http.get_response = '{"models":[{"name":"a"},{"name":"b"}]}' + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local prefs = {} + local props = { serverHost = "localhost:11434" } + + c.saveServerAndRefresh(props, prefs) + + assert.are_same(2, #props.modelItems) + assert.are_same("a", props.modelItems[1].title) + assert.are_same("a", props.modelItems[1].value) + assert.are_same("b", props.modelItems[2].title) + end) + + it("preserves selected model when still available", function() + local f = makeFakes() + f.json.decode_return = { + models = { { name = "a" }, { name = "b" } } + } + f.http.get_response = '{}' + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local prefs = {} + local props = { serverHost = "localhost:11434", selectedModel = "b" } + + c.saveServerAndRefresh(props, prefs) + + assert.are_same("b", props.selectedModel) + end) + + it("selects first model when old selection unavailable", function() + local f = makeFakes() + f.json.decode_return = { + models = { { name = "new1" }, { name = "new2" } } + } + f.http.get_response = '{}' + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local prefs = {} + local props = { serverHost = "localhost:11434", selectedModel = "old" } + + c.saveServerAndRefresh(props, prefs) + + assert.are_same("new1", props.selectedModel) + end) + + it("reports correct count in success message", function() + local f = makeFakes() + f.json.decode_return = { + models = { { name = "x" }, { name = "y" }, { name = "z" } } + } + f.http.get_response = '{}' + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local prefs = {} + local props = { serverHost = "localhost:11434" } + + local ok, msg = c.saveServerAndRefresh(props, prefs) + + assert.is_true(ok) + assert.are_same("Loaded 3 model(s)", msg) + end) + + it("model items contain title and value pairs", function() + local f = makeFakes() + f.json.decode_return = { models = { { name = "only" } } } + f.http.get_response = '{}' + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local prefs = {} + local props = { serverHost = "localhost:11434" } + + c.saveServerAndRefresh(props, prefs) + + assert.are_same("only", props.modelItems[1].title) + assert.are_same("only", props.modelItems[1].value) + end) + + it("passes updated prefs into model discovery", function() + local f = makeFakes() + f.http.get_response = '{}' + -- The get_calls URL reveals which host was used + local Client = assert(loadfile(path .. "OllamaClient.lua"))() + local c = Client.new(f) + local prefs = {} + local props = { serverHost = "custom:7777" } + + c.saveServerAndRefresh(props, prefs) + + -- fetchModels should be called with updated prefs; the URL shows it + assert.are_same(1, #f.http.get_calls) + assert.are_same("http://custom:7777/api/tags", f.http.get_calls[1]) + end) + end) +end) diff --git a/tests/spec/ollama_client_validate_spec.lua b/tests/spec/ollama_client_validate_spec.lua new file mode 100644 index 0000000..c110366 --- /dev/null +++ b/tests/spec/ollama_client_validate_spec.lua @@ -0,0 +1,119 @@ +--- ollama_client_validate_spec.lua — Unit tests for OllamaClient.validateServerHost +--- Tests input normalization, host:port validation, and error cases. + +local path = PLUGIN_PATH + +describe("OllamaClient.validateServerHost", function() + local client + + before_each(function() + client = assert(loadfile(path .. "OllamaClient.lua"))() + end) + + -------------------------------------------------------------------- + -- Defaults + -------------------------------------------------------------------- + describe("defaults", function() + it("accepts nil and returns default host", function() + local ok, result = client.validateServerHost(nil) + assert.is_true(ok) + assert.are_same("localhost:11434", result) + end) + + it("accepts empty string and returns default host", function() + local ok, result = client.validateServerHost("") + assert.is_true(ok) + assert.are_same("localhost:11434", result) + end) + end) + + -------------------------------------------------------------------- + -- Valid inputs + -------------------------------------------------------------------- + describe("valid inputs", function() + it("accepts localhost with default port", function() + local ok, result = client.validateServerHost("localhost:11434") + assert.is_true(ok) + assert.are_same("localhost:11434", result) + end) + + it("accepts IP address with port", function() + local ok, result = client.validateServerHost("192.168.1.10:11434") + assert.is_true(ok) + assert.are_same("192.168.1.10:11434", result) + end) + + it("accepts domain name with port", function() + local ok, result = client.validateServerHost("ollama.example.com:9000") + assert.is_true(ok) + assert.are_same("ollama.example.com:9000", result) + end) + + it("strips http:// scheme", function() + local ok, result = client.validateServerHost("http://localhost:11434") + assert.is_true(ok) + assert.are_same("localhost:11434", result) + end) + + it("strips https:// scheme", function() + local ok, result = client.validateServerHost("https://myserver:8080") + assert.is_true(ok) + assert.are_same("myserver:8080", result) + end) + + it("strips trailing slashes", function() + local ok, result = client.validateServerHost("localhost:11434///") + assert.is_true(ok) + assert.are_same("localhost:11434", result) + end) + + it("handles scheme + trailing slash combo", function() + local ok, result = client.validateServerHost("http://host:8080/") + assert.is_true(ok) + assert.are_same("host:8080", result) + end) + + it("accepts port edge values", function() + local ok1, _ = client.validateServerHost("h:1") + assert.is_true(ok1) + local ok2, _ = client.validateServerHost("h:65535") + assert.is_true(ok2) + end) + end) + + -------------------------------------------------------------------- + -- Invalid inputs + -------------------------------------------------------------------- + describe("invalid inputs", function() + it("rejects missing port", function() + local ok, err = client.validateServerHost("localhost") + assert.is_false(ok) + assert.is_not_nil(err) + end) + + it("rejects non-numeric port", function() + local ok, err = client.validateServerHost("host:abc") + assert.is_false(ok) + end) + + it("rejects port > 65535", function() + local ok, err = client.validateServerHost("h:99999") + assert.is_false(ok) + end) + + it("rejects port 0", function() + local ok, err = client.validateServerHost("h:0") + assert.is_false(ok) + end) + + it("rejects hostname with invalid characters", function() + local ok, err = client.validateServerHost("host name:80") + assert.is_false(ok) + end) + + it("rejects bare scheme without host", function() + local ok, err = client.validateServerHost("http://") + assert.is_false(ok) + end) + end) +end) diff --git a/tests/spec/prompt_builder_spec.lua b/tests/spec/prompt_builder_spec.lua new file mode 100644 index 0000000..6ae5ed0 --- /dev/null +++ b/tests/spec/prompt_builder_spec.lua @@ -0,0 +1,139 @@ +--- prompt_builder_spec.lua — Unit tests for PromptBuilder.lua +--- Tests user-prompt assembly, request-body construction, and system prompts. + +local path = PLUGIN_PATH + +describe("PromptBuilder", function() + local prompt + + before_each(function() + prompt = assert(loadfile(path .. "PromptBuilder.lua"))() + end) + + -------------------------------------------------------------------- + -- System prompts exist and are non-empty + -------------------------------------------------------------------- + describe("system prompts", function() + it("has a default system prompt", function() + assert.is_true(#prompt.defaultSystemPrompt > 100) + assert.is_not_nil(string.find(prompt.defaultSystemPrompt, "title")) + assert.is_not_nil(string.find(prompt.defaultSystemPrompt, "caption")) + assert.is_not_nil(string.find(prompt.defaultSystemPrompt, "keywords")) + end) + + it("has a single-photo system prompt", function() + assert.is_true(#prompt.singlePhotoSystemPrompt > 50) + end) + + it("uses different prompts for batch vs single-photo modes", function() + assert.is_not.equal( + prompt.defaultSystemPrompt, + prompt.singlePhotoSystemPrompt + ) + end) + end) + + -------------------------------------------------------------------- + -- buildUserPrompt + -------------------------------------------------------------------- + describe("buildUserPrompt", function() + it("returns instruction when no current data", function() + local got = prompt.buildUserPrompt("Caption this photo", nil, false) + assert.are_same("Caption this photo", got) + end) + + it("returns instruction when useCurrentData is false", function() + local got = prompt.buildUserPrompt( + "Go", { title = "T", caption = "C" }, false + ) + assert.are_same("Go", got) + end) + + it("prepends title and caption when useCurrentData is true", function() + local got = prompt.buildUserPrompt( + "Update metadata", + { title = "Sunset", caption = "Golden hour beach" }, + true + ) + assert.are_same( + "Title: Sunset Caption: Golden hour beach Update metadata", + got + ) + end) + + it("handles empty strings for title/caption", function() + local got = prompt.buildUserPrompt( + "Do it", + { title = "", caption = "" }, + true + ) + assert.are_same("Title: Caption: Do it", got) + end) + + it("escapes double quotes in title and caption", function() + local got = prompt.buildUserPrompt( + "Go", + { title = 'He said "hi"', caption = 'It\'s "great"' }, + true + ) + assert.are_same( + [[Title: He said \"hi\" Caption: It's \"great\" Go]], + got + ) + end) + + it("handles nil title or caption gracefully", function() + local got = prompt.buildUserPrompt( + "Go", + { title = nil, caption = "C" }, + true + ) + assert.are_same("Title: Caption: C Go", got) + end) + end) + + -------------------------------------------------------------------- + -- assembleRequestBody + -------------------------------------------------------------------- + describe("assembleRequestBody", function() + it("includes model, prompt, format, and stream fields", function() + local body = prompt.assembleRequestBody( + "Caption this", "gemma4:latest", true, nil + ) + assert.are_same("gemma4:latest", body.model) + assert.are_same("Caption this", body.prompt) + assert.are_same("json", body.format) + assert.is_false(body.stream) + end) + + it("includes system prompt when useSystemPrompt is true", function() + local body = prompt.assembleRequestBody( + "Go", "llama3", true, nil + ) + assert.is_not_nil(body.system) + assert.is_same(prompt.defaultSystemPrompt, body.system) + end) + + it("omits system prompt when useSystemPrompt is false", function() + local body = prompt.assembleRequestBody( + "Go", "llama3", false, nil + ) + assert.is_nil(body.system) + end) + + it("uses override system prompt when provided", function() + local custom = "You are a pirate." + local body = prompt.assembleRequestBody( + "Arrr", "gemma4:latest", true, custom + ) + assert.are_same(custom, body.system) + end) + + it("does not include images field", function() + local body = prompt.assembleRequestBody( + "Go", "model", true, nil + ) + assert.is_nil(body.images) + end) + end) +end) diff --git a/tests/spec/response_validator_spec.lua b/tests/spec/response_validator_spec.lua new file mode 100644 index 0000000..3ee4ac7 --- /dev/null +++ b/tests/spec/response_validator_spec.lua @@ -0,0 +1,146 @@ +--- response_validator_spec.lua — Unit tests for ResponseValidator.lua +--- Tests fence stripping, envelope parsing, schema validation, and the full pipeline. + +local path = PLUGIN_PATH + +describe("ResponseValidator", function() + local validate + + before_each(function() + -- Reload fresh each time so cached JSON state doesn't leak between tests + validate = assert(loadfile(path .. "ResponseValidator.lua"))() + end) + + -------------------------------------------------------------------- + -- stripMarkdownFences + -------------------------------------------------------------------- + describe("stripMarkdownFences", function() + it("returns bare JSON unchanged", function() + local json = '{"title":"A","caption":"B","keywords":["c"]}' + assert.are.same(json, validate.stripMarkdownFences(json)) + end) + + it("strips ```json ... ``` fences", function() + local raw = "```json\n{\"title\":\"A\"}\n```" + local got = validate.stripMarkdownFences(raw) + assert.are_same('{"title":"A"}', got) + end) + + it("strips bare ``` fences without language tag", function() + local raw = "```\n{}\n```" + local got = validate.stripMarkdownFences(raw) + assert.are_same('{}', got) + end) + + it("handles leading/trailing whitespace around fences", function() + local raw = " ```json\n{\"x\":1}\n``` " + local got = validate.stripMarkdownFences(raw) + assert.are_same('{"x":1}', got) + end) + end) + + -------------------------------------------------------------------- + -- parseApiEnvelope + -------------------------------------------------------------------- + describe("parseApiEnvelope", function() + it("decodes a valid envelope with response field", function() + local envelope = '{"response":"hello world"}' + local data, err = validate.parseApiEnvelope(envelope) + assert.falsy(err) + assert.are_same("hello world", data.response) + end) + + it("rejects malformed JSON", function() + local data, err = validate.parseApiEnvelope("{bad json") + assert.falsy(data) + assert.is_not_nil(err) + end) + + it("rejects an envelope missing the response field", function() + local data, err = validate.parseApiEnvelope('{"foo":1}') + assert.falsy(data) + assert.is_not_nil(err) + end) + end) + + -------------------------------------------------------------------- + -- checkMetadataSchema + -------------------------------------------------------------------- + describe("checkMetadataSchema", function() + it("accepts valid title, caption, keywords", function() + local ok, err = validate.checkMetadataSchema({ + title = "Sunset over ocean", + caption = "A beautiful scene", + keywords = { "ocean", "sunset" }, + }) + assert.is_true(ok) + assert.falsy(err) + end) + + it("rejects missing title", function() + local ok, err = validate.checkMetadataSchema({ + caption = "ok", keywords = { "a" } + }) + assert.is_false(ok) + assert.is_not_nil(err) + end) + + it("rejects empty keyword string in array", function() + local ok, err = validate.checkMetadataSchema({ + title = "T", caption = "C", keywords = { "", "b" } + }) + assert.is_false(ok) + assert.is_not_nil(err) + end) + + it("rejects whitespace-only keyword", function() + local ok, err = validate.checkMetadataSchema({ + title = "T", caption = "C", keywords = { " ", "b" } + }) + assert.is_false(ok) + end) + end) + + -------------------------------------------------------------------- + -- validateAndParse (full pipeline) + -------------------------------------------------------------------- + describe("validateAndParse", function() + local function makeEnvelope(innerJson) + -- Build a proper outer envelope string. The JSON.lua decoder is + --- available via _PLUGIN + LrPathUtils, but simpler to inline. + return '{"response":' .. innerJson .. '}' + end + + it("returns parsed metadata for clean JSON response", function() + local inner = [[{"title":"Beach sunset","caption":"Waves roll in","keywords":["beach","sunset"]}]] + -- Need to escape quotes for embedding inside the envelope JSON string + local envelope = '{"response":' .. '"' .. string.gsub(inner, '"', '\\"') .. '"}' + local metadata, err = validate.validateAndParse(envelope) + assert.falsy(err) + assert.are_same("Beach sunset", metadata.title) + end) + + it("handles markdown-fenced inner JSON", function() + -- Build: {"response": "```json\n{...}\n```"} + local fenced = '```json\n{"title":"X","caption":"Y","keywords":["z"]}\n```' + local escaped = string.gsub(fenced, '"', '\\"') + local envelope = '{"response":"' .. escaped .. '"}' + local metadata, err = validate.validateAndParse(envelope) + assert.falsy(err) + assert.are_same("X", metadata.title) + end) + + it("returns error for non-json response body", function() + local envelope = '{"response":"just plain text"}' + local metadata, err = validate.validateAndParse(envelope) + assert.falsy(metadata) + assert.is_not_nil(err) + end) + + it("returns error when envelope is invalid", function() + local metadata, err = validate.validateAndParse("not json at all") + assert.falsy(metadata) + assert.is_not_nil(err) + end) + end) +end) diff --git a/tests/spec/single_photo_controller_spec.lua b/tests/spec/single_photo_controller_spec.lua new file mode 100644 index 0000000..dcd36ab --- /dev/null +++ b/tests/spec/single_photo_controller_spec.lua @@ -0,0 +1,532 @@ +--- single_photo_controller_spec.lua — Unit tests for SinglePhotoController. +--- Covers initial state, generate success/failure, save server, save metadata. + +local path = PLUGIN_PATH + +-------------------------------------------------------------------------------- +--- Helper: build fresh fake dependencies each test. +-------------------------------------------------------------------------------- +local function makeFakes() + -- Consolidated fake: serves as photo, common, and client simultaneously. + -- Tests pass the same table for multiple dep roles to keep things simple. + local f = {} + + -- photo fake with getFormattedMetadata + f.metadata = { title = "", caption = "" } + f.getFormattedMetadata = function(_, key) + return f.metadata[key] or "" + end + + -- common fake (parseKeywordCsv + getLlmKeywordsFromPhoto) + f.parseKeywordCsv = function(csv) + if not csv or csv == "" then return {} end + local result = {} + for kw in string.gmatch(csv, "([^,]+)") do + local trimmed = kw:match("^%s*(.-)%s*$") + if trimmed ~= "" then table.insert(result, trimmed) end + end + return result + end + f.getLlmKeywordsFromPhoto = function() + return f.keywords or {} + end + + -- client fake constants + f.defaultModel = "gemma4:latest" + f.defaultServerHost = "localhost:11434" + f.fetchModels_called = false + f.fetchModels_models = { "gemma4:latest", "llama3" } + f.generate_calls = {} + f.generate_response = nil + f.generate_error = nil + f.makeModelItems = function(names) + local items = {} + for _, n in ipairs(names) do + table.insert(items, { title = n, value = n }) + end + return items + end + + -- saveServerAndRefresh tracking + f.saveServerAndRefresh_calls = {} + f.saveServerAndRefresh_ok = true + f.saveServerAndRefresh_msg = "Loaded 2 model(s)" + + f.validateServerHost_called = false + f.validateServerHost_ok = true + f.validateServerHost_result = "localhost:11434" + + return f +end + +local function buildClient(client) + client.fetchModels = function() + client.fetchModels_called = true + return client.fetchModels_models + end + client.generate = function(photo, instruction, currentData, useCurrent, + useSystem, model, systemOverride) + table.insert(client.generate_calls, { + photo = photo, instruction = instruction, + currentData = currentData, useCurrent = useCurrent, + useSystem = useSystem, model = model, systemOverride = systemOverride + }) + return client.generate_response, client.generate_error + end + client.saveServerAndRefresh = function(state, prefs) + table.insert(client.saveServerAndRefresh_calls, { state, prefs }) + return client.saveServerAndRefresh_ok, + client.saveServerAndRefresh_msg + end + client.validateServerHost = function(input) + client.validateServerHost_called = true + return client.validateServerHost_ok, + client.validateServerHost_result + end + return client +end + +-------------------------------------------------------------------------------- +describe("SinglePhotoController", function() + local Controller + + before_each(function() + Controller = assert(loadfile(path .. "SinglePhotoController.lua"))() + end) + + --------------------------------------------------------------- + -- A. INITIAL STATE + --------------------------------------------------------------- + describe("initial state", function() + + it("empty title/caption → useCurrentData = false", function() + local f = makeFakes() + buildClient(f) + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, + catalog = {} + }) + local state = ctrl.initState() + assert.is_false(state.useCurrentData) + end) + + it("existing title → useCurrentData = true", function() + local f = makeFakes() + buildClient(f) + f.metadata.title = "Sunset" + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + assert.is_true(state.useCurrentData) + end) + + it("existing caption → useCurrentData = true", function() + local f = makeFakes() + buildClient(f) + f.metadata.caption = "A beach" + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + assert.is_true(state.useCurrentData) + end) + + it("joins existing LLM keywords with commas", function() + local f = makeFakes() + buildClient(f) + f.keywords = { "sunset", "beach" } + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + assert.are_same("sunset, beach", state.keywords) + end) + + it("uses saved server host from prefs", function() + local f = makeFakes() + buildClient(f) + local prefs = { ollamaServerHost = "myhost:9000" } + local ctrl = Controller.new({ + client = f, common = f, prefs = prefs, photo = f, catalog = {} + }) + local state = ctrl.initState() + assert.are_same("myhost:9000", state.serverHost) + end) + + it("uses default server host when prefs missing", function() + local f = makeFakes() + buildClient(f) + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + assert.are_same("localhost:11434", state.serverHost) + end) + + it("fetches available models", function() + local f = makeFakes() + buildClient(f) + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + ctrl.initState() + assert.is_true(f.fetchModels_called) + end) + + it("builds model items from fetched models", function() + local f = makeFakes() + buildClient(f) + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + assert.are_same(2, #state.modelItems) + assert.are_same("gemma4:latest", state.modelItems[1].title) + assert.are_same("llama3", state.modelItems[2].title) + end) + + it("selects first model by default", function() + local f = makeFakes() + buildClient(f) + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + assert.are_same("gemma4:latest", state.selectedModel) + end) + + it("sets default prompt and system-prompt settings", function() + local f = makeFakes() + buildClient(f) + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + assert.are_same("Caption this photo", state.prompt) + assert.is_true(state.useSystemPrompt) + end) + + end) + + --------------------------------------------------------------- + -- B. GENERATE SUCCESS + --------------------------------------------------------------- + describe("generate — success", function() + + it("calls API with all expected arguments", function() + local f = makeFakes() + buildClient(f) + f.generate_response = { title = "T", caption = "C", keywords = { "k" } } + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {}, + promptBuilder = { singlePhotoSystemPrompt = "sys" } + }) + local state = ctrl.initState() + ctrl.generate(state) + assert.are_same(1, #f.generate_calls) + end) + + it("sets status to working before API call", function() + local f = makeFakes() + buildClient(f) + f.generate_response = { title = "T", caption = "C", keywords = {} } + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + + -- Override generate to check status at call time + local capturedStatus = nil + f.generate = function(...) + capturedStatus = state.statusKind + return f.generate_response + end + ctrl.generate(state) + assert.are_same("working", capturedStatus) + end) + + it("updates title, caption from response", function() + local f = makeFakes() + buildClient(f) + f.generate_response = { title = "New T", caption = "New C", keywords = {} } + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + ctrl.generate(state) + assert.are_same("New T", state.title) + assert.are_same("New C", state.caption) + end) + + it("joins keyword array into comma-separated string", function() + local f = makeFakes() + buildClient(f) + f.generate_response = { title = "T", caption = "C", keywords = { "a", "b" } } + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + ctrl.generate(state) + assert.are_same("a, b", state.keywords) + end) + + it("retains response data in state", function() + local f = makeFakes() + buildClient(f) + f.generate_response = { title = "T", caption = "C", keywords = {} } + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + ctrl.generate(state) + assert.is_not_nil(state.response) + end) + + it("returns to ready/success status", function() + local f = makeFakes() + buildClient(f) + f.generate_response = { title = "T", caption = "C", keywords = {} } + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + ctrl.generate(state) + assert.are_same("success", state.statusKind) + end) + + it("returns a successful result table", function() + local f = makeFakes() + buildClient(f) + f.generate_response = { title = "T", caption = "C", keywords = {} } + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + local result = ctrl.generate(state) + assert.is_true(result.ok) + end) + + end) + + --------------------------------------------------------------- + -- C. GENERATE FAILURE + --------------------------------------------------------------- + describe("generate — failure", function() + + it("preserves existing editable fields on error", function() + local f = makeFakes() + buildClient(f) + f.generate_error = "network failure" + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + -- Mutate fields before generate to verify preservation + state.title = "Existing Title" + + ctrl.generate(state) + assert.are_same("Existing Title", state.title) + end) + + it("sets status to error", function() + local f = makeFakes() + buildClient(f) + f.generate_error = "boom" + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + ctrl.generate(state) + assert.are_same("error", state.statusKind) + assert.truthy(string.find(state.status, "Error")) + end) + + it("failure result contains the error", function() + local f = makeFakes() + buildClient(f) + f.generate_error = "no network" + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + local result = ctrl.generate(state) + assert.is_false(result.ok) + end) + + end) + + --------------------------------------------------------------- + -- D. SAVE SERVER SUCCESS + --------------------------------------------------------------- + describe("saveServer — success", function() + + it("passes state and prefs to saveServerAndRefresh", function() + local f = makeFakes() + buildClient(f) + local prefs = {} + local ctrl = Controller.new({ + client = f, common = f, prefs = prefs, photo = f, catalog = {} + }) + local state = ctrl.initState() + ctrl.saveServer(state) + assert.are_same(1, #f.saveServerAndRefresh_calls) + end) + + it("sets success status and message", function() + local f = makeFakes() + buildClient(f) + f.saveServerAndRefresh_msg = "Loaded 3 model(s)" + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + ctrl.saveServer(state) + assert.are_same("success", state.statusKind) + assert.are_same(f.saveServerAndRefresh_msg, state.status) + end) + + end) + + --------------------------------------------------------------- + -- E. SAVE SERVER FAILURE + --------------------------------------------------------------- + describe("saveServer — failure", function() + + it("sets error status", function() + local f = makeFakes() + buildClient(f) + f.saveServerAndRefresh_ok = false + f.saveServerAndRefresh_msg = "bad server" + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + ctrl.saveServer(state) + assert.are_same("error", state.statusKind) + end) + + it("returns failure result", function() + local f = makeFakes() + buildClient(f) + f.saveServerAndRefresh_ok = false + f.saveServerAndRefresh_msg = "bad server" + local ctrl = Controller.new({ + client = f, common = f, prefs = {}, photo = f, catalog = {} + }) + local state = ctrl.initState() + local result = ctrl.saveServer(state) + assert.is_false(result.ok) + end) + + end) + + --------------------------------------------------------------- + -- F. SAVE METADATA SUCCESS + --------------------------------------------------------------- + describe("saveMetadata — success", function() + + it("validates server host and saves to prefs", function() + local f = makeFakes() + buildClient(f) + f.validateServerHost_ok = true + f.validateServerHost_result = "localhost:11434" + local prefs = {} + local written = nil + local ctrl = Controller.new({ + client = f, common = f, prefs = prefs, photo = f, catalog = {}, + writeMetadata = function(catalog, photo, meta) + written = meta + end, + }) + local state = ctrl.initState() + ctrl.saveMetadata(state) + assert.are_same("localhost:11434", prefs.ollamaServerHost) + end) + + it("parses keywords through parseKeywordCsv", function() + local f = makeFakes() + buildClient(f) + f.validateServerHost_ok = true + f.validateServerHost_result = "localhost:11434" + local prefs = {} + local written = nil + local ctrl = Controller.new({ + client = f, common = f, prefs = prefs, photo = f, catalog = {}, + writeMetadata = function(catalog, photo, meta) + written = meta + end, + }) + local state = ctrl.initState() + state.keywords = "sunset,, beach, ,portrait" + ctrl.saveMetadata(state) + assert.are_same({ "sunset", "beach", "portrait" }, written.keywords) + end) + + it("passes title and caption to write adapter", function() + local f = makeFakes() + buildClient(f) + f.validateServerHost_ok = true + f.validateServerHost_result = "localhost:11434" + local prefs = {} + local written = nil + local ctrl = Controller.new({ + client = f, common = f, prefs = prefs, photo = f, catalog = {}, + writeMetadata = function(catalog, photo, meta) + written = meta + end, + }) + local state = ctrl.initState() + state.title = "T" + state.caption = "C" + ctrl.saveMetadata(state) + assert.are_same("T", written.title) + assert.are_same("C", written.caption) + end) + + it("returns success result", function() + local f = makeFakes() + buildClient(f) + f.validateServerHost_ok = true + f.validateServerHost_result = "localhost:11434" + local prefs = {} + local ctrl = Controller.new({ + client = f, common = f, prefs = prefs, photo = f, catalog = {}, + writeMetadata = function() end, + }) + local state = ctrl.initState() + local result = ctrl.saveMetadata(state) + assert.is_true(result.ok) + end) + + end) + + --------------------------------------------------------------- + -- G. SAVE METADATA VALIDATION FAILURE + --------------------------------------------------------------- + describe("saveMetadata — validation failure", function() + + it("returns structured validation error for invalid server", function() + local f = makeFakes() + buildClient(f) + f.validateServerHost_ok = false + f.validateServerHost_result = "must be host:port" + local prefs = {} + local writeCalled = false + local ctrl = Controller.new({ + client = f, common = f, prefs = prefs, photo = f, catalog = {}, + writeMetadata = function() writeCalled = true end, + }) + local state = ctrl.initState() + state.serverHost = "bad" + local result = ctrl.saveMetadata(state) + + assert.is_false(result.ok) + assert.are_same("invalid_server", result.errorKind) + assert.is_not_nil(result.message) + assert.is_false(writeCalled) + end) + + end) + +end) diff --git a/tests/spec/thumbnail_service_spec.lua b/tests/spec/thumbnail_service_spec.lua new file mode 100644 index 0000000..c12b0c2 --- /dev/null +++ b/tests/spec/thumbnail_service_spec.lua @@ -0,0 +1,335 @@ +--- thumbnail_service_spec.lua — Direct unit tests for ThumbnailService module. +--- Covers cleanup(), encodeBase64(), and export() with mocked SDK dependencies. + +local path = PLUGIN_PATH + +-------------------------------------------------------------------------------- +--- Helper: load ThumbnailService with configurable fake SDK modules. +--- Overrides import() temporarily so that the module captures our fakes as +--- its local LrFileUtils, LrPathUtils, LrStringUtils, LrLogger references. +-------------------------------------------------------------------------------- +local function loadWithFakes(fakeLrFileUtils, fakeLrPathUtils, fakeLrStringUtils) + local oldImport = _G.import + + -- LrLogger is a constructor: LrLogger('name') → logger instance. + local loggerFactory = setmetatable({}, { + __call = function(_, _) + return { + info = function() end, + warn = function() end, + error = function() end, + enable = function() end, + } + end, + }) + + _G.import = function(className) + if className == "LrFileUtils" then return fakeLrFileUtils end + if className == "LrPathUtils" then return fakeLrPathUtils end + if className == "LrStringUtils" then return fakeLrStringUtils end + if className == "LrLogger" then return loggerFactory end + return oldImport(className) + end + + local thumbnail = dofile(path .. "ThumbnailService.lua") + + -- Restore immediately so other tests are unaffected. + _G.import = oldImport + return thumbnail +end + +-------------------------------------------------------------------------------- +describe("ThumbnailService — cleanup()", function() + + it("deletes an existing file", function() + local fakeFs = { delete_calls = {} } + fakeFs.exists = function(p) return true end + fakeFs.delete = function(p) table.insert(fakeFs.delete_calls, p) end + + local ts = loadWithFakes(fakeFs, {}, {}) + ts.cleanup("/tmp/thumb.jpg") + + assert.are_same(1, #fakeFs.delete_calls) + assert.are_same("/tmp/thumb.jpg", fakeFs.delete_calls[1]) + end) + + it("does nothing when the file does not exist", function() + local fakeFs = { delete_calls = {} } + fakeFs.exists = function(_) return false end + fakeFs.delete = function(p) table.insert(fakeFs.delete_calls, p) end + + local ts = loadWithFakes(fakeFs, {}, {}) + ts.cleanup("/tmp/gone.jpg") + + assert.are_same(0, #fakeFs.delete_calls) + end) + + it("does nothing when given a nil path", function() + local fakeFs = { delete_calls = {} } + fakeFs.exists = function(p) return true end + fakeFs.delete = function(p) table.insert(fakeFs.delete_calls, p) end + + local ts = loadWithFakes(fakeFs, {}, {}) + ts.cleanup(nil) + + assert.are_same(0, #fakeFs.delete_calls) + end) + + it("passes the exact path to deletion", function() + local fakeFs = { delete_calls = {} } + local expectedPath = "/var/folders/xx/unique_name.jpg" + fakeFs.exists = function(p) return p == expectedPath end + fakeFs.delete = function(p) table.insert(fakeFs.delete_calls, p) end + + local ts = loadWithFakes(fakeFs, {}, {}) + ts.cleanup(expectedPath) + + assert.are_same(1, #fakeFs.delete_calls) + assert.are_same(expectedPath, fakeFs.delete_calls[1]) + end) +end) + +-------------------------------------------------------------------------------- +describe("ThumbnailService — encodeBase64()", function() + + it("returns nil when the file does not exist", function() + local fakeFs = {} + fakeFs.exists = function(_) return false end + + local ts = loadWithFakes(fakeFs, {}, {}) + local result = ts.encodeBase64("/tmp/nope.jpg") + + assert.is_nil(result) + end) + + it("returns nil when io.open fails for a file that exists", function() + -- Mock exists=true but the path doesn't exist on disk, so io.open fails. + local fakeFs = {} + fakeFs.exists = function(_) return true end + + local ts = loadWithFakes(fakeFs, {}, {}) + local result = ts.encodeBase64("/nonexistent/doesnotexist.jpg") + + assert.is_nil(result) + end) + + it("returns nil for an empty file", function() + -- Create a real empty temp file. + local tmpname = path .. "empty_test_tmp_" .. os.time() .. ".jpg" + local f = io.open(tmpname, "wb") + if f then f:close() end + + local fakeFs = {} + fakeFs.exists = function(p) return p == tmpname end + + local fakeStrUtils = {} + fakeStrUtils.encodeBase64 = function(data) return "encoded" end + + local ts = loadWithFakes(fakeFs, {}, fakeStrUtils) + local result = ts.encodeBase64(tmpname) + + assert.is_nil(result) + os.remove(tmpname) + end) + + it("returns encoded value for a valid file", function() + -- Create a real temp file with known content. + local tmpname = path .. "valid_test_tmp_" .. os.time() .. ".jpg" + local f = io.open(tmpname, "wb") + if f then f:write("\xFF\xD8\xFF\xE0"); f:close() end + + local fakeFs = {} + fakeFs.exists = function(p) return p == tmpname end + + local encodedBytes = nil + local fakeStrUtils = {} + fakeStrUtils.encodeBase64 = function(data) + encodedBytes = data + return "base64encoded" + end + + local ts = loadWithFakes(fakeFs, {}, fakeStrUtils) + local result = ts.encodeBase64(tmpname) + + assert.are_same("base64encoded", result) + assert.are_same("\xFF\xD8\xFF\xE0", encodedBytes) + os.remove(tmpname) + end) + + it("returns nil when the Base64 encoder returns nil", function() + local tmpname = path .. "encoder_fail_tmp_" .. os.time() .. ".jpg" + local f = io.open(tmpname, "wb") + if f then f:write("data"); f:close() end + + local fakeFs = {} + fakeFs.exists = function(p) return p == tmpname end + + local fakeStrUtils = {} + fakeStrUtils.encodeBase64 = function(_) return nil end + + local ts = loadWithFakes(fakeFs, {}, fakeStrUtils) + local result = ts.encodeBase64(tmpname) + + assert.is_nil(result) + os.remove(tmpname) + end) +end) + +-------------------------------------------------------------------------------- +describe("ThumbnailService — export()", function() + + it("returns nil when the temp directory does not exist", function() + local fakePathUtils = {} + fakePathUtils.getStandardFilePath = function(kind) return "/nonexistent" end + + local fakeFs = {} + fakeFs.exists = function(_) return false end + fakeFs.chooseUniqueFileName = function(template) return template end + + local ts = loadWithFakes(fakeFs, fakePathUtils, {}) + local result = ts.export({ uuid = "p1" }) + + assert.is_nil(result) + end) + + it("returns nil when the photo API returns failure", function() + local fakePathUtils = {} + fakePathUtils.getStandardFilePath = function(kind) return "/tmp" end + + local fakeFs = {} + fakeFs.exists = function(_) return true end + fakeFs.chooseUniqueFileName = function(template) return template end + + local photo = { + requestJpegThumbnail = function(_, width, height, cb) + return false, "no thumbnail available" + end, + } + + local ts = loadWithFakes(fakeFs, fakePathUtils, {}) + local result = ts.export(photo) + + assert.is_nil(result) + end) + + it("returns nil when the callback receives no JPEG data", function() + local fakePathUtils = {} + fakePathUtils.getStandardFilePath = function(kind) return "/tmp" end + + local fakeFs = {} + fakeFs.exists = function(_) return true end + fakeFs.chooseUniqueFileName = function(template) return template end + + local photo = { + requestJpegThumbnail = function(_, width, height, cb) + cb(nil) + return true, "ok" + end, + } + + local ts = loadWithFakes(fakeFs, fakePathUtils, {}) + local result = ts.export(photo) + + assert.is_nil(result) + end) + + it("returns the path on successful export", function() + local fakePathUtils = {} + fakePathUtils.getStandardFilePath = function(kind) return "/tmp" end + + local writtenBytes = nil + local capturedPath = nil + local originalIoOpen = io.open + -- Intercept io.open to capture written data. + io.open = function(filepath, mode) + capturedPath = filepath + return { + write = function(_, data) writtenBytes = data end, + close = function() end, + } + end + + local fakeFs = {} + -- exists is called twice: once for temp dir check, once for final verification + fakeFs.exists = function(_) return true end + fakeFs.chooseUniqueFileName = function(template) return "/tmp/thumb_1.jpg" end + + local photo = { + requestJpegThumbnail = function(_, width, height, cb) + assert.are_same(512, width, "Should request 512px width") + assert.are_same(512, height, "Should request 512px height") + cb("JPEG_BINARY_DATA") + return true, "ok" + end, + } + + local ts = loadWithFakes(fakeFs, fakePathUtils, {}) + local result = ts.export(photo) + + io.open = originalIoOpen + + assert.are_same("/tmp/thumb_1.jpg", result) + assert.are_same("JPEG_BINARY_DATA", writtenBytes) + end) + + it("returns nil when io.open fails inside the callback", function() + local fakePathUtils = {} + fakePathUtils.getStandardFilePath = function(kind) return "/tmp" end + + local originalIoOpen = io.open + io.open = function(_, _) return nil end -- simulate file-open failure + + local fakeFs = {} + fakeFs.exists = function(_) return true end + fakeFs.chooseUniqueFileName = function(template) return "/tmp/thumb_1.jpg" end + + local photo = { + requestJpegThumbnail = function(_, width, height, cb) + cb("JPEG_BINARY_DATA") + return true, "ok" + end, + } + + local ts = loadWithFakes(fakeFs, fakePathUtils, {}) + local result = ts.export(photo) + + io.open = originalIoOpen + + assert.is_nil(result) + end) + + it("requests 512x512 dimensions", function() + local fakePathUtils = {} + fakePathUtils.getStandardFilePath = function(kind) return "/tmp" end + + local dims = { w = nil, h = nil } + local originalIoOpen = io.open + io.open = function(_, _) + return { + write = function(_, _) end, + close = function() end, + } + end + + local fakeFs = {} + fakeFs.exists = function(_) return true end + fakeFs.chooseUniqueFileName = function(template) return "/tmp/thumb_1.jpg" end + + local photo = { + requestJpegThumbnail = function(_, width, height, cb) + dims.w = width + dims.h = height + cb("data") + return true, "ok" + end, + } + + local ts = loadWithFakes(fakeFs, fakePathUtils, {}) + ts.export(photo) + + io.open = originalIoOpen + + assert.are_same(512, dims.w) + assert.are_same(512, dims.h) + end) +end)