From 99da500a6ddc82487471c7c88665add5bb288ffa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 12 Jan 2026 19:07:45 +0000 Subject: [PATCH 1/2] Initial plan From f5029e32df01372c352fbbf5eeb41d845df73318 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Jan 2026 06:43:29 +0000 Subject: [PATCH 2/2] Merge branch claude/add-error-handling-mk82zx2vzck9nv9m-IMm3C Co-authored-by: codeGlaze <11318451+codeGlaze@users.noreply.github.com> --- .devcontainer/Dockerfile | 45 ++ .devcontainer/devcontainer.json | 62 +++ .devcontainer/post-create.sh | 49 ++ .vscode/tasks.json | 26 ++ AGENTS.md | 137 ++++++ docs/CODEBASE.md | 228 ++++++++++ docs/ERROR_HANDLING.md | 209 +++++++++ docs/ORCBREW_FILE_VALIDATION.md | 416 +++++++++++++++++ menu | 37 ++ src/clj/orcpub/datomic.clj | 39 +- src/clj/orcpub/email.clj | 156 +++++-- src/clj/orcpub/pdf.clj | 77 +++- src/clj/orcpub/routes.clj | 295 ++++++++---- src/clj/orcpub/routes/party.clj | 112 ++++- src/clj/orcpub/system.clj | 9 +- src/cljc/orcpub/common.cljc | 17 +- src/cljc/orcpub/errors.cljc | 157 ++++++- src/cljs/orcpub/dnd/e5/events.cljs | 164 +++++-- src/cljs/orcpub/dnd/e5/import_validation.cljs | 428 ++++++++++++++++++ start.sh | 119 +++++ test/clj/orcpub/errors_test.clj | 177 ++++++++ .../orcpub/dnd/e5/import_validation_test.cljs | 410 +++++++++++++++++ 22 files changed, 3166 insertions(+), 203 deletions(-) create mode 100644 .devcontainer/Dockerfile create mode 100644 .devcontainer/devcontainer.json create mode 100644 .devcontainer/post-create.sh create mode 100644 .vscode/tasks.json create mode 100644 AGENTS.md create mode 100644 docs/CODEBASE.md create mode 100644 docs/ERROR_HANDLING.md create mode 100644 docs/ORCBREW_FILE_VALIDATION.md create mode 100755 menu create mode 100644 src/cljs/orcpub/dnd/e5/import_validation.cljs create mode 100755 start.sh create mode 100644 test/clj/orcpub/errors_test.clj create mode 100644 test/cljs/orcpub/dnd/e5/import_validation_test.cljs diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile new file mode 100644 index 000000000..8b131e657 --- /dev/null +++ b/.devcontainer/Dockerfile @@ -0,0 +1,45 @@ +# OrcPub Development Container +# Requires: Java 8, Leiningen, Node.js (for ClojureScript tooling) + +FROM mcr.microsoft.com/devcontainers/base:ubuntu-22.04 + +# Avoid prompts during package installation +ENV DEBIAN_FRONTEND=noninteractive + +# Install OpenJDK 8 and other dependencies +RUN apt-get update && apt-get install -y \ + openjdk-8-jdk \ + openjdk-8-jdk-headless \ + curl \ + wget \ + unzip \ + git \ + rlwrap \ + nodejs \ + npm \ + && apt-get clean \ + && rm -rf /var/lib/apt/lists/* + +# Set Java 8 as default +ENV JAVA_HOME=/usr/lib/jvm/java-8-openjdk-amd64 +ENV PATH="${JAVA_HOME}/bin:${PATH}" + +# Verify Java version +RUN java -version + +# Install Leiningen +RUN mkdir -p /usr/local/bin \ + && curl -fsSL https://raw.githubusercontent.com/technomancy/leiningen/stable/bin/lein -o /usr/local/bin/lein \ + && chmod +x /usr/local/bin/lein + +# Pre-fetch leiningen dependencies as root (will be cached) +RUN lein --version + +# Create workspace directory +WORKDIR /workspaces/orcpub + +# Switch to vscode user for development +USER vscode + +# Ensure lein works for vscode user (will download to user home) +RUN lein --version diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json new file mode 100644 index 000000000..736da4a3f --- /dev/null +++ b/.devcontainer/devcontainer.json @@ -0,0 +1,62 @@ +{ + "name": "OrcPub Development", + "build": { + "dockerfile": "Dockerfile", + "context": ".." + }, + "features": { + "ghcr.io/devcontainers/features/common-utils:2": { + "installZsh": true, + "configureZshAsDefaultShell": true, + "installOhMyZsh": true, + "upgradePackages": true + }, + "ghcr.io/devcontainers/features/git:1": {} + }, + "customizations": { + "vscode": { + "extensions": [ + "betterthantomorrow.calva", + "betterthantomorrow.calva-spritz", + "vscjava.vscode-java-pack", + "redhat.java" + ], + "settings": { + "java.configuration.runtimes": [ + { + "name": "JavaSE-1.8", + "path": "/usr/lib/jvm/java-8-openjdk-amd64", + "default": true + } + ], + "calva.replConnectSequences": [ + { + "name": "OrcPub Server", + "projectType": "Leiningen", + "cljsType": "Figwheel Main", + "menuSelections": { + "leinProfiles": ["dev"] + } + } + ] + } + } + }, + "forwardPorts": [4334, 3000, 8080], + "portsAttributes": { + "4334": { + "label": "Datomic Transactor", + "onAutoForward": "silent" + }, + "3000": { + "label": "ClojureScript Dev Server", + "onAutoForward": "notify" + }, + "8080": { + "label": "OrcPub Server", + "onAutoForward": "openBrowser" + } + }, + "postCreateCommand": "bash .devcontainer/post-create.sh", + "remoteUser": "vscode" +} diff --git a/.devcontainer/post-create.sh b/.devcontainer/post-create.sh new file mode 100644 index 000000000..2650514d3 --- /dev/null +++ b/.devcontainer/post-create.sh @@ -0,0 +1,49 @@ +#!/usr/bin/env bash +set -euo pipefail + +echo "=== OrcPub Development Container Setup ===" + +# Verify Java 8 +echo "Checking Java version..." +java -version 2>&1 | head -2 +JAVA_VER=$(java -version 2>&1 | awk -F '"' '/version/ {print $2}') +if [[ "${JAVA_VER}" != 1.8* ]]; then + echo "WARNING: Expected Java 8, found $JAVA_VER" +else + echo "✓ Java 8 verified" +fi + +# Verify Leiningen +echo "Checking Leiningen..." +lein --version +echo "✓ Leiningen verified" + +# Extract Datomic if not already extracted +DATOMIC_DIR="lib/datomic-free-0.9.5703" +DATOMIC_ARCHIVE="lib/datomic-free-0.9.5703.tar.gz" + +if [ -f "$DATOMIC_ARCHIVE" ] && [ ! -d "$DATOMIC_DIR/bin" ]; then + echo "Extracting Datomic..." + tar -xzf "$DATOMIC_ARCHIVE" -C lib/ + echo "✓ Datomic extracted" +elif [ -d "$DATOMIC_DIR/bin" ]; then + echo "✓ Datomic already extracted" +else + echo "WARNING: Datomic archive not found at $DATOMIC_ARCHIVE" +fi + +# Fetch project dependencies +echo "Fetching project dependencies (this may take a while on first run)..." +lein deps || echo "WARNING: Could not fetch all dependencies" + +echo "" +echo "=== Setup Complete ===" +echo "" +echo "To start the development environment, run:" +echo " ./start.sh" +echo "" +echo "Or manually:" +echo " 1. Start Datomic: $DATOMIC_DIR/bin/transactor $DATOMIC_DIR/config/samples/free-transactor-template.properties" +echo " 2. Start server: lein run" +echo " 3. Start REPL: lein repl" +echo "" diff --git a/.vscode/tasks.json b/.vscode/tasks.json new file mode 100644 index 000000000..b0198926c --- /dev/null +++ b/.vscode/tasks.json @@ -0,0 +1,26 @@ +{ + "version": "2.0.0", + "tasks": [ + { + "label": "Datomic Transactor", + "type": "shell", + "command": "./lib/datomic-free-0.9.5703/bin/transactor ./lib/datomic-free-0.9.5703/config/working-transactor.properties", + "isBackground": true, + "problemMatcher": [] + }, + { + "label": "Clojure Server", + "type": "shell", + "command": "lein run", + "isBackground": true, + "problemMatcher": [] + }, + { + "label": "Lein REPL", + "type": "shell", + "command": "lein repl", + "isBackground": true, + "problemMatcher": [] + } + ] +} diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000..78645436f --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,137 @@ +# Agent Guidelines + +> **This is a living document.** Update it as conventions evolve or new patterns emerge. When you establish a new pattern or discover a better approach, document it here. + +--- + +## First Steps + +**Before doing anything else, read [`docs/CODEBASE.md`](./docs/CODEBASE.md).** + +That document contains: +- Tech stack and architecture overview +- Directory structure and key files +- Core concepts (entity/template/modifier system) +- Patterns and conventions +- Known quirks and gotchas +- Accumulated learnings from previous work + +**Update `docs/CODEBASE.md`** when you: +- Discover something non-obvious about the codebase +- Find a quirk or gotcha that would trip up future agents +- Learn how a subsystem works that isn't documented +- Complete significant work that changes how things work + +--- + +## Development Setup + +### Quick Start (Local) + +1. **Java 8** required (not newer) +2. **Leiningen** for build/REPL +3. **Datomic transactor** running: + ```bash + # Windows (use bundled version due to path length bug) + bin\transactor config/samples/free-transactor-template.properties + + # Mac/Linux + bin/transactor config/samples/free-transactor-template.properties + ``` +4. **Backend REPL**: `lein with-profile +start-server repl` + ```clojure + (init-database) ; only once per fresh DB + (start-server) + ``` +5. **Frontend**: `lein figwheel` (hot reloads) + +### Using Dev Container + +The `.devcontainer/` setup handles all dependencies automatically. + +### Using start.sh + +The `start.sh` script automates the above steps with dependency checking. + +--- + +## Code Conventions + +### Error Handling + +Use the DRY macros from `src/cljc/orcpub/errors.cljc`: + +```clojure +(require '[orcpub.errors :as errors]) + +;; Database operations +(errors/with-db-error-handling :operation-failed {:user-id id} "Failed to save" + (d/transact conn tx-data)) + +;; Email operations +(errors/with-email-error-handling :email-failed {:to email} "Failed to send" + (postal/send-message msg)) + +;; Parsing/validation +(errors/with-validation :invalid-input {:field "id"} "Invalid ID format" + (Long/parseLong id-string)) +``` + +See `docs/ERROR_HANDLING.md` for full documentation. + +### File Organization + +- **Backend only**: `src/clj/orcpub/` +- **Frontend only**: `src/cljs/orcpub/` +- **Shared code**: `src/cljc/orcpub/` (runs on both JVM and browser) + +### Testing + +- Mirror source structure in `test/clj/` and `test/cljs/` +- Run backend tests: `lein test` +- Run frontend tests: `lein doo` + +--- + +## Working on This Repo + +### Before Making Changes + +1. Read `docs/CODEBASE.md` for context +2. Understand the entity/template/modifier system if touching character logic +3. Check existing patterns in similar files + +### After Making Changes + +1. Run relevant tests +2. Update `docs/CODEBASE.md` if you learned something new +3. Update this file if you established new conventions + +### Commit Style + +Follow conventional commits: +- `feat:` new features +- `fix:` bug fixes +- `docs:` documentation changes +- `refactor:` code restructuring +- `test:` adding/updating tests + +--- + +## Key Documentation + +| Document | Purpose | +|----------|---------| +| [`docs/CODEBASE.md`](./docs/CODEBASE.md) | **Start here.** Architecture, patterns, learnings | +| [`docs/ERROR_HANDLING.md`](./docs/ERROR_HANDLING.md) | Error handling utilities and patterns | +| [`docs/ORCBREW_FILE_VALIDATION.md`](./docs/ORCBREW_FILE_VALIDATION.md) | File import/export validation | +| [`README.md`](./README.md) | Setup, deployment, contributing | + +--- + +## Notes for Future Agents + +- The modifier system uses `?symbol` syntax - this is intentional DSL, not a typo +- Datomic Free on Windows requires the bundled version (path length bug) +- Frontend changes hot-reload; backend changes need REPL reload +- `clojure.edn/read-string` is safe; `clojure.core/read-string` is not - we use the safe one diff --git a/docs/CODEBASE.md b/docs/CODEBASE.md new file mode 100644 index 000000000..7f4176d23 --- /dev/null +++ b/docs/CODEBASE.md @@ -0,0 +1,228 @@ +# OrcPub Codebase Knowledge + +> **This is a living document.** It is meant to be updated by agents (and humans) as understanding of the codebase evolves. When you learn something new or discover a quirk, add it here. See also: [AGENTS.md](../AGENTS.md) for contribution guidelines. + +--- + +## Quick Overview + +**OrcPub / Dungeon Master's Vault** is a D&D 5e character sheet generator. It's a full-stack Clojure/ClojureScript application forked from the original OrcPub2. + +### Tech Stack + +| Layer | Technology | +|-------|------------| +| **Backend** | Clojure (JVM) | +| **Frontend** | ClojureScript (browser) | +| **HTTP Server** | Pedestal + Ring | +| **Database** | Datomic (entity-attribute-value store) | +| **Auth** | Buddy (JWT, hashing) | +| **Email** | Postal (SMTP) | +| **PDF** | Apache PDFBox | +| **UI Framework** | Reagent (React wrapper) | +| **State Management** | re-frame (Redux-like) | +| **CSS** | Garden (CSS-in-Clojure) | +| **Build** | Leiningen | +| **Live Reload** | Figwheel | + +### Directory Structure + +``` +orcpub/ +├── src/ +│ ├── clj/ # Backend (JVM) code +│ │ └── orcpub/ +│ │ ├── datomic.clj # Database component & queries +│ │ ├── email.clj # Email operations (verification, reset) +│ │ ├── pdf.clj # PDF character sheet generation +│ │ ├── routes.clj # HTTP route handlers +│ │ ├── routes/ # Route handler modules +│ │ │ └── party.clj # Party management +│ │ └── system.clj # Component system setup +│ │ +│ ├── cljs/ # Frontend (browser) code +│ │ └── orcpub/ +│ │ └── dnd/e5/ +│ │ ├── events.cljs # re-frame event handlers +│ │ ├── views.cljs # Reagent UI components +│ │ └── import_validation.cljs # Orcbrew file validation +│ │ +│ └── cljc/ # Shared code (JVM + browser) +│ └── orcpub/ +│ ├── common.cljc # Shared utilities +│ ├── errors.cljc # Error handling utilities +│ └── entity.cljc # Core entity/modifier system +│ +├── test/ +│ ├── clj/ # Backend tests +│ └── cljs/ # Frontend tests +│ +├── docs/ # Documentation +├── resources/ # Static resources, templates +├── deploy/ # Deployment configs (nginx, SSL) +├── docker/ # Docker configs +└── project.clj # Leiningen project config +``` + +--- + +## Core Concepts + +### The Entity/Template/Modifier System + +This is the heart of OrcPub. Understanding this is critical. + +**The Problem**: D&D characters have hundreds of interacting options (race, class, feats, items) that modify stats in complex ways. A naive approach of hardcoding every interaction doesn't scale. + +**The Solution**: A declarative system where: + +1. **Entity** = A record of hierarchical choices made (e.g., "I chose Elf > High Elf") +2. **Template** = Defines available options and their modifiers +3. **Modifier** = A transformation applied to character attributes +4. **Built Entity** = The final computed character after applying all modifiers + +```clojure +;; Entity: just stores choices +{:options {:race {:key :elf + :options {:subrace {:key :high-elf}}}}} + +;; Template: defines what options exist and their effects +{:selections [{:key :race + :options [{:name "Elf" + :key :elf + :modifiers [(modifier ?dex-bonus (+ ?dex-bonus 2))] + :selections [{:key :subrace ...}]}]}]} + +;; Built entity: computed result +{:race "Elf", :subrace "High Elf", :dex-bonus 2, :int-bonus 1} +``` + +**Key insight**: Modifiers use `?attribute` syntax to reference and update values. The system builds a dependency graph and applies modifiers in topologically sorted order. + +### Data Flow + +``` +[Browser UI] <--re-frame events--> [ClojureScript State] + | + HTTP/Transit + | + [Pedestal Routes] + | + [Datomic Database] +``` + +- **Frontend state**: Managed by re-frame (subscriptions + events) +- **API format**: Transit (Clojure's efficient serialization) +- **Database**: Datomic stores entities as attribute-value facts + +--- + +## Key Files + +| File | Responsibility | +|------|----------------| +| `src/clj/orcpub/routes.clj` | Main HTTP route handlers (character CRUD, auth) | +| `src/clj/orcpub/datomic.clj` | Database connection, schema, queries | +| `src/clj/orcpub/pdf.clj` | PDF character sheet generation | +| `src/clj/orcpub/email.clj` | Email sending (verification, password reset) | +| `src/clj/orcpub/system.clj` | Component system initialization | +| `src/cljc/orcpub/entity.cljc` | Core entity building logic | +| `src/cljc/orcpub/errors.cljc` | Error handling utilities (macros) | +| `src/cljs/orcpub/dnd/e5/events.cljs` | re-frame event handlers | +| `src/cljs/orcpub/dnd/e5/views.cljs` | Main UI components | +| `project.clj` | Dependencies, build profiles, aliases | + +--- + +## Patterns & Conventions + +### Error Handling + +The codebase uses DRY error handling macros defined in `src/cljc/orcpub/errors.cljc`: + +```clojure +;; For database operations +(with-db-error-handling :error-code {:context "data"} "User message" + (db-operation)) + +;; For email operations +(with-email-error-handling :error-code {:context "data"} "User message" + (send-email)) + +;; For parsing/validation +(with-validation :error-code {:context "data"} "User message" + (parse-input)) +``` + +All errors follow a consistent structure: +```clojure +{:error :error-code-keyword + :context-key "context-value" + :message "underlying exception message"} +``` + +See `docs/ERROR_HANDLING.md` for full details. + +### State Management (Frontend) + +Uses re-frame pattern: +- **Events**: `(rf/dispatch [:event-name data])` - trigger state changes +- **Subscriptions**: `(rf/subscribe [:sub-name])` - reactive data access +- **Effects**: Side effects (HTTP, local storage) via effect handlers + +### Testing + +- Backend: `clojure.test` - run with `lein test` +- Frontend: `cljs.test` - run with `lein doo` +- Test files mirror source structure in `test/` directory + +--- + +## Known Quirks & Gotchas + +### Datomic on Windows +- Must use the bundled Datomic version in `lib/datomic-free-0.9.5703.tar.gz` +- Newer versions have Windows path length issues +- Known upstream bug that won't be fixed + +### Java Version +- Requires Java 8 specifically +- Later versions may have compatibility issues with Datomic Free + +### REPL Workflow +- Frontend changes hot-reload via Figwheel +- Backend changes require REPL reload or server restart +- `(init-database)` only needs to run once per fresh DB + +### PDF Generation +- Image loading has a 10-second timeout +- External image URLs can fail silently if unreachable +- Uses PDFBox which has specific JPEG handling quirks + +### Orcbrew Files +- EDN format for homebrew content +- Can become corrupted (nil values, missing fields) +- Import validation now handles gracefully (see `docs/ORCBREW_FILE_VALIDATION.md`) + +--- + +## Learnings & Discoveries + +> Add discoveries here as you work on the codebase. Format: `- [Date] [Agent/Person]: Discovery` + +- [2026-01-10] Claude: Comprehensive error handling added across all critical operations (database, email, PDF, routes). Uses DRY macros to reduce boilerplate by ~30%. + +- [2026-01-10] Claude: Orcbrew import validation now uses progressive strategy - imports valid items even if some are corrupted, rather than failing entirely. + +- [2026-01-10] Claude: The `clojure.edn/read-string` is used (safe) rather than `clojure.core/read-string` (unsafe, allows code execution). + +- [2026-01-12] Claude: Orcbrew auto-cleaning is split into two phases: (1) string-level for syntax fixes only (trailing commas, `disabled? nil`), (2) data-level for semantic fixes after parsing. Nil handling is field-specific: preserve for semantic fields (`:spell-list-kw`), remove for numeric fields (`:str`, `:dex`), replace for others (`:option-pack`). String-level regex manipulation of top-level keys caused duplicate key errors. + +--- + +## Related Documentation + +- [ERROR_HANDLING.md](./ERROR_HANDLING.md) - Error handling patterns and utilities +- [ORCBREW_FILE_VALIDATION.md](./ORCBREW_FILE_VALIDATION.md) - File import/export validation +- [AGENTS.md](../AGENTS.md) - Guidelines for AI agents working on this repo +- [README.md](../README.md) - Setup, deployment, and contributing guide diff --git a/docs/ERROR_HANDLING.md b/docs/ERROR_HANDLING.md new file mode 100644 index 000000000..0206f7829 --- /dev/null +++ b/docs/ERROR_HANDLING.md @@ -0,0 +1,209 @@ +# Error Handling Guide + +This document describes the error handling approach used throughout the OrcPub application. + +## Overview + +The application uses a consistent, DRY approach to error handling built on Clojure's `ex-info` for structured exceptions. All error handling utilities are centralized in the `orcpub.errors` namespace. + +## Core Principles + +1. **User-Friendly Messages**: All errors include clear, actionable messages for end users +2. **Structured Data**: Errors use `ex-info` with structured data for programmatic handling +3. **Logging**: All errors are logged with context for debugging +4. **Fail Fast**: Operations fail immediately with clear errors rather than silently continuing +5. **DRY**: Common error handling patterns use reusable macros and utilities + +## Error Handling Utilities + +### Database Operations + +Use `with-db-error-handling` for all database transactions: + +```clojure +(require '[orcpub.errors :as errors]) + +(defn save-user [conn user-data] + (errors/with-db-error-handling :user-save-failed + {:username (:username user-data)} + "Unable to save user. Please try again." + @(d/transact conn [user-data]))) +``` + +**Benefits:** +- Automatically logs database errors with context +- Creates user-friendly error messages +- Includes structured error codes for programmatic handling +- Re-throws `ExceptionInfo` as-is (for already-handled errors) + +### Email Operations + +Use `with-email-error-handling` for sending emails: + +```clojure +(defn send-welcome-email [user-email] + (errors/with-email-error-handling :welcome-email-failed + {:email user-email} + "Unable to send welcome email. Please contact support." + (postal/send-message config message))) +``` + +**Benefits:** +- Handles SMTP connection failures gracefully +- Logs email failures for ops monitoring +- Prevents application crashes when email server is down + +### Validation & Parsing + +Use `with-validation` for parsing user input: + +```clojure +(defn parse-user-id [id-string] + (errors/with-validation :invalid-user-id + {:id-string id-string} + "Invalid user ID format. Expected a number." + (Long/parseLong id-string))) +``` + +**Benefits:** +- Handles `NumberFormatException` and other parsing errors +- Provides clear validation error messages +- Includes the invalid input in error data for debugging + +## Error Data Structure + +All errors created by the utilities include: + +```clojure +{:error :error-code-keyword ; Machine-readable error type + ;; Additional context fields specific to the operation + :user-id 123 + :operation-specific-data "..."} +``` + +Example exception: + +```clojure +(ex-info "Unable to save user. Please try again." + {:error :user-save-failed + :username "alice" + :message "Connection timeout"} + original-exception) +``` + +## Error Codes + +Error codes are defined as keywords in `orcpub.errors`: + +### Database Errors +- `:party-creation-failed` - Failed to create a party +- `:party-update-failed` - Failed to update party data +- `:party-remove-character-failed` - Failed to remove character from party +- `:party-deletion-failed` - Failed to delete party +- `:verification-failed` - Failed to create verification record +- `:password-reset-failed` - Failed to initiate password reset +- `:password-update-failed` - Failed to update password +- `:entity-creation-failed` - Failed to create entity +- `:entity-update-failed` - Failed to update entity + +### Email Errors +- `:verification-email-failed` - Failed to send verification email +- `:reset-email-failed` - Failed to send password reset email +- `:invalid-port` - Invalid email server port configuration + +### Validation Errors +- `:invalid-character-id` - Invalid character ID format +- `:invalid-pdf-data` - Invalid PDF request data + +### PDF Errors +- `:image-load-timeout` - Image loading timed out +- `:unknown-host` - Unknown host for image URL +- `:invalid-image-format` - Invalid or corrupt image +- `:image-load-failed` - Generic image loading failure +- `:jpeg-load-failed` - JPEG-specific loading failure + +## Testing Error Handling + +All error handling utilities are fully tested. See `test/clj/orcpub/errors_test.clj` for examples: + +```clojure +(deftest test-with-db-error-handling + (testing "wraps exceptions with proper context" + (try + (errors/with-db-error-handling :db-test-error + {:user-id 789} + "Unable to save to database" + (throw (Exception. "Connection timeout"))) + (is false "Should have thrown exception") + (catch clojure.lang.ExceptionInfo e + (is (= "Unable to save to database" (.getMessage e))) + (is (= :db-test-error (:error (ex-data e)))) + (is (= 789 (:user-id (ex-data e)))))))) +``` + +## Migration Guide + +### Before (Bespoke Error Handling) + +```clojure +(defn create-party [conn party] + (try + (let [result @(d/transact conn [party])] + {:status 200 :body result}) + (catch Exception e + (println "ERROR: Failed to create party:" (.getMessage e)) + (throw (ex-info "Unable to create party. Please try again." + {:error :party-creation-failed} + e))))) +``` + +### After (DRY with Utilities) + +```clojure +(defn create-party [conn party] + (errors/with-db-error-handling :party-creation-failed + {:party-data party} + "Unable to create party. Please try again." + (let [result @(d/transact conn [party])] + {:status 200 :body result}))) +``` + +**Benefits of migration:** +- 7 lines → 5 lines (30% reduction) +- Consistent error logging format +- No need to remember logging syntax +- Easier to read and maintain + +## Best Practices + +### DO: +- Use the provided macros for common operations +- Include relevant context in error data +- Write user-friendly error messages +- Test error handling paths + +### DON'T: +- Catch exceptions without re-throwing +- Use generic error messages like "An error occurred" +- Log errors without context +- Silently swallow exceptions + +## Future Improvements + +Potential enhancements to consider: + +1. **Retry Logic**: Add automatic retry for transient failures (network, db) +2. **Circuit Breakers**: Prevent cascading failures in external dependencies +3. **Error Monitoring**: Integration with error tracking services (Sentry, Rollbar) +4. **Rate Limiting**: Add rate limiting context to prevent abuse +5. **Internationalization**: Support multiple languages for error messages + +## Related Files + +- `src/cljc/orcpub/errors.cljc` - Error handling utilities +- `test/clj/orcpub/errors_test.clj` - Comprehensive test suite +- `src/clj/orcpub/email.clj` - Email operations with error handling +- `src/clj/orcpub/datomic.clj` - Database connection with error handling +- `src/clj/orcpub/routes.clj` - HTTP routes with error handling +- `src/clj/orcpub/routes/party.clj` - Party operations (demonstrates DRY refactoring) +- `src/clj/orcpub/pdf.clj` - PDF generation with timeout and error handling diff --git a/docs/ORCBREW_FILE_VALIDATION.md b/docs/ORCBREW_FILE_VALIDATION.md new file mode 100644 index 000000000..04a9e65e7 --- /dev/null +++ b/docs/ORCBREW_FILE_VALIDATION.md @@ -0,0 +1,416 @@ +# OrcBrew File Import/Export Validation + +## Overview + +This document explains the comprehensive validation system for `.orcbrew` files, which helps catch and fix bugs before they frustrate users. + +## What Changed? + +### Before +- ❌ Generic "Invalid .orcbrew file" errors +- ❌ No way to know what's wrong with a file +- ❌ One bad item breaks the entire import +- ❌ Bugs exported into files with no warning +- ❌ Silent failures and data loss + +### After +- ✅ Detailed error messages explaining what's wrong +- ✅ Progressive import (recovers valid items, skips invalid ones) +- ✅ Pre-export validation (catches bugs before they're saved) +- ✅ Automatic fixing of common corruption patterns +- ✅ Console logging for debugging + +## Features + +### 1. **Pre-Export Validation** + +Before creating an `.orcbrew` file, the system now validates your content: + +``` +Exporting "My Homebrew Pack"... +✓ Checking for nil values +✓ Checking for empty option-pack strings +✓ Validating data structure +✓ Running spec validation + +Export successful! +``` + +**If issues are found:** +- ⚠️ **Warnings** - File exports but issues are logged +- ❌ **Errors** - File won't export, must fix issues first + +**Check the browser console** (F12) for detailed information about any warnings or errors. + +### 2. **Enhanced Import Validation** + +When importing an `.orcbrew` file, you now get detailed feedback: + +#### **Successful Import** +``` +✅ Import successful + +Imported 25 items + +To be safe, export all content now to create a clean backup. +``` + +#### **Import with Warnings** +``` +⚠️ Import completed with warnings + +Imported: 23 valid items +Skipped: 2 invalid items + +Invalid items were skipped. Check the browser console for details. + +To be safe, export all content now to create a clean backup. +``` + +#### **Parse Error** +``` +⚠️ Could not read file + +Error: EOF while reading +Line: 45 + +The file may be corrupted or incomplete. Try exporting a +fresh copy if you have the original source. +``` + +#### **Validation Error** +``` +⚠️ Invalid orcbrew file + +Validation errors found: + • at spells > fireball: Missing required field: :option-pack + • at races > dwarf: Invalid value format + Got: {:name "Dwarf", :option-pack nil} + +To recover data from this file, you can: +1. Try progressive import (imports valid items, skips invalid ones) +2. Check the browser console for detailed validation errors +3. Export a fresh copy if you have the original source +``` + +### 3. **Progressive Import** + +The default import strategy is **progressive**, which means: + +- ✅ Valid items are imported successfully +- ⚠️ Invalid items are skipped and reported +- 📊 You get a count of imported vs skipped items +- 🔍 Detailed errors for skipped items appear in the console + +**Example:** + +If your file has 10 spells and 2 of them are missing the `option-pack` field: +- The 8 valid spells are imported +- The 2 invalid spells are skipped +- You see: "Imported: 8 valid items, Skipped: 2 invalid items" +- Console shows exactly which spells were skipped and why + +### 4. **Automatic Cleaning** + +The system automatically fixes these common corruption patterns: + +| Pattern | Fix | +|---------|-----| +| `disabled? nil` | `disabled? false` | +| `nil nil,` | (removed) | +| `:field-name nil` | (removed) | +| `option-pack ""` | `option-pack "Default Option Source"` | +| Empty plugin name `""` | `"Default Option Source"` | + +**This happens automatically** - you don't need to do anything! + +### 5. **Detailed Console Logging** + +Open the browser console (F12) to see: + +**On Export:** +```javascript +Export warnings for "My Pack": + Item spells/fireball has missing option-pack + Found nil value for key: :some-field +``` + +**On Import:** +```javascript +Import validation result: { + success: true, + imported_count: 23, + skipped_count: 2, + skipped_items: [ + {key: :invalid-spell, errors: "..."}, + {key: :bad-race, errors: "..."} + ] +} + +Skipped invalid items: + :invalid-spell + Errors: Missing required field: :option-pack + :bad-race + Errors: Invalid value format +``` + +## How to Use + +### Exporting Content + +**Single Plugin:** +1. Go to "Manage Homebrew" +2. Click "Export" next to your plugin +3. If warnings appear, check the console (F12) +4. Fix any issues and export again + +**All Plugins:** +1. Go to "Manage Homebrew" +2. Click "Export All" +3. If any plugin has errors, export is blocked +4. Check console for which plugin has issues +5. Fix issues and try again + +### Importing Content + +**Standard Import (Progressive):** +1. Click "Import Content" +2. Select your `.orcbrew` file +3. Read the import message +4. If warnings appear, check console for details +5. **IMPORTANT:** Export all content to create a clean backup + +**Strict Import (All-or-Nothing):** + +For users who want the old behavior (reject entire file if any item is invalid): + +*This feature is available via browser console only:* +```javascript +// In browser console +dispatch(['orcpub.dnd.e5.events/import-plugin-strict', 'Plugin Name', 'file contents']) +``` + +## Common Error Messages & Fixes + +### "Missing required field: :option-pack" + +**Cause:** Item doesn't have an `option-pack` field + +**Fix:** Each item MUST have `:option-pack ""` + +```clojure +;; Bad +{:name "Fireball" :level 3} + +;; Good +{:option-pack "My Homebrew" + :name "Fireball" + :level 3} +``` + +### "EOF while reading" + +**Cause:** File is incomplete or has unmatched brackets + +**Fix:** +1. Check for missing `}`, `]`, or `)` +2. Make sure every opening bracket has a closing bracket +3. If file is very corrupted, you may need to restore from backup + +### "Invalid value format" + +**Cause:** Value doesn't match expected format (e.g., number instead of string) + +**Fix:** Check the item in console logs to see which field is wrong + +### "File appears to be corrupted" + +**Cause:** File is incomplete, truncated, or contains invalid characters + +**Fix:** +1. Try re-exporting from the original source +2. If you only have this copy, try progressive import to recover what you can + +## Best Practices + +### 1. **Always Export After Importing** + +After importing any file: +``` +Import → Check Message → Export All → Save Backup +``` + +This creates a clean version with all auto-fixes applied. + +### 2. **Check Console Regularly** + +When working with homebrew content, keep the browser console open (F12) to catch issues early. + +### 3. **Keep Backups** + +Export your content regularly: +- After major changes +- After importing new content +- Before deleting or modifying existing content + +### 4. **Use Progressive Import for Recovery** + +If you have a corrupted file, progressive import can recover valid items: +- You'll lose invalid items, but keep everything else +- Console will show exactly what was lost +- You can manually recreate lost items + +### 5. **Fix Warnings Before Sharing** + +If you're sharing your homebrew: +- Export and check for warnings +- Fix any issues +- Export again to create a clean file +- Share the clean file + +## Technical Details + +### Validation Process + +1. **Auto-Clean** - Fix common corruption patterns +2. **Parse EDN** - Convert text to data structure +3. **Validate Structure** - Check against spec +4. **Item-Level Validation** - Check each item individually +5. **Report Results** - Show user-friendly messages + +### Import Strategies + +**Progressive (Default):** +- Imports valid items +- Skips invalid items +- Reports what was skipped +- Best for recovery + +**Strict (Optional):** +- All items must be valid +- Rejects entire file if any item is invalid +- Best for validation + +### Spec Requirements + +Every homebrew item must have: +- `:option-pack` (string) - The source/pack name +- Valid qualified keywords for content types +- Keywords must start with a letter +- Content types must be in `orcpub.dnd.e5` namespace + +### Error Data Structure + +All errors include: +- **error** - Error message +- **context** - Where the error occurred +- **hint** - Suggested fix (when available) +- **line** - Line number (for parse errors) + +## Troubleshooting + +### Import Fails with "Could not read file" + +**Likely Cause:** File is corrupted or incomplete + +**Solutions:** +1. Check file size - is it much smaller than expected? +2. Open in text editor - does it end abruptly? +3. Try progressive import to recover what you can +4. If you have the original source, re-export + +### Import Shows "Skipped X items" + +**Likely Cause:** Some items are invalid + +**Solutions:** +1. Check console (F12) for detailed errors +2. Note which items were skipped +3. Re-create those items manually +4. Export all content to save clean version + +### Export Blocked with Errors + +**Likely Cause:** Your content has invalid data + +**Solutions:** +1. Check console for which plugin has errors +2. Look for items with empty/nil option-pack +3. Fix the issues +4. Try export again + +### Console Shows "nil value" Warnings + +**Likely Cause:** Some fields have `nil` instead of proper values + +**Impact:** Usually auto-fixed, but may indicate data quality issues + +**Solutions:** +1. Review your content +2. Fill in missing fields +3. Export to create clean version + +## Support + +If you encounter issues not covered here: + +1. **Check the console** (F12) for detailed errors +2. **Create an issue** on GitHub with: + - Error message from console + - Steps to reproduce + - Sample .orcbrew file (if you can share it) +3. **Try progressive import** to recover data + +## Migration Guide + +### For Existing Users + +Your existing `.orcbrew` files will continue to work! The system: + +1. Auto-cleans common issues +2. Uses progressive import by default +3. Helps you create cleaner files going forward + +**Recommended:** +1. Import your existing files +2. Check for any warnings +3. Export all content to create clean versions +4. Use the clean versions going forward + +### For Developers + +If you're building tools that generate `.orcbrew` files: + +1. **Use the validation API** before creating files +2. **Ensure all items have option-pack** +3. **Avoid nil values** in output +4. **Test with the validator** to catch issues early + +```clojure +;; Validate before export +(require '[orcpub.dnd.e5.import-validation :as import-val]) + +(let [result (import-val/validate-before-export my-plugin)] + (if (:valid result) + (export-to-file my-plugin) + (handle-errors (:errors result)))) +``` + +## Version History + +### v2.0 - Comprehensive Validation (Current) +- Added pre-export validation +- Added progressive import +- Added detailed error messages +- Added automatic cleaning +- Added console logging +- 30+ validation test cases + +### v1.0 - Basic Validation (Legacy) +- Simple spec validation +- All-or-nothing import +- Generic error messages + +--- + +**Questions?** Open an issue on GitHub or check the browser console for detailed error information. diff --git a/menu b/menu new file mode 100755 index 000000000..ed1075aac --- /dev/null +++ b/menu @@ -0,0 +1,37 @@ +#!/usr/bin/env bash + +while true; do + echo "OrcPub Service Launcher" + echo "----------------------" + echo "1) Datomic Transactor" + echo "2) Clojure REPL (for init/start)" + echo "3) Figwheel (ClojureScript)" + echo "4) Quit" + echo + read -p "Select a service to launch [1-4]: " choice + + case $choice in + 1) + echo "Starting Datomic Transactor..." + /workspaces/orcpub/lib/datomic-free-0.9.5703/bin/transactor /workspaces/orcpub/lib/datomic-free-0.9.5703/config/working-transactor.properties & + ;; + 2) + echo "Starting Clojure REPL..." + lein repl & + echo "You can now run (init-database), (start-server), etc. in the REPL." + ;; + 3) + echo "Starting Figwheel (ClojureScript)..." + lein figwheel & + ;; + 4) + echo "Exiting." + exit 0 + ;; + *) + echo "Invalid choice." + ;; + esac + echo + sleep 1 +done diff --git a/src/clj/orcpub/datomic.clj b/src/clj/orcpub/datomic.clj index 41c93044a..188942780 100644 --- a/src/clj/orcpub/datomic.clj +++ b/src/clj/orcpub/datomic.clj @@ -1,4 +1,9 @@ (ns orcpub.datomic + "Datomic database component with connection management and error handling. + + Provides a component that manages the database connection lifecycle, + including database creation, connection establishment, and schema initialization. + All operations include error handling with clear error messages." (:require [com.stuartsierra.component :as component] [datomic.api :as d] [orcpub.db.schema :as schema])) @@ -8,11 +13,37 @@ (start [this] (if (:conn this) this - (do + (try + (when (nil? uri) + (throw (ex-info "Database URI is required but not configured" + {:error :missing-db-uri}))) + + (println "Creating/connecting to Datomic database:" uri) (d/create-database uri) - (let [connection (d/connect uri)] - (d/transact connection schema/all-schemas) - (assoc this :conn connection))))) + + (let [connection (try + (d/connect uri) + (catch Exception e + (throw (ex-info "Failed to connect to Datomic database. Please verify the database URI and that Datomic is running." + {:error :db-connection-failed + :uri uri} + e))))] + (try + @(d/transact connection schema/all-schemas) + (println "Successfully initialized database schema") + (catch Exception e + (throw (ex-info "Failed to initialize database schema. The database may be in an inconsistent state." + {:error :schema-initialization-failed + :uri uri} + e)))) + (assoc this :conn connection)) + (catch clojure.lang.ExceptionInfo e + (throw e)) + (catch Exception e + (throw (ex-info "Unexpected error during database initialization" + {:error :db-init-failed + :uri uri} + e)))))) (stop [this] (assoc this :conn nil))) diff --git a/src/clj/orcpub/email.clj b/src/clj/orcpub/email.clj index 8af50a0ea..a84ce80e3 100644 --- a/src/clj/orcpub/email.clj +++ b/src/clj/orcpub/email.clj @@ -1,4 +1,9 @@ (ns orcpub.email + "Email sending functionality with error handling. + + Provides functions for sending verification emails, password reset emails, + and error notification emails. All operations include comprehensive error + handling to prevent silent failures when the SMTP server is unavailable." (:require [hiccup.core :as hiccup] [postal.core :as postal] [environ.core :as environ] @@ -28,26 +33,58 @@ :content (hiccup/html (verification-email-html first-and-last-name username verification-url))}]) (defn email-cfg [] - {:user (environ/env :email-access-key) - :pass (environ/env :email-secret-key) - :host (environ/env :email-server-url) - :port (Integer/parseInt (or (environ/env :email-server-port) "587")) - :ssl (or (str/to-bool (environ/env :email-ssl)) nil) - :tls (or (str/to-bool (environ/env :email-tls)) nil) - }) + (try + {:user (environ/env :email-access-key) + :pass (environ/env :email-secret-key) + :host (environ/env :email-server-url) + :port (Integer/parseInt (or (environ/env :email-server-port) "587")) + :ssl (or (str/to-bool (environ/env :email-ssl)) nil) + :tls (or (str/to-bool (environ/env :email-tls)) nil)} + (catch NumberFormatException e + (throw (ex-info "Invalid email server port configuration. Expected a number." + {:error :invalid-port + :port (environ/env :email-server-port)} + e))))) (defn emailfrom [] (if (not (s/blank? (environ/env :email-from-address))) (environ/env :email-from-address) (str "no-reply@orcpub.com"))) -(defn send-verification-email [base-url {:keys [email username first-and-last-name]} verification-key] - (postal/send-message (email-cfg) - {:from (str "OrcPub Team <" (emailfrom) ">") - :to email - :subject "OrcPub Email Verification" - :body (verification-email - first-and-last-name - username - (str base-url (routes/path-for routes/verify-route) "?key=" verification-key))})) +(defn send-verification-email + "Sends account verification email to a new user. + + Args: + base-url - Base URL for the application (for verification link) + user-map - Map containing :email, :username, and :first-and-last-name + verification-key - Unique key for email verification + + Returns: + Postal send-message result + + Throws: + ExceptionInfo with :verification-email-failed error code if email cannot be sent" + [base-url {:keys [email username first-and-last-name]} verification-key] + (try + (let [result (postal/send-message (email-cfg) + {:from (str "OrcPub Team <" (emailfrom) ">") + :to email + :subject "OrcPub Email Verification" + :body (verification-email + first-and-last-name + username + (str base-url (routes/path-for routes/verify-route) "?key=" verification-key))})] + (when (not= :SUCCESS (:error result)) + (throw (ex-info "Failed to send verification email" + {:error :email-send-failed + :email email + :postal-response result}))) + result) + (catch Exception e + (println "ERROR: Failed to send verification email to" email ":" (.getMessage e)) + (throw (ex-info "Unable to send verification email. Please check your email configuration or try again later." + {:error :verification-email-failed + :email email + :username username} + e))))) (defn reset-password-email-html [first-and-last-name reset-url] [:div @@ -72,24 +109,71 @@ [{:type "text/html" :content (hiccup/html (reset-password-email-html first-and-last-name reset-url))}]) -(defn send-reset-email [base-url {:keys [email username first-and-last-name]} reset-key] - (postal/send-message (email-cfg) - {:from (str "OrcPub Team <" (emailfrom) ">") - :to email - :subject "OrcPub Password Reset" - :body (reset-password-email - first-and-last-name - (str base-url (routes/path-for routes/reset-password-page-route) "?key=" reset-key))})) - -(defn send-error-email [context exception] - (if (not-empty (environ/env :email-errors-to)) - (postal/send-message (email-cfg) - {:from (str "OrcPub Errors <" (emailfrom) ">") - :to (str (environ/env :email-errors-to)) - :subject "Exception" - :body [{:type "text/plain" - :content (let [writer (java.io.StringWriter.)] - (clojure.pprint/pprint (:request context) writer) - (clojure.pprint/pprint (or (ex-data exception) exception) writer) - (str writer))}]}))) +(defn send-reset-email + "Sends password reset email to a user. + + Args: + base-url - Base URL for the application (for reset link) + user-map - Map containing :email, :username, and :first-and-last-name + reset-key - Unique key for password reset + + Returns: + Postal send-message result + + Throws: + ExceptionInfo with :reset-email-failed error code if email cannot be sent" + [base-url {:keys [email username first-and-last-name]} reset-key] + (try + (let [result (postal/send-message (email-cfg) + {:from (str "OrcPub Team <" (emailfrom) ">") + :to email + :subject "OrcPub Password Reset" + :body (reset-password-email + first-and-last-name + (str base-url (routes/path-for routes/reset-password-page-route) "?key=" reset-key))})] + (when (not= :SUCCESS (:error result)) + (throw (ex-info "Failed to send password reset email" + {:error :email-send-failed + :email email + :postal-response result}))) + result) + (catch Exception e + (println "ERROR: Failed to send password reset email to" email ":" (.getMessage e)) + (throw (ex-info "Unable to send password reset email. Please check your email configuration or try again later." + {:error :reset-email-failed + :email email + :username username} + e))))) + +(defn send-error-email + "Sends error notification email to configured admin email. + + This function is called when unhandled exceptions occur in the application. + It includes request context and exception details for debugging. + + Args: + context - Request context map + exception - The exception that occurred + + Returns: + Postal send-message result, or nil if no error email is configured + or if sending fails (failures are logged but not thrown)" + [context exception] + (when (not-empty (environ/env :email-errors-to)) + (try + (let [result (postal/send-message (email-cfg) + {:from (str "OrcPub Errors <" (emailfrom) ">") + :to (str (environ/env :email-errors-to)) + :subject "Exception" + :body [{:type "text/plain" + :content (let [writer (java.io.StringWriter.)] + (clojure.pprint/pprint (:request context) writer) + (clojure.pprint/pprint (or (ex-data exception) exception) writer) + (str writer))}]})] + (when (not= :SUCCESS (:error result)) + (println "WARNING: Failed to send error notification email:" (:error result))) + result) + (catch Exception e + (println "ERROR: Failed to send error notification email:" (.getMessage e)) + nil)))) diff --git a/src/clj/orcpub/pdf.clj b/src/clj/orcpub/pdf.clj index 50788de24..5172dd107 100644 --- a/src/clj/orcpub/pdf.clj +++ b/src/clj/orcpub/pdf.clj @@ -91,22 +91,61 @@ (def user-agent "Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.22 (KHTML, like Gecko) Chrome/25.0.1364.172") (defn draw-non-jpg [doc page url x y width height] - (with-open [c-stream (content-stream doc page)] - (let [buff-image (ImageIO/read (.getInputStream - (doto - (.openConnection (URL. url)) - (.setRequestProperty "User-Agent" user-agent)))) - img (LosslessFactory/createFromImage doc buff-image)] - (draw-imagex c-stream img x y width height)))) + (try + (with-open [c-stream (content-stream doc page)] + (let [connection (doto (.openConnection (URL. url)) + (.setRequestProperty "User-Agent" user-agent) + (.setConnectTimeout 10000) + (.setReadTimeout 10000)) + buff-image (ImageIO/read (.getInputStream connection))] + (when (nil? buff-image) + (throw (ex-info "Unable to read image from URL" + {:error :invalid-image-format + :url url}))) + (let [img (LosslessFactory/createFromImage doc buff-image)] + (draw-imagex c-stream img x y width height)))) + (catch java.net.SocketTimeoutException e + (throw (ex-info (str "Timeout loading image from URL: " url) + {:error :image-load-timeout + :url url} + e))) + (catch java.net.UnknownHostException e + (throw (ex-info (str "Unable to resolve host for image URL: " url) + {:error :unknown-host + :url url} + e))) + (catch Exception e + (throw (ex-info (str "Failed to load image from URL: " url) + {:error :image-load-failed + :url url} + e))))) (defn draw-jpg [doc page url x y width height] - (with-open [c-stream (content-stream doc page) - image-stream (.getInputStream - (doto - (.openConnection (URL. url)) - (.setRequestProperty "User-Agent" user-agent)))] - (let [img (JPEGFactory/createFromStream doc image-stream)] - (draw-imagex c-stream img x y width height)))) + (try + (with-open [c-stream (content-stream doc page) + image-stream (.getInputStream + (doto + (.openConnection (URL. url)) + (.setRequestProperty "User-Agent" user-agent) + (.setConnectTimeout 10000) + (.setReadTimeout 10000)))] + (let [img (JPEGFactory/createFromStream doc image-stream)] + (draw-imagex c-stream img x y width height))) + (catch java.net.SocketTimeoutException e + (throw (ex-info (str "Timeout loading image from URL: " url) + {:error :image-load-timeout + :url url} + e))) + (catch java.net.UnknownHostException e + (throw (ex-info (str "Unable to resolve host for image URL: " url) + {:error :unknown-host + :url url} + e))) + (catch Exception e + (throw (ex-info (str "Failed to load JPEG image from URL: " url) + {:error :jpeg-load-failed + :url url} + e))))) (defn draw-image! [doc page url x y width height] (let [lower-case-url (s/lower-case url) @@ -115,8 +154,16 @@ draw-fn (if jpg? draw-jpg draw-non-jpg)] (try (draw-fn doc page url x y width height) + (catch clojure.lang.ExceptionInfo e + (println "ERROR: Failed to load image for PDF:" (.getMessage e)) + (println " URL:" url) + (println " Details:" (ex-data e)) + nil) (catch Exception e - (prn "failed loading image" (clojure.stacktrace/print-stack-trace e)))))) + (println "ERROR: Unexpected error loading image for PDF:" (.getMessage e)) + (println " URL:" url) + (clojure.stacktrace/print-stack-trace e) + nil)))) (defn get-page [doc index] (.getPage doc index)) diff --git a/src/clj/orcpub/routes.clj b/src/clj/orcpub/routes.clj index a40390546..b92ccea1b 100644 --- a/src/clj/orcpub/routes.clj +++ b/src/clj/orcpub/routes.clj @@ -253,15 +253,21 @@ (defn do-verification [request params conn & [tx-data]] (let [verification-key (str (java.util.UUID/randomUUID)) now (java.util.Date.)] - @(d/transact + (try + @(d/transact conn [(merge tx-data {:orcpub.user/verified? false - :orcpub.user/verification-key verification-key - :orcpub.user/verification-sent now})]) + :orcpub.user/verification-key verification-key + :orcpub.user/verification-sent now})]) (send-verification-email request params verification-key) - {:status 200})) + {:status 200} + (catch Exception e + (println "ERROR: Failed to create verification record:" (.getMessage e)) + (throw (ex-info "Unable to complete registration. Please try again or contact support." + {:error :verification-failed} + e)))))) (defn register [{:keys [json-params db conn] :as request}] (let [{:keys [username email password send-updates?]} json-params @@ -341,17 +347,24 @@ (defn do-send-password-reset [user-id email conn request] (let [key (str (java.util.UUID/randomUUID))] - @(d/transact - conn - [{:db/id user-id - :orcpub.user/password-reset-key key - :orcpub.user/password-reset-sent (java.util.Date.)}]) - (email/send-reset-email - (base-url request) - {:first-and-last-name "OrcPub Patron" - :email email} - key) - {:status 200})) + (try + @(d/transact + conn + [{:db/id user-id + :orcpub.user/password-reset-key key + :orcpub.user/password-reset-sent (java.util.Date.)}]) + (email/send-reset-email + (base-url request) + {:first-and-last-name "OrcPub Patron" + :email email} + key) + {:status 200} + (catch Exception e + (println "ERROR: Failed to initiate password reset for user" user-id ":" (.getMessage e)) + (throw (ex-info "Unable to initiate password reset. Please try again or contact support." + {:error :password-reset-failed + :user-id user-id} + e)))))) (defn password-reset-expired? [password-reset-sent] (and password-reset-sent (t/before? (tc/from-date password-reset-sent) (-> 24 hours ago)))) @@ -373,13 +386,20 @@ (catch Throwable e (prn e) (throw e)))) (defn do-password-reset [conn user-id password] - @(d/transact - conn - [{:db/id user-id - :orcpub.user/password (hashers/encrypt (s/trim password)) - :orcpub.user/password-reset (java.util.Date.) - :orcpub.user/verified? true}]) - {:status 200}) + (try + @(d/transact + conn + [{:db/id user-id + :orcpub.user/password (hashers/encrypt (s/trim password)) + :orcpub.user/password-reset (java.util.Date.) + :orcpub.user/verified? true}]) + {:status 200} + (catch Exception e + (println "ERROR: Failed to reset password for user" user-id ":" (.getMessage e)) + (throw (ex-info "Unable to reset password. Please try again or contact support." + {:error :password-update-failed + :user-id user-id} + e))))) (defn reset-password [{:keys [json-params db conn cookies identity] :as request}] (try @@ -453,7 +473,12 @@ (catch Exception e (prn "FAILED ADDING SPELLS CARDS!" e)))) (defn character-pdf-2 [req] - (let [fields (-> req :form-params :body edn/read-string) + (let [fields (try + (-> req :form-params :body edn/read-string) + (catch Exception e + (throw (ex-info "Invalid character data format. Unable to parse PDF request." + {:error :invalid-pdf-data} + e)))) {:keys [image-url image-url-failed faction-image-url faction-image-url-failed spells-known custom-spells spell-save-dcs spell-attack-mods print-spell-cards? print-character-sheet-style? print-spell-card-dc-mod? character-name class-level player-name]} fields @@ -594,14 +619,21 @@ (-> result :tempids (get temp-id))) (defn create-entity [conn username entity owner-prop] - (as-> entity $ - (entity/remove-ids $) - (assoc $ - :db/id "tempid" - owner-prop username) - @(d/transact conn [$]) - (get-new-id "tempid" $) - (d/pull (d/db conn) '[*] $))) + (try + (as-> entity $ + (entity/remove-ids $) + (assoc $ + :db/id "tempid" + owner-prop username) + @(d/transact conn [$]) + (get-new-id "tempid" $) + (d/pull (d/db conn) '[*] $)) + (catch Exception e + (println "ERROR: Failed to create entity for user" username ":" (.getMessage e)) + (throw (ex-info "Unable to create entity. Please try again or contact support." + {:error :entity-creation-failed + :username username} + e))))) (defn email-for-username [db username] (d/q '[:find ?email . @@ -613,25 +645,35 @@ username)) (defn update-entity [conn username entity owner-prop] - (let [id (:db/id entity) - current (d/pull (d/db conn) '[*] id) - owner (get current owner-prop) - email (email-for-username (d/db conn) username)] - (if ((set [username email]) owner) - (let [current-ids (entity/db-ids current) - new-ids (entity/db-ids entity) - retract-ids (sets/difference current-ids new-ids) - retractions (map - (fn [retract-id] - [:db/retractEntity retract-id]) - retract-ids) - remove-ids (sets/difference new-ids current-ids) - with-ids-removed (entity/remove-specific-ids entity remove-ids) - new-entity (assoc with-ids-removed owner-prop username) - result @(d/transact conn (concat retractions [new-entity]))] - (d/pull (d/db conn) '[*] id)) - (throw (ex-info "Not user entity" - {:error :not-user-entity}))))) + (try + (let [id (:db/id entity) + current (d/pull (d/db conn) '[*] id) + owner (get current owner-prop) + email (email-for-username (d/db conn) username)] + (if ((set [username email]) owner) + (let [current-ids (entity/db-ids current) + new-ids (entity/db-ids entity) + retract-ids (sets/difference current-ids new-ids) + retractions (map + (fn [retract-id] + [:db/retractEntity retract-id]) + retract-ids) + remove-ids (sets/difference new-ids current-ids) + with-ids-removed (entity/remove-specific-ids entity remove-ids) + new-entity (assoc with-ids-removed owner-prop username) + result @(d/transact conn (concat retractions [new-entity]))] + (d/pull (d/db conn) '[*] id)) + (throw (ex-info "Not user entity" + {:error :not-user-entity})))) + (catch clojure.lang.ExceptionInfo e + (throw e)) + (catch Exception e + (println "ERROR: Failed to update entity for user" username ":" (.getMessage e)) + (throw (ex-info "Unable to update entity. Please try again or contact support." + {:error :entity-update-failed + :username username + :entity-id (:db/id entity)} + e))))) (defn save-entity [conn username e owner-prop] (let [without-empty-fields (entity/remove-empty-fields e)] @@ -703,14 +745,30 @@ (throw (ex-info "Not user character" {:error :not-user-character}))))) -(defn create-new-character [conn character username] - (let [result @(d/transact conn - [(-> character - (assoc :db/id "tempid" - ::se/owner username) - add-dnd-5e-character-tags)]) - new-id (get-new-id "tempid" result)] - (d/pull (d/db conn) '[*] new-id))) +(defn create-new-character + "Creates a new D&D 5e character. + + Args: + conn - Database connection + character - Character data map + username - Owner username + + Returns: + Created character entity + + Throws: + ExceptionInfo on database failure" + [conn character username] + (errors/with-db-error-handling :character-creation-failed + {:username username} + "Unable to create character. Please try again or contact support." + (let [result @(d/transact conn + [(-> character + (assoc :db/id "tempid" + ::se/owner username) + add-dnd-5e-character-tags)]) + new-id (get-new-id "tempid" result)] + (d/pull (d/db conn) '[*] new-id)))) (defn clean-up-character [character] (if (-> character ::se/values ::char5e/xps string?) @@ -764,10 +822,23 @@ :body item} {:status 404}))) -(defn delete-item [{:keys [db conn username] {:keys [:id]} :path-params}] +(defn delete-item + "Deletes a magic item owned by the user. + + Args: + request - HTTP request with item ID + + Returns: + HTTP 200 on success, 401 if not owned + + Throws: + ExceptionInfo on database failure" + [{:keys [db conn username] {:keys [:id]} :path-params}] (let [{:keys [::mi5e/owner]} (d/pull db '[::mi5e/owner] id)] (if (= username owner) - (do + (errors/with-db-error-handling :item-deletion-failed + {:item-id id} + "Unable to delete item. Please try again or contact support." @(d/transact conn [[:db/retractEntity id]]) {:status 200}) {:status 401}))) @@ -822,29 +893,73 @@ results)] {:status 200 :body characters})) -(defn follow-user [{:keys [db conn identity] {:keys [user]} :path-params}] +(defn follow-user + "Adds a user to the authenticated user's following list. + + Args: + request - HTTP request with username to follow + + Returns: + HTTP 200 on success + + Throws: + ExceptionInfo on database failure" + [{:keys [db conn identity] {:keys [user]} :path-params}] (let [other-user-id (user-id-for-username db user) username (:user identity) user-id (user-id-for-username db username)] - @(d/transact conn [{:db/id user-id - :orcpub.user/following other-user-id}]) - {:status 200})) + (errors/with-db-error-handling :follow-user-failed + {:follower username :followed user} + "Unable to follow user. Please try again or contact support." + @(d/transact conn [{:db/id user-id + :orcpub.user/following other-user-id}]) + {:status 200}))) + +(defn unfollow-user + "Removes a user from the authenticated user's following list. + + Args: + request - HTTP request with username to unfollow + + Returns: + HTTP 200 on success -(defn unfollow-user [{:keys [db conn identity] {:keys [user]} :path-params}] + Throws: + ExceptionInfo on database failure" + [{:keys [db conn identity] {:keys [user]} :path-params}] (let [other-user-id (user-id-for-username db user) username (:user identity) user-id (user-id-for-username db username)] - @(d/transact conn [[:db/retract user-id :orcpub.user/following other-user-id]]) - {:status 200})) - -(defn delete-character [{:keys [db conn identity] {:keys [id]} :path-params}] - (let [parsed-id (Long/parseLong id) + (errors/with-db-error-handling :unfollow-user-failed + {:follower username :unfollowed user} + "Unable to unfollow user. Please try again or contact support." + @(d/transact conn [[:db/retract user-id :orcpub.user/following other-user-id]]) + {:status 200}))) + +(defn delete-character + "Deletes a character owned by the authenticated user. + + Args: + request - HTTP request with character ID in path params + + Returns: + HTTP 200 on success, 400 for problems, 401 if not owned + + Throws: + ExceptionInfo on invalid ID or database failure" + [{:keys [db conn identity] {:keys [id]} :path-params}] + (let [parsed-id (errors/with-validation :invalid-character-id + {:id id} + "Invalid character ID format" + (Long/parseLong id)) username (:user identity) character (d/pull db '[*] parsed-id) problems [] #_(dnd-e5-char-type-problems character)] (if (owns-entity? db username parsed-id) (if (empty? problems) - (do + (errors/with-db-error-handling :character-deletion-failed + {:character-id parsed-id} + "Unable to delete character. Please try again or contact support." @(d/transact conn [[:db/retractEntity parsed-id]]) {:status 200}) {:status 400 :body problems}) @@ -860,8 +975,22 @@ (defn character-summary-for-id [db id] {:keys [::se/summary]} (d/pull db '[::se/summary {::se/values [::char5e/description ::char5e/image-url]}] id)) -(defn get-character [{:keys [db] {:keys [:id]} :path-params}] - (let [parsed-id (Long/parseLong id)] +(defn get-character + "Retrieves a character by ID. + + Args: + request - HTTP request with character ID in path params + + Returns: + HTTP response with character data + + Throws: + ExceptionInfo on invalid ID format" + [{:keys [db] {:keys [:id]} :path-params}] + (let [parsed-id (errors/with-validation :invalid-character-id + {:id id} + "Invalid character ID format" + (Long/parseLong id))] (get-character-for-id db parsed-id))) (defn get-user [{:keys [db identity]}] @@ -869,15 +998,29 @@ user (find-user-by-username-or-email db username)] {:status 200 :body (user-body db user)})) -(defn delete-user [{:keys [db conn identity]}] +(defn delete-user + "Deletes the authenticated user's account. + + Args: + request - HTTP request with authenticated user identity + + Returns: + HTTP 200 on success + + Throws: + ExceptionInfo on database failure" + [{:keys [db conn identity]}] (let [username (:user identity) user (d/q '[:find ?u . :in $ ?username :where [?u :orcpub.user/username ?username]] db username)] - @(d/transact conn [[:db/retractEntity user]]) - {:status 200})) + (errors/with-db-error-handling :user-deletion-failed + {:username username} + "Unable to delete user account. Please try again or contact support." + @(d/transact conn [[:db/retractEntity user]]) + {:status 200}))) (defn character-summary-description [{:keys [::char5e/race-name ::char5e/subrace-name ::char5e/classes]}] (str race-name diff --git a/src/clj/orcpub/routes/party.clj b/src/clj/orcpub/routes/party.clj index a7d120a19..751960a89 100644 --- a/src/clj/orcpub/routes/party.clj +++ b/src/clj/orcpub/routes/party.clj @@ -1,14 +1,35 @@ (ns orcpub.routes.party + "HTTP route handlers for party management operations. + + Provides CRUD operations for D&D parties with error handling." (:require [clojure.spec.alpha :as spec] [datomic.api :as d] [orcpub.dnd.e5.party :as party] - [orcpub.entity.strict :as se])) + [orcpub.entity.strict :as se] + [orcpub.errors :as errors])) + +(defn create-party + "Creates a new party owned by the authenticated user. + + Args: + request - HTTP request map with: + :conn - Database connection + :identity - Authenticated user identity + :transit-params - Party data -(defn create-party [{:keys [db conn identity] party :transit-params}] - (let [username (:user identity) - result @(d/transact conn [(assoc party ::party/owner username)]) - new-id (-> result :tempids first val)] - {:status 200 :body (d/pull (d/db conn) '[*] new-id)})) + Returns: + HTTP response with created party data + + Throws: + ExceptionInfo on database failure with :party-creation-failed error code" + [{:keys [db conn identity] party :transit-params}] + (let [username (:user identity)] + (errors/with-db-error-handling :party-creation-failed + {:username username} + "Unable to create party. Please try again or contact support." + (let [result @(d/transact conn [(assoc party ::party/owner username)]) + new-id (-> result :tempids first val)] + {:status 200 :body (d/pull (d/db conn) '[*] new-id)})))) (def pull-party [:db/id ::party/name {::party/character-ids [:db/id ::se/owner ::se/summary]}]) @@ -35,27 +56,74 @@ {:status 200 :body mapped})) -(defn update-party-name [{:keys [db conn identity] - party-name :transit-params - {:keys [id]} :path-params}] - @(d/transact conn [{:db/id id - ::party/name party-name}]) - {:status 200 - :body (d/pull (d/db conn) pull-party id)}) +(defn update-party-name + "Updates a party's name. + + Args: + request - HTTP request with party name and party ID + + Returns: + HTTP response with updated party data + + Throws: + ExceptionInfo on database failure with :party-update-failed error code" + [{:keys [db conn identity] + party-name :transit-params + {:keys [id]} :path-params}] + (errors/with-db-error-handling :party-update-failed + {:party-id id} + "Unable to update party name. Please try again or contact support." + @(d/transact conn [{:db/id id + ::party/name party-name}]) + {:status 200 + :body (d/pull (d/db conn) pull-party id)})) (defn add-character [{:keys [db conn identity] character-id :transit-params {:keys [id]} :path-params}] - @(d/transact conn [{:db/id id - ::party/character-ids character-id}]) - {:status 200 :body (d/pull db '[*] id)}) + (try + @(d/transact conn [{:db/id id + ::party/character-ids character-id}]) + {:status 200 :body (d/pull db '[*] id)} + (catch Exception e + (println "ERROR: Failed to add character" character-id "to party" id ":" (.getMessage e)) + (throw (ex-info "Unable to add character to party. Please try again or contact support." + {:error :party-add-character-failed + :party-id id + :character-id character-id} + e))))) + +(defn remove-character + "Removes a character from a party. + + Args: + request - HTTP request with party ID and character ID + + Returns: + HTTP response with updated party data -(defn remove-character [{:keys [db conn identity] - {:keys [id character-id]} :path-params}] - @(d/transact conn [[:db/retract id ::party/character-ids (Long/parseLong character-id)]]) - {:status 200 :body (d/pull db '[*] id)}) + Throws: + ExceptionInfo on invalid character ID or database failure" + [{:keys [db conn identity] + {:keys [id character-id]} :path-params}] + (let [char-id (errors/with-validation :invalid-character-id + {:character-id character-id} + "Invalid character ID format" + (Long/parseLong character-id))] + (errors/with-db-error-handling :party-remove-character-failed + {:party-id id :character-id char-id} + "Unable to remove character from party. Please try again or contact support." + @(d/transact conn [[:db/retract id ::party/character-ids char-id]]) + {:status 200 :body (d/pull db '[*] id)}))) (defn delete-party [{:keys [db conn identity] {:keys [id]} :path-params}] - @(d/transact conn [[:db/retractEntity id]]) - {:status 200}) + (try + @(d/transact conn [[:db/retractEntity id]]) + {:status 200} + (catch Exception e + (println "ERROR: Failed to delete party" id ":" (.getMessage e)) + (throw (ex-info "Unable to delete party. Please try again or contact support." + {:error :party-deletion-failed + :party-id id} + e))))) diff --git a/src/clj/orcpub/system.clj b/src/clj/orcpub/system.clj index 0345e86a6..3101128de 100644 --- a/src/clj/orcpub/system.clj +++ b/src/clj/orcpub/system.clj @@ -22,7 +22,14 @@ {::http/routes routes/routes ::http/type :jetty ::http/port (let [port-str (System/getenv "PORT")] - (when port-str (Integer/parseInt port-str))) + (when port-str + (try + (Integer/parseInt port-str) + (catch NumberFormatException e + (throw (ex-info "Invalid PORT environment variable. Expected a number." + {:error :invalid-port + :port port-str} + e)))))) ::http/join false ::http/resource-path "/public" ::http/container-options {:context-configurator (fn [c] diff --git a/src/cljc/orcpub/common.cljc b/src/cljc/orcpub/common.cljc index 16192ec04..3660c72ef 100644 --- a/src/cljc/orcpub/common.cljc +++ b/src/cljc/orcpub/common.cljc @@ -179,18 +179,11 @@ ;; Case Insensitive `sort-by` (defn aloof-sort-by [sorter coll] - (sort-by (fn [x] - (let [v (sorter x)] - (cond - (string? v) (s/lower-case v) - (nil? v) "" - :else (s/lower-case (str v))))) - coll) + (sort-by (comp s/lower-case sorter) coll) ) (defn ->kebab-case [s] - (when (string? s) - (-> s - ;; Insert hyphen before each capital letter, but not at the start. - (s/replace #"([A-Z])" "-$1") - (s/lower-case)))) + (-> s + ;; Insert hyphen before each capital letter, but not at the start. + (s/replace #"([A-Z])" "-$1") + .toLowerCase)) diff --git a/src/cljc/orcpub/errors.cljc b/src/cljc/orcpub/errors.cljc index c5591f29e..7199a26d0 100644 --- a/src/cljc/orcpub/errors.cljc +++ b/src/cljc/orcpub/errors.cljc @@ -1,9 +1,162 @@ -(ns orcpub.errors) +(ns orcpub.errors + "Error handling utilities and error code constants. + This namespace provides: + - Error code constants for application-level errors + - Reusable error handling utilities for common operations + - Consistent error logging and exception creation") + +;; Error code constants (def bad-credentials :bad-credentials) (def unverified :unverified) (def unverified-expired :unverified-expired) (def no-account :no-account) (def username-required :username-required) (def password-required :password-required) -(def too-many-attempts :too-many-attempts) \ No newline at end of file +(def too-many-attempts :too-many-attempts) + +;; Error handling utilities + +(defn log-error + "Logs an error message with optional context data. + + Args: + prefix - A prefix string (e.g., 'ERROR:', 'WARNING:') + message - The error message + context - Optional map of context data to log + + Example: + (log-error \"ERROR\" \"Failed to save\" {:user-id 123})" + ([prefix message] + (println prefix message)) + ([prefix message context] + (println prefix message) + (when (seq context) + (println " Context:" context)))) + +(defn create-error + "Creates a structured exception with ex-info. + + Args: + user-msg - User-friendly error message + error-code - Keyword identifying the error type + context - Map of additional context data + cause - Optional underlying exception + + Returns: + An ExceptionInfo with structured data + + Example: + (create-error \"Unable to save\" :save-failed {:id 123} original-exception)" + ([user-msg error-code context] + (ex-info user-msg (assoc context :error error-code))) + ([user-msg error-code context cause] + (ex-info user-msg (assoc context :error error-code) cause))) + +(defn with-error-handling* + "Core function for wrapping operations with error handling. + + This is typically not called directly - use the macro versions instead. + + Args: + operation-fn - Zero-arg function to execute + opts - Map with keys: + :operation-name - Name for logging (e.g., 'database transaction') + :user-message - Message shown to users on failure + :error-code - Keyword for the error type + :context - Additional context data + :on-error - Optional function called with exception + + Returns: + Result of operation-fn, or re-throws with structured error" + [operation-fn {:keys [operation-name user-message error-code context on-error]}] + (try + (operation-fn) + (catch #?(:clj clojure.lang.ExceptionInfo :cljs ExceptionInfo) e + ;; Re-throw ExceptionInfo as-is (already structured) + (throw e)) + (catch #?(:clj Exception :cljs :default) e + (log-error "ERROR:" (str "Failed " operation-name ":") + (merge context {:message (.getMessage e)})) + (when on-error + (on-error e)) + (throw (create-error user-message error-code context e))))) + +#?(:clj + (defmacro with-db-error-handling + "Wraps database operations with consistent error handling. + + Automatically logs errors and creates user-friendly exceptions. + + Args: + error-code - Keyword identifying the error type + context - Map of context data (will be in error's ex-data) + user-message - User-friendly error message + & body - Code to execute + + Example: + (with-db-error-handling :user-creation-failed + {:username \"alice\"} + \"Unable to create user. Please try again.\" + @(d/transact conn [{:db/id \"temp\" :user/name \"alice\"}]))" + [error-code context user-message & body] + `(with-error-handling* + (fn [] ~@body) + {:operation-name "database operation" + :user-message ~user-message + :error-code ~error-code + :context ~context}))) + +#?(:clj + (defmacro with-email-error-handling + "Wraps email operations with consistent error handling. + + Automatically logs errors and creates user-friendly exceptions. + + Args: + error-code - Keyword identifying the error type + context - Map of context data (e.g., {:email \"user@example.com\"}) + user-message - User-friendly error message + & body - Code to execute + + Example: + (with-email-error-handling :verification-email-failed + {:email user-email :username username} + \"Unable to send verification email.\" + (postal/send-message config message))" + [error-code context user-message & body] + `(with-error-handling* + (fn [] ~@body) + {:operation-name "email operation" + :user-message ~user-message + :error-code ~error-code + :context ~context}))) + +#?(:clj + (defmacro with-validation + "Wraps parsing/validation operations with error handling. + + Specifically handles NumberFormatException and other parsing errors. + + Args: + error-code - Keyword identifying the error type + context - Map of context data + user-message - User-friendly error message + & body - Code to execute + + Example: + (with-validation :invalid-id + {:id-string \"abc\"} + \"Invalid ID format.\" + (Long/parseLong id-string))" + [error-code context user-message & body] + `(try + ~@body + (catch NumberFormatException e# + (log-error "ERROR:" "Validation failed:" ~context) + (throw (create-error ~user-message ~error-code ~context e#))) + (catch clojure.lang.ExceptionInfo e# + (throw e#)) + (catch Exception e# + (log-error "ERROR:" "Validation failed:" ~context) + (throw (create-error ~user-message ~error-code ~context e#)))))) \ No newline at end of file diff --git a/src/cljs/orcpub/dnd/e5/events.cljs b/src/cljs/orcpub/dnd/e5/events.cljs index 687655c69..aafb4c5cf 100644 --- a/src/cljs/orcpub/dnd/e5/events.cljs +++ b/src/cljs/orcpub/dnd/e5/events.cljs @@ -8,6 +8,7 @@ [orcpub.dnd.e5 :as e5] [orcpub.dnd.e5.template :as t5e] [orcpub.dnd.e5.common :as common5e] + [orcpub.dnd.e5.import-validation :as import-val] [orcpub.dnd.e5.character :as char5e] [orcpub.dnd.e5.char-decision-tree :as char-dec5e] [orcpub.dnd.e5.backgrounds :as bg5e] @@ -3161,20 +3162,66 @@ (reg-event-fx ::e5/export-plugin (fn [_ [_ name plugin]] - (let [blob (js/Blob. - (clj->js [(str plugin)]) - (clj->js {:type "text/plain;charset=utf-8"}))] - (js/saveAs blob (str name ".orcbrew")) - {}))) + ;; Validate before export to catch bugs early + (let [validation (import-val/validate-before-export plugin)] + (if (:valid validation) + (do + ;; Log warnings if any + (when (seq (:warnings validation)) + (js/console.warn "Export warnings for" name ":") + (doseq [warning (:warnings validation)] + (js/console.warn " " warning))) + + ;; Proceed with export + (let [blob (js/Blob. + (clj->js [(str plugin)]) + (clj->js {:type "text/plain;charset=utf-8"}))] + (js/saveAs blob (str name ".orcbrew")) + (if (seq (:warnings validation)) + {:dispatch [:show-warning-message + (str "Plugin '" name "' exported with warnings. Check console for details.")]} + {}))) + + ;; Validation failed - don't export + (do + (js/console.error "Export validation failed for" name ":") + (js/console.error (:errors validation)) + {:dispatch [:show-error-message + (str "Cannot export '" name "' - contains invalid data. Check console for details.")]}))))) (reg-event-fx ::e5/export-all-plugins (fn [_ _] - (let [blob (js/Blob. - (clj->js [(str @(subscribe [::e5/plugins]))]) - (clj->js {:type "text/plain;charset=utf-8"}))] - (js/saveAs blob (str "all-content.orcbrew")) - {}))) + (let [all-plugins @(subscribe [::e5/plugins]) + ;; Validate each plugin + validations (into {} + (map (fn [[name plugin]] + [name (import-val/validate-before-export plugin)]) + all-plugins)) + has-errors (some (fn [[_ v]] (not (:valid v))) validations) + has-warnings (some (fn [[_ v]] (seq (:warnings v))) validations)] + + (when (or has-errors has-warnings) + (js/console.warn "Export validation results:") + (doseq [[name validation] validations] + (when-not (:valid validation) + (js/console.error "Plugin" name "has errors:" (:errors validation))) + (when (seq (:warnings validation)) + (js/console.warn "Plugin" name "has warnings:" (:warnings validation))))) + + (if has-errors + {:dispatch [:show-error-message + "Cannot export all plugins - some contain invalid data. Check console for details."]} + + (do + (let [blob (js/Blob. + (clj->js [(str all-plugins)]) + (clj->js {:type "text/plain;charset=utf-8"}))] + (js/saveAs blob (str "all-content.orcbrew")) + (if has-warnings + {:dispatch [:show-warning-message + "All plugins exported with some warnings. Check console for details."]} + {}))))))) (reg-event-fx ::e5/export-plugin-pretty-print @@ -3208,7 +3255,10 @@ (fn [{:keys [db]} [_ plugin-name type-key key]] {:dispatch [::e5/set-plugins (-> db :plugins (update-in [plugin-name type-key key :disabled?] not))]})) -(defn clean-plugin-errors [plugin-text] +(defn clean-plugin-errors + "DEPRECATED: Use import-validation/validate-import instead. + Kept for backward compatibility only." + [plugin-text] (-> plugin-text (clojure.string/replace #"disabled\?\s+nil" "disabled? false") ; disabled? nil - replace w/disabled? false (clojure.string/replace #"(?m)nil nil, " "") ; nil nil, - find+remove @@ -3220,31 +3270,75 @@ (reg-event-fx ::e5/import-plugin (fn [{:keys [db]} [_ plugin-name plugin-text]] - (let [cleaned-plugin-text (clean-plugin-errors plugin-text) - plugin (try - (reader/read-string cleaned-plugin-text) - (catch js/Error e nil))] - (cond - (spec/valid? ::e5/plugin plugin) - {:dispatch-n [[::e5/set-plugins (assoc (:plugins db) - plugin-name - plugin)] - [:show-warning-message (str "File imported as '" plugin-name "'. To be safe, you should 'Export All' and save to a safe location now.")]]} - - (spec/valid? ::e5/plugins plugin) - {:dispatch-n [[::e5/set-plugins (e5/merge-all-plugins - (:plugins db) - plugin)] - [:show-warning-message "Imported content was merged into your existing content. To be safe, you should 'Export All' and save to a safe location now."]]} - - :else + ;; Use comprehensive validation with progressive import strategy + (let [result (import-val/validate-import plugin-text {:strategy :progressive + :auto-clean true}) + user-message (import-val/format-import-result result)] + + ;; Log detailed results to console for debugging + (js/console.log "Import validation result:" (clj->js result)) + + (cond + ;; Parse error - cannot recover + (:parse-error result) (do - (prn "PLUGIN" plugin) - (prn "INVALID PLUGINS FILE" - (spec/explain-data ::e5/plugins plugin)) - (prn "INVALID PLUGIN FILE" - (spec/explain-data ::e5/plugin plugin)) - {:dispatch [:show-error-message "Invalid .orcbrew file"]}))))) + (js/console.error "Parse error:" (:error result)) + {:dispatch [:show-error-message user-message]}) + + ;; Validation failed completely + (and (not (:success result)) (:errors result)) + (do + (js/console.error "Validation errors:" (clj->js (:errors result))) + {:dispatch [:show-error-message user-message]}) + + ;; Progressive import succeeded (may have skipped some items) + (:success result) + (let [plugin (:data result) + is-multi-plugin (and (spec/valid? ::e5/plugins plugin) + (not (spec/valid? ::e5/plugin plugin)))] + + ;; Log skipped items if any + (when (:had-errors result) + (js/console.warn "Skipped invalid items:") + (doseq [item (:skipped-items result)] + (js/console.warn " " (:key item)) + (js/console.warn " Errors:" (:errors item)))) + + {:dispatch-n (cond-> [] + ;; Set the plugins + true + (conj (if is-multi-plugin + [::e5/set-plugins (e5/merge-all-plugins (:plugins db) plugin)] + [::e5/set-plugins (assoc (:plugins db) plugin-name plugin)])) + + ;; Show appropriate message + true + (conj (if (:had-errors result) + [:show-warning-message user-message] + [:show-warning-message user-message])))}) + + ;; Unknown state + :else + {:dispatch [:show-error-message "Unknown import error. Check console for details."]})))) + +;; Add a strict import option for users who want all-or-nothing behavior +(reg-event-fx + ::e5/import-plugin-strict + (fn [{:keys [db]} [_ plugin-name plugin-text]] + (let [result (import-val/validate-import plugin-text {:strategy :strict + :auto-clean true}) + user-message (import-val/format-import-result result)] + + (js/console.log "Strict import validation result:" (clj->js result)) + + (if (:success result) + (let [plugin (:data result)] + {:dispatch-n [[::e5/set-plugins (if (= :multi-plugin (:strategy result)) + (e5/merge-all-plugins (:plugins db) plugin) + (assoc (:plugins db) plugin-name plugin))] + [:show-warning-message user-message]]}) + + {:dispatch [:show-error-message user-message]})))) (reg-event-db ::spells/set-spell diff --git a/src/cljs/orcpub/dnd/e5/import_validation.cljs b/src/cljs/orcpub/dnd/e5/import_validation.cljs new file mode 100644 index 000000000..f2b378fef --- /dev/null +++ b/src/cljs/orcpub/dnd/e5/import_validation.cljs @@ -0,0 +1,428 @@ +(ns orcpub.dnd.e5.import-validation + "Comprehensive validation for orcbrew file import/export. + + Provides detailed error messages and progressive validation to help users + identify and fix issues with their orcbrew files." + (:require [cljs.spec.alpha :as spec] + [cljs.reader :as reader] + [clojure.string :as str] + [orcpub.dnd.e5 :as e5] + [orcpub.common :as common])) + +;; ============================================================================ +;; Error Message Formatting +;; ============================================================================ + +(defn format-spec-problem + "Converts a spec problem into a human-readable error message." + [{:keys [path pred val via in]}] + (let [location (if (seq in) + (str "at " (str/join " > " (map name in))) + "at root")] + (str " • " location ": " + (cond + (and (seq? pred) (= 'clojure.core/fn (first pred))) + "Invalid value format" + + (and (seq? pred) (= 'clojure.spec.alpha/keys (first pred))) + (str "Missing required field: " (second pred)) + + :else + (str "Failed validation: " pred)) + (when val + (str "\n Got: " (pr-str (if (> (count (pr-str val)) 50) + (str (subs (pr-str val) 0 47) "...") + val))))))) + +(defn format-validation-errors + "Formats spec validation errors into user-friendly messages." + [explain-data] + (when explain-data + (let [problems (:cljs.spec.alpha/problems explain-data)] + (str "Validation errors found:\n" + (str/join "\n" (map format-spec-problem problems)))))) + +;; ============================================================================ +;; Parse Error Detection +;; ============================================================================ + +(defn parse-edn + "Attempts to parse EDN text with detailed error reporting. + + Returns: + {:success true :data } on success + {:success false :error :line } on failure" + [edn-text] + (try + (let [result (reader/read-string edn-text)] + {:success true :data result}) + (catch js/Error e + (let [msg (.-message e) + line-match (re-find #"line (\d+)" msg) + line-num (when line-match (js/parseInt (second line-match)))] + {:success false + :error msg + :line line-num + :hint (cond + (str/includes? msg "Unmatched delimiter") + "Check for missing or extra brackets/braces/parentheses" + + (str/includes? msg "EOF") + "File appears to be incomplete or corrupted" + + (str/includes? msg "Invalid token") + "File contains invalid characters or syntax" + + :else + "Check the file syntax and ensure it's valid EDN format")})))) + +;; ============================================================================ +;; Progressive Validation +;; ============================================================================ + +(defn validate-item + "Validates a single homebrew item. + + Returns: + {:valid true} if valid + {:valid false :errors [...]} if invalid" + [item-key item] + (if (spec/valid? ::e5/homebrew-item item) + {:valid true} + {:valid false + :item-key item-key + :errors (format-validation-errors (spec/explain-data ::e5/homebrew-item item))})) + +(defn validate-content-group + "Validates a group of homebrew content (e.g., all spells, all races). + + Returns map of: + :valid-count - number of valid items + :invalid-count - number of invalid items + :invalid-items - vector of {:key :errors } for invalid items" + [content-key items] + (let [results (map (fn [[k v]] (assoc (validate-item k v) :key k)) items) + valid (filter :valid results) + invalid (remove :valid results)] + {:content-type content-key + :valid-count (count valid) + :invalid-count (count invalid) + :invalid-items (mapv #(select-keys % [:key :errors]) invalid)})) + +(defn validate-plugin-progressive + "Validates a plugin progressively, identifying which specific items are invalid. + + Returns map with: + :valid - true if entire plugin is valid + :content-groups - validation results for each content type + :valid-items-count - total valid items + :invalid-items-count - total invalid items" + [plugin] + (let [content-groups (filter + (fn [[k _]] (and (qualified-keyword? k) + (= (namespace k) "orcpub.dnd.e5"))) + plugin) + validations (mapv + (fn [[k v]] + (if (map? v) + (validate-content-group k v) + {:content-type k :valid-count 1 :invalid-count 0 :invalid-items []})) + content-groups) + total-valid (reduce + 0 (map :valid-count validations)) + total-invalid (reduce + 0 (map :invalid-count validations))] + {:valid (zero? total-invalid) + :content-groups validations + :valid-items-count total-valid + :invalid-items-count total-invalid})) + +;; ============================================================================ +;; Pre-Export Validation +;; ============================================================================ + +(defn validate-before-export + "Validates plugin data before export to catch bugs early. + + Returns: + {:valid true :warnings [...]} if exportable + {:valid false :errors [...]} if not exportable" + [plugin-data] + (let [warnings (atom [])] + + ;; Check for nil values + (doseq [[k v] plugin-data] + (when (nil? v) + (swap! warnings conj (str "Found nil value for key: " k)))) + + ;; Check for empty option-pack strings + (doseq [[content-key items] plugin-data] + (when (and (qualified-keyword? content-key) (map? items)) + (doseq [[item-key item] items] + (when (and (map? item) + (or (nil? (:option-pack item)) + (= "" (:option-pack item)))) + (swap! warnings conj + (str "Item " (name content-key) "/" (name item-key) + " has missing option-pack")))))) + + ;; Run full spec validation + (if (spec/valid? ::e5/plugin plugin-data) + {:valid true + :warnings @warnings} + {:valid false + :errors (format-validation-errors (spec/explain-data ::e5/plugin plugin-data)) + :warnings @warnings}))) + +;; ============================================================================ +;; Import Strategies +;; ============================================================================ + +(defn import-all-or-nothing + "Traditional import: all content must be valid or none is imported." + [plugin] + (cond + (spec/valid? ::e5/plugin plugin) + {:success true + :strategy :single-plugin + :data plugin} + + (spec/valid? ::e5/plugins plugin) + {:success true + :strategy :multi-plugin + :data plugin} + + :else + {:success false + :errors [(str "Invalid plugin structure\n\n" + (format-validation-errors (spec/explain-data ::e5/plugin plugin)) + "\n\nIf this is a multi-plugin file:\n" + (format-validation-errors (spec/explain-data ::e5/plugins plugin)))]})) + +(defn remove-invalid-items + "Removes invalid items from a content group, keeping only valid ones." + [content-key items] + (into {} + (filter (fn [[k v]] + (:valid (validate-item k v))) + items))) + +(defn import-progressive + "Progressive import: imports valid items and reports invalid ones. + + Returns: + {:success true + :data + :imported-count + :skipped-count + :skipped-items [...]} + + This allows users to recover as much data as possible from corrupted files." + [plugin] + (if (map? plugin) + (let [validation (validate-plugin-progressive plugin) + cleaned-plugin (into {} + (map (fn [[k v]] + (if (and (qualified-keyword? k) (map? v)) + [k (remove-invalid-items k v)] + [k v])) + plugin)) + invalid-items (mapcat :invalid-items (:content-groups validation))] + {:success true + :data cleaned-plugin + :imported-count (:valid-items-count validation) + :skipped-count (:invalid-items-count validation) + :skipped-items invalid-items + :had-errors (pos? (:invalid-items-count validation))}) + {:success false + :errors ["Plugin is not a valid map structure"]})) + +;; ============================================================================ +;; Data-Level Cleaning (after parse) +;; ============================================================================ + +;; Fields where nil should be replaced with a default value +(def nil-replace-defaults + {:disabled? false + :option-pack "Unnamed Content"}) + +;; Fields where nil is semantically meaningful and should be preserved +(def nil-preserve-fields + #{:spell-list-kw :spellcasting :ability :class-key}) + +;; Fields where nil should be removed entirely (inside nested maps) +;; These are typically numeric fields where nil is accidental +(def nil-remove-in-maps + #{:str :dex :con :int :wis :cha ; ability scores + :ac :hp :speed ; stats + :level :modifier :die :die-count}) ; numeric fields + +(defn clean-nil-in-map + "Removes nil values for specific keys in a map." + [m] + (if (map? m) + (into {} + (keep (fn [[k v]] + (cond + ;; Replace with default if in replace list + (and (nil? v) (contains? nil-replace-defaults k)) + [k (get nil-replace-defaults k)] + + ;; Preserve nil if in preserve list + (and (nil? v) (contains? nil-preserve-fields k)) + [k v] + + ;; Remove nil if in remove list + (and (nil? v) (contains? nil-remove-in-maps k)) + nil + + ;; Recurse into nested maps/vectors + (map? v) + [k (clean-nil-in-map v)] + + (vector? v) + [k (mapv #(if (map? %) (clean-nil-in-map %) %) v)] + + ;; Keep everything else as-is + :else + [k v])) + m)) + m)) + +(defn fix-empty-option-pack + "Fixes empty string option-pack values in items." + [data] + (if (map? data) + (into {} + (map (fn [[k v]] + (cond + ;; Fix empty option-pack in homebrew items + (and (= k :option-pack) (= v "")) + [k "Unnamed Content"] + + ;; Recurse into nested structures + (map? v) + [k (fix-empty-option-pack v)] + + (vector? v) + [k (mapv #(if (map? %) (fix-empty-option-pack %) %) v)] + + :else + [k v])) + data)) + data)) + +(defn rename-empty-plugin-key + "Renames empty string plugin key to a unique name." + [data] + (if (and (map? data) (contains? data "")) + (let [base-name "Unnamed Content" + ;; Find a unique name if base-name already exists + unique-name (if (contains? data base-name) + (loop [n 2] + (let [candidate (str base-name " " n)] + (if (contains? data candidate) + (recur (inc n)) + candidate))) + base-name)] + (-> data + (assoc unique-name (get data "")) + (dissoc ""))) + data)) + +(defn clean-data + "Applies all data-level cleaning transformations." + [data] + (-> data + rename-empty-plugin-key + fix-empty-option-pack + clean-nil-in-map)) + +;; ============================================================================ +;; Main Validation Entry Point +;; ============================================================================ + +(defn validate-import + "Main validation function for orcbrew file imports. + + Options: + :strategy - :strict (all-or-nothing) or :progressive (import valid items) + :auto-clean - whether to apply automatic cleaning fixes + + Returns detailed validation results with user-friendly error messages." + [edn-text {:keys [strategy auto-clean] :or {strategy :progressive auto-clean true}}] + + ;; Step 1: String-level cleaning (syntax fixes only) + (let [cleaned-text (if auto-clean + (-> edn-text + ;; Fix disabled? nil -> disabled? false (common toggle artifact) + (str/replace #"disabled\?\s+nil" "disabled? false") + ;; Clean up trailing commas before closing braces/brackets + (str/replace #",\s*\}" "}") + (str/replace #",\s*\]" "]")) + edn-text)] + + ;; Step 2: Parse EDN + (let [parse-result (parse-edn cleaned-text)] + (if (:success parse-result) + + ;; Step 3: Data-level cleaning (semantic fixes) + (let [cleaned-data (if auto-clean + (clean-data (:data parse-result)) + (:data parse-result))] + + ;; Step 4: Validate structure based on strategy + (if (= strategy :strict) + (import-all-or-nothing cleaned-data) + (import-progressive cleaned-data))) + + ;; Parse failed - return detailed error + {:success false + :parse-error true + :error (:error parse-result) + :line (:line parse-result) + :hint (:hint parse-result)})))) + +;; ============================================================================ +;; User-Friendly Error Messages +;; ============================================================================ + +(defn format-import-result + "Formats validation result into a user-friendly message." + [result] + (cond + ;; Parse error + (:parse-error result) + (str "⚠️ Could not read file\n\n" + "Error: " (:error result) "\n" + (when (:line result) + (str "Line: " (:line result) "\n")) + "\n" (:hint result) + "\n\nThe file may be corrupted or incomplete. " + "Try exporting a fresh copy if you have the original source.") + + ;; Validation error (strict mode) + (and (not (:success result)) (:errors result)) + (str "⚠️ Invalid orcbrew file\n\n" + (str/join "\n\n" (:errors result)) + "\n\nTo recover data from this file, you can:" + "\n1. Try progressive import (imports valid items, skips invalid ones)" + "\n2. Check the browser console for detailed validation errors" + "\n3. Export a fresh copy if you have the original source") + + ;; Progressive import with some items skipped + (:had-errors result) + (str "⚠️ Import completed with warnings\n\n" + "Imported: " (:imported-count result) " valid items\n" + "Skipped: " (:skipped-count result) " invalid items\n\n" + "Invalid items were skipped. Check the browser console for details.\n\n" + "To be safe, export all content now to create a clean backup.") + + ;; Successful import + (:success result) + (str "✅ Import successful\n\n" + (when (:imported-count result) + (str "Imported " (:imported-count result) " items\n\n")) + "To be safe, export all content now to create a clean backup.") + + ;; Unknown result + :else + "❌ Unknown import result")) diff --git a/start.sh b/start.sh new file mode 100755 index 000000000..d44c1bfd2 --- /dev/null +++ b/start.sh @@ -0,0 +1,119 @@ +#!/usr/bin/env bash +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +echo "Running start.sh from $SCRIPT_DIR; CWD=$(pwd)" + +# Validate Java 8 is present +JAVA_BIN="$(command -v java || true)" +if [ -z "${JAVA_BIN}" ]; then + echo "Java not found in PATH. Java 8 (1.8.x) is required. Install via SDKMAN (recommended) or your package manager." + echo "SDKMAN: curl -s \"https://get.sdkman.io\" | bash && source \"$HOME/.sdkman/bin/sdkman-init.sh\" && sdk install java 8.0.xx-tem" + exit 1 +fi +JAVA_VER="$($JAVA_BIN -version 2>&1 | awk -F '"' '/version/ {print $2}')" +if [[ "${JAVA_VER}" != 1.8* ]]; then + echo "Java 8 is required, but found: $JAVA_VER" + echo "Please install Java 8 and ensure 'java' points to it (or set JAVA_HOME appropriately)." + exit 1 +fi + +echo "Found Java $JAVA_VER at $JAVA_BIN" + +# Validate Leiningen is present +if ! command -v lein >/dev/null 2>&1; then + echo "Leiningen not found in PATH. Install via SDKMAN (sdk install leiningen) or your package manager." + echo "If you prefer not to install now, you can run the transactor manually and run 'lein run' in another shell." + exit 1 +fi + +# Set default values for environment variables if not already set +: "${ADMIN_PASSWORD:=admin}" +: "${DATOMIC_PASSWORD:=datomic}" +: "${ALT_HOST:=127.0.0.1}" +: "${ENCRYPT_CHANNEL:=true}" + +DATOMIC_DIR="$SCRIPT_DIR/lib/datomic-free-0.9.5703" +PROPERTIES_FILE="$DATOMIC_DIR/config/samples/free-transactor-template.properties" + +echo "Looking for properties file: $PROPERTIES_FILE" +if [ ! -f "$PROPERTIES_FILE" ]; then + echo "Datomic properties file not found: $PROPERTIES_FILE" + echo "Contents of $DATOMIC_DIR/config/samples:" + ls -la "$DATOMIC_DIR/config/samples" || true + exit 1 +fi + +# Ensure config dir exists +mkdir -p "$DATOMIC_DIR/config" +# Copy the template to a working properties file +cp -f "$PROPERTIES_FILE" "$DATOMIC_DIR/config/working-transactor.properties" +WORKING_PROPERTIES="$DATOMIC_DIR/config/working-transactor.properties" + +echo "Using working properties: $WORKING_PROPERTIES" + +# Update the properties file with environment variables +if grep -q "^alt-host=" "$WORKING_PROPERTIES"; then + sed -i "s/^alt-host=.*/alt-host=${ALT_HOST}/" "$WORKING_PROPERTIES" +else + sed -i "/^host=/a alt-host=${ALT_HOST}" "$WORKING_PROPERTIES" +fi + +# Replace commented password lines if present +sed -i "s/^#\s*storage-admin-password=.*/storage-admin-password=${ADMIN_PASSWORD}/" "$WORKING_PROPERTIES" || true +sed -i "s/^#\s*storage-datomic-password=.*/storage-datomic-password=${DATOMIC_PASSWORD}/" "$WORKING_PROPERTIES" || true +sed -i "s/^#\s*encrypt-channel=true/encrypt-channel=${ENCRYPT_CHANNEL}/" "$WORKING_PROPERTIES" || true + +if [ -n "${ADMIN_PASSWORD_OLD:-}" ]; then + sed -i "s/^#\s*old-storage-admin-password=.*/old-storage-admin-password=${ADMIN_PASSWORD_OLD}/" "$WORKING_PROPERTIES" || true +fi + +if [ -n "${DATOMIC_PASSWORD_OLD:-}" ]; then + sed -i "s/^#\s*old-storage-datomic-password=.*/old-storage-datomic-password=${DATOMIC_PASSWORD_OLD}/" "$WORKING_PROPERTIES" || true +fi + +# Verify transactor exists +TRANS="$DATOMIC_DIR/bin/transactor" +if [ ! -x "$TRANS" ]; then + echo "Datomic transactor not found or not executable: $TRANS" + echo "Contents of $DATOMIC_DIR/bin:" + ls -la "$DATOMIC_DIR/bin" || true + exit 1 +fi + +# Cleanup handler +cleanup() { + echo "Cleaning up..." + if [ -n "${DATOMIC_PID:-}" ]; then + kill "$DATOMIC_PID" 2>/dev/null || true + fi + if [ -n "${SERVER_PID:-}" ]; then + kill "$SERVER_PID" 2>/dev/null || true + fi +} +trap cleanup EXIT INT TERM + +# Start Datomic transactor +"$TRANS" "$WORKING_PROPERTIES" & +DATOMIC_PID=$! +echo "Started Datomic transactor with PID $DATOMIC_PID" + +# Wait and check if port is listening +sleep 3 + +# Start the Clojure server +if command -v lein >/dev/null 2>&1; then + lein run & + SERVER_PID=$! + echo "Started Clojure server with PID $SERVER_PID" +else + echo "Leiningen not found in PATH; skipping server start. You can run 'lein run' manually." +fi + +# Optionally, start a REPL for testing if lein exists +if command -v lein >/dev/null 2>&1; then + echo "Starting Leiningen REPL for testing..." + lein repl +else + echo "Leiningen not available; exiting after starting transactor and server if any." +fi diff --git a/test/clj/orcpub/errors_test.clj b/test/clj/orcpub/errors_test.clj new file mode 100644 index 000000000..2f40c9be9 --- /dev/null +++ b/test/clj/orcpub/errors_test.clj @@ -0,0 +1,177 @@ +(ns orcpub.errors-test + "Tests for error handling utilities." + (:require + [clojure.test :refer [deftest is testing]] + [orcpub.errors :as errors])) + +(deftest test-log-error + (testing "log-error logs message without context" + (is (nil? (errors/log-error "ERROR:" "Test message")))) + + (testing "log-error logs message with context" + (is (nil? (errors/log-error "ERROR:" "Test message" {:id 123}))))) + +(deftest test-create-error + (testing "create-error creates ExceptionInfo without cause" + (let [ex (errors/create-error "User message" :test-error {:id 123})] + (is (instance? clojure.lang.ExceptionInfo ex)) + (is (= "User message" (.getMessage ex))) + (is (= :test-error (:error (ex-data ex)))) + (is (= 123 (:id (ex-data ex)))))) + + (testing "create-error creates ExceptionInfo with cause" + (let [cause (Exception. "Original error") + ex (errors/create-error "User message" :test-error {:id 123} cause)] + (is (instance? clojure.lang.ExceptionInfo ex)) + (is (= "User message" (.getMessage ex))) + (is (= :test-error (:error (ex-data ex)))) + (is (= cause (.getCause ex)))))) + +(deftest test-with-error-handling* + (testing "with-error-handling* returns result on success" + (let [result (errors/with-error-handling* + (fn [] 42) + {:operation-name "test operation" + :user-message "Test failed" + :error-code :test-error + :context {:test true}})] + (is (= 42 result)))) + + (testing "with-error-handling* re-throws ExceptionInfo as-is" + (let [original-ex (ex-info "Original" {:error :original})] + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Original" + (errors/with-error-handling* + (fn [] (throw original-ex)) + {:operation-name "test operation" + :user-message "Test failed" + :error-code :test-error + :context {:test true}}))))) + + (testing "with-error-handling* wraps generic exceptions" + (try + (errors/with-error-handling* + (fn [] (throw (Exception. "Generic error"))) + {:operation-name "test operation" + :user-message "User-friendly message" + :error-code :test-error + :context {:user-id 456}}) + (is false "Should have thrown exception") + (catch clojure.lang.ExceptionInfo e + (is (= "User-friendly message" (.getMessage e))) + (is (= :test-error (:error (ex-data e)))) + (is (= 456 (:user-id (ex-data e)))) + (is (instance? Exception (.getCause e))) + (is (= "Generic error" (.getMessage (.getCause e))))))) + + (testing "with-error-handling* calls on-error callback" + (let [error-captured (atom nil)] + (try + (errors/with-error-handling* + (fn [] (throw (Exception. "Test error"))) + {:operation-name "test operation" + :user-message "Failed" + :error-code :test-error + :context {} + :on-error (fn [e] (reset! error-captured e))}) + (catch Exception _)) + (is (some? @error-captured)) + (is (= "Test error" (.getMessage @error-captured)))))) + +(deftest test-with-db-error-handling + (testing "with-db-error-handling returns result on success" + (let [result (errors/with-db-error-handling :test-error + {:id 123} + "Database operation failed" + (+ 1 2 3))] + (is (= 6 result)))) + + (testing "with-db-error-handling wraps exceptions with proper context" + (try + (errors/with-db-error-handling :db-test-error + {:user-id 789} + "Unable to save to database" + (throw (Exception. "Connection timeout"))) + (is false "Should have thrown exception") + (catch clojure.lang.ExceptionInfo e + (is (= "Unable to save to database" (.getMessage e))) + (is (= :db-test-error (:error (ex-data e)))) + (is (= 789 (:user-id (ex-data e)))) + (is (= "Connection timeout" (.getMessage (.getCause e)))))))) + +(deftest test-with-email-error-handling + (testing "with-email-error-handling returns result on success" + (let [result (errors/with-email-error-handling :test-error + {:email "test@example.com"} + "Email operation failed" + {:status :SUCCESS})] + (is (= {:status :SUCCESS} result)))) + + (testing "with-email-error-handling wraps exceptions with proper context" + (try + (errors/with-email-error-handling :email-send-failed + {:email "user@example.com" :username "alice"} + "Unable to send email" + (throw (Exception. "SMTP server unavailable"))) + (is false "Should have thrown exception") + (catch clojure.lang.ExceptionInfo e + (is (= "Unable to send email" (.getMessage e))) + (is (= :email-send-failed (:error (ex-data e)))) + (is (= "user@example.com" (:email (ex-data e)))) + (is (= "alice" (:username (ex-data e)))) + (is (= "SMTP server unavailable" (.getMessage (.getCause e)))))))) + +(deftest test-with-validation + (testing "with-validation returns result on success" + (let [result (errors/with-validation :test-error + {:input "123"} + "Invalid input" + (Long/parseLong "123"))] + (is (= 123 result)))) + + (testing "with-validation handles NumberFormatException" + (try + (errors/with-validation :invalid-number + {:input "abc"} + "Invalid number format" + (Long/parseLong "abc")) + (is false "Should have thrown exception") + (catch clojure.lang.ExceptionInfo e + (is (= "Invalid number format" (.getMessage e))) + (is (= :invalid-number (:error (ex-data e)))) + (is (= "abc" (:input (ex-data e)))) + (is (instance? NumberFormatException (.getCause e)))))) + + (testing "with-validation re-throws ExceptionInfo as-is" + (let [original-ex (ex-info "Original validation error" {:error :original})] + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Original validation error" + (errors/with-validation :test-error + {:test true} + "Validation failed" + (throw original-ex)))))) + + (testing "with-validation wraps other exceptions" + (try + (errors/with-validation :validation-error + {:data "test"} + "Validation failed" + (throw (RuntimeException. "Unexpected error"))) + (is false "Should have thrown exception") + (catch clojure.lang.ExceptionInfo e + (is (= "Validation failed" (.getMessage e))) + (is (= :validation-error (:error (ex-data e)))) + (is (= "test" (:data (ex-data e)))) + (is (instance? RuntimeException (.getCause e))))))) + +(deftest test-error-constants + (testing "error code constants are keywords" + (is (= :bad-credentials errors/bad-credentials)) + (is (= :unverified errors/unverified)) + (is (= :unverified-expired errors/unverified-expired)) + (is (= :no-account errors/no-account)) + (is (= :username-required errors/username-required)) + (is (= :password-required errors/password-required)) + (is (= :too-many-attempts errors/too-many-attempts)))) diff --git a/test/cljs/orcpub/dnd/e5/import_validation_test.cljs b/test/cljs/orcpub/dnd/e5/import_validation_test.cljs new file mode 100644 index 000000000..6813461b3 --- /dev/null +++ b/test/cljs/orcpub/dnd/e5/import_validation_test.cljs @@ -0,0 +1,410 @@ +(ns orcpub.dnd.e5.import-validation-test + (:require [cljs.test :refer-macros [deftest testing is]] + [orcpub.dnd.e5.import-validation :as import-val] + [orcpub.dnd.e5 :as e5] + [cljs.spec.alpha :as spec])) + +;; ============================================================================ +;; Test Data +;; ============================================================================ + +(def valid-plugin-edn + "{:orcpub.dnd.e5/spells + {:fireball {:option-pack \"My Homebrew\" + :name \"Fireball\" + :level 3} + :lightning-bolt {:option-pack \"My Homebrew\" + :name \"Lightning Bolt\" + :level 3}}}") + +(def invalid-plugin-edn-parse-error + "{:orcpub.dnd.e5/spells + {:fireball {:option-pack \"My Homebrew\"") ; Missing closing braces + +(def plugin-with-missing-option-pack + "{:orcpub.dnd.e5/spells + {:fireball {:name \"Fireball\" + :level 3}}}") ; Missing :option-pack + +(def plugin-with-empty-option-pack + "{:orcpub.dnd.e5/spells + {:fireball {:option-pack \"\" + :name \"Fireball\" + :level 3}}}") + +(def plugin-with-nil-values + "{:orcpub.dnd.e5/spells + {:fireball {:option-pack \"My Homebrew\" + :name nil + :level 3}}}") + +(def plugin-with-disabled-nil + "{:disabled? nil + :orcpub.dnd.e5/spells + {:fireball {:option-pack \"My Homebrew\" + :name \"Fireball\" + :level 3}}}") + +(def multi-plugin-edn + "{\"Plugin 1\" {:orcpub.dnd.e5/spells + {:fireball {:option-pack \"Plugin 1\" + :name \"Fireball\"}}} + \"Plugin 2\" {:orcpub.dnd.e5/races + {:elf {:option-pack \"Plugin 2\" + :name \"Elf\"}}}}") + +(def plugin-with-mixed-validity + "{:orcpub.dnd.e5/spells + {:valid-spell {:option-pack \"Test\" + :name \"Valid Spell\"} + :invalid-spell {:name \"No Option Pack\"} + :another-valid {:option-pack \"Test\" + :name \"Another Valid\"}}}") + +;; ============================================================================ +;; Parse Tests +;; ============================================================================ + +(deftest test-parse-edn-success + (testing "Parsing valid EDN" + (let [result (import-val/parse-edn valid-plugin-edn)] + (is (:success result)) + (is (map? (:data result))) + (is (contains? (:data result) :orcpub.dnd.e5/spells))))) + +(deftest test-parse-edn-failure + (testing "Parsing invalid EDN" + (let [result (import-val/parse-edn invalid-plugin-edn-parse-error)] + (is (not (:success result))) + (is (:error result)) + (is (string? (:hint result)))))) + +(deftest test-parse-edn-empty-string + (testing "Parsing empty string" + (let [result (import-val/parse-edn "")] + (is (not (:success result)))))) + +;; ============================================================================ +;; Validation Tests +;; ============================================================================ + +(deftest test-validate-item-valid + (testing "Validating a valid item" + (let [item {:option-pack "Test Pack" :name "Test Item"} + result (import-val/validate-item :test-item item)] + (is (:valid result))))) + +(deftest test-validate-item-missing-option-pack + (testing "Validating item without option-pack" + (let [item {:name "Test Item"} + result (import-val/validate-item :test-item item)] + (is (not (:valid result))) + (is (:errors result))))) + +(deftest test-validate-content-group + (testing "Validating a content group with mixed items" + (let [items {:valid1 {:option-pack "Test" :name "Valid 1"} + :invalid1 {:name "No Pack"} + :valid2 {:option-pack "Test" :name "Valid 2"}} + result (import-val/validate-content-group :orcpub.dnd.e5/spells items)] + (is (= 2 (:valid-count result))) + (is (= 1 (:invalid-count result))) + (is (= 1 (count (:invalid-items result))))))) + +(deftest test-validate-plugin-progressive + (testing "Progressive validation of plugin" + (let [plugin (read-string plugin-with-mixed-validity) + result (import-val/validate-plugin-progressive plugin)] + (is (not (:valid result))) ; Has invalid items + (is (= 2 (:valid-items-count result))) + (is (= 1 (:invalid-items-count result)))))) + +;; ============================================================================ +;; Pre-Export Validation Tests +;; ============================================================================ + +(deftest test-validate-before-export-valid + (testing "Pre-export validation of valid plugin" + (let [plugin (read-string valid-plugin-edn) + result (import-val/validate-before-export plugin)] + (is (:valid result)) + (is (or (nil? (:warnings result)) + (empty? (:warnings result))))))) + +(deftest test-validate-before-export-empty-option-pack + (testing "Pre-export validation detects empty option-pack" + (let [plugin (read-string plugin-with-empty-option-pack) + result (import-val/validate-before-export plugin)] + (is (seq (:warnings result))) + (is (some #(re-find #"option-pack" %) (:warnings result)))))) + +(deftest test-validate-before-export-nil-values + (testing "Pre-export validation detects nil values" + (let [plugin {:orcpub.dnd.e5/spells {:test {:option-pack nil}} + :some-key nil} + result (import-val/validate-before-export plugin)] + (is (seq (:warnings result)))))) + +;; ============================================================================ +;; Import Strategy Tests +;; ============================================================================ + +(deftest test-import-all-or-nothing-valid + (testing "All-or-nothing import with valid data" + (let [plugin (read-string valid-plugin-edn) + result (import-val/import-all-or-nothing plugin)] + (is (:success result)) + (is (= :single-plugin (:strategy result))) + (is (= plugin (:data result)))))) + +(deftest test-import-all-or-nothing-invalid + (testing "All-or-nothing import with invalid data" + (let [plugin (read-string plugin-with-missing-option-pack) + result (import-val/import-all-or-nothing plugin)] + (is (not (:success result))) + (is (:errors result))))) + +(deftest test-import-progressive-with-errors + (testing "Progressive import recovers valid items" + (let [plugin (read-string plugin-with-mixed-validity) + result (import-val/import-progressive plugin)] + (is (:success result)) + (is (:had-errors result)) + (is (= 2 (:imported-count result))) + (is (= 1 (:skipped-count result))) + (is (= 1 (count (:skipped-items result)))) + ;; Verify cleaned plugin only has valid items + (let [cleaned-spells (get-in result [:data :orcpub.dnd.e5/spells])] + (is (= 2 (count cleaned-spells))) + (is (contains? cleaned-spells :valid-spell)) + (is (contains? cleaned-spells :another-valid)) + (is (not (contains? cleaned-spells :invalid-spell))))))) + +(deftest test-import-progressive-all-valid + (testing "Progressive import with all valid items" + (let [plugin (read-string valid-plugin-edn) + result (import-val/import-progressive plugin)] + (is (:success result)) + (is (not (:had-errors result))) + (is (= 2 (:imported-count result))) + (is (= 0 (:skipped-count result)))))) + +;; ============================================================================ +;; Auto-Cleaning Tests +;; ============================================================================ + +(deftest test-validate-import-with-auto-clean + (testing "Auto-clean fixes disabled? nil" + (let [result (import-val/validate-import plugin-with-disabled-nil + {:strategy :progressive + :auto-clean true})] + (is (:success result)) + ;; After cleaning, disabled? should be false instead of nil + (let [disabled (get-in result [:data :disabled?])] + (is (= false disabled)))))) + +(deftest test-validate-import-empty-option-pack-auto-clean + (testing "Auto-clean fixes empty option-pack" + (let [result (import-val/validate-import plugin-with-empty-option-pack + {:strategy :progressive + :auto-clean true})] + ;; Should succeed after cleaning + (is (:success result))))) + +;; ============================================================================ +;; Complete Workflow Tests +;; ============================================================================ + +(deftest test-full-import-workflow-valid + (testing "Complete import workflow with valid file" + (let [result (import-val/validate-import valid-plugin-edn + {:strategy :progressive + :auto-clean true})] + (is (:success result)) + (is (not (:had-errors result))) + (is (map? (:data result)))))) + +(deftest test-full-import-workflow-parse-error + (testing "Complete import workflow with parse error" + (let [result (import-val/validate-import invalid-plugin-edn-parse-error + {:strategy :progressive + :auto-clean true})] + (is (not (:success result))) + (is (:parse-error result)) + (is (:error result)) + (is (:hint result))))) + +(deftest test-full-import-workflow-progressive + (testing "Complete import workflow with progressive strategy" + (let [result (import-val/validate-import plugin-with-mixed-validity + {:strategy :progressive + :auto-clean true})] + (is (:success result)) + (is (:had-errors result)) + ;; Should have imported 2 valid items and skipped 1 invalid + (is (= 2 (:imported-count result))) + (is (= 1 (:skipped-count result)))))) + +(deftest test-full-import-workflow-strict + (testing "Complete import workflow with strict strategy" + (let [result (import-val/validate-import plugin-with-mixed-validity + {:strategy :strict + :auto-clean true})] + ;; Strict mode should fail because not all items are valid + (is (not (:success result))) + (is (:errors result))))) + +;; ============================================================================ +;; Multi-Plugin Tests +;; ============================================================================ + +(deftest test-import-multi-plugin + (testing "Importing multi-plugin file" + (let [result (import-val/validate-import multi-plugin-edn + {:strategy :strict + :auto-clean true})] + (is (:success result)) + (is (= :multi-plugin (:strategy result))) + (is (map? (:data result))) + (is (contains? (:data result) "Plugin 1")) + (is (contains? (:data result) "Plugin 2"))))) + +;; ============================================================================ +;; Error Message Formatting Tests +;; ============================================================================ + +(deftest test-format-import-result-success + (testing "Formatting successful import result" + (let [result {:success true :imported-count 5} + message (import-val/format-import-result result)] + (is (string? message)) + (is (re-find #"✅" message)) + (is (re-find #"successful" message))))) + +(deftest test-format-import-result-with-warnings + (testing "Formatting import result with warnings" + (let [result {:success true + :had-errors true + :imported-count 2 + :skipped-count 1} + message (import-val/format-import-result result)] + (is (string? message)) + (is (re-find #"⚠️" message)) + (is (re-find #"warning" message))))) + +(deftest test-format-import-result-parse-error + (testing "Formatting parse error result" + (let [result {:success false + :parse-error true + :error "Unexpected token" + :line 5 + :hint "Check brackets"} + message (import-val/format-import-result result)] + (is (string? message)) + (is (re-find #"⚠️" message)) + (is (re-find #"Could not read" message)) + (is (re-find #"Line: 5" message))))) + +(deftest test-format-import-result-validation-error + (testing "Formatting validation error result" + (let [result {:success false + :errors ["Error 1" "Error 2"]} + message (import-val/format-import-result result)] + (is (string? message)) + (is (re-find #"⚠️" message)) + (is (re-find #"Invalid" message))))) +;; ============================================================================ +;; Data-Level Cleaning Tests +;; ============================================================================ + +(def plugin-with-preserved-nil + "{:orcpub.dnd.e5/classes + {:wizard {:option-pack \"Test\" + :name \"Wizard\" + :spellcasting {:spell-list-kw nil}}}}") + +(def plugin-with-removed-nil + "{:orcpub.dnd.e5/monsters + {:monster {:option-pack \"Test\" + :name \"Test Monster\" + :saving-throws {:str nil, :dex 5, :con nil}}}}") + +(def plugin-with-ability-nils + "{:orcpub.dnd.e5/monsters + {:monster {:option-pack \"Test\" + :name \"Test Monster\" + :abilities {:str nil, :dex nil, :con 10}}}}") + +(def plugin-with-trailing-comma + "{:orcpub.dnd.e5/spells + {:fireball {:option-pack \"Test\" + :name \"Fireball\",}}}") + +(def multi-plugin-with-empty-key + "{\"\" {:orcpub.dnd.e5/races {:elf {:option-pack \"\" :name \"Elf\"}}} + \"Existing Pack\" {:orcpub.dnd.e5/spells {:fireball {:option-pack \"Existing Pack\" :name \"Fireball\"}}}}") + +(deftest test-data-clean-preserves-semantic-nil + (testing "Data cleaning preserves nil for semantic fields like spell-list-kw" + (let [result (import-val/validate-import plugin-with-preserved-nil + {:strategy :progressive + :auto-clean true})] + (is (:success result)) + ;; spell-list-kw nil should be PRESERVED (it means custom spell list) + (let [wizard (get-in result [:data :orcpub.dnd.e5/classes :wizard])] + (is (= "Wizard" (:name wizard))) + (is (contains? (:spellcasting wizard) :spell-list-kw)) + (is (nil? (get-in wizard [:spellcasting :spell-list-kw]))))))) + +(deftest test-data-clean-removes-numeric-nil + (testing "Data cleaning removes nil for numeric fields like ability scores" + (let [result (import-val/validate-import plugin-with-removed-nil + {:strategy :progressive + :auto-clean true})] + (is (:success result)) + ;; :str nil and :con nil should be REMOVED (accidental leftovers) + (let [saving-throws (get-in result [:data :orcpub.dnd.e5/monsters :monster :saving-throws])] + (is (not (contains? saving-throws :str))) + (is (not (contains? saving-throws :con))) + (is (= 5 (:dex saving-throws))))))) + +(deftest test-data-clean-ability-nils + (testing "Data cleaning removes nil ability scores" + (let [result (import-val/validate-import plugin-with-ability-nils + {:strategy :progressive + :auto-clean true})] + (is (:success result)) + (let [abilities (get-in result [:data :orcpub.dnd.e5/monsters :monster :abilities])] + (is (not (contains? abilities :str))) + (is (not (contains? abilities :dex))) + (is (= 10 (:con abilities))))))) + +(deftest test-string-clean-trailing-comma + (testing "String cleaning removes trailing commas before closing braces" + (let [result (import-val/validate-import plugin-with-trailing-comma + {:strategy :progressive + :auto-clean true})] + (is (:success result)) + (is (= "Fireball" (get-in result [:data :orcpub.dnd.e5/spells :fireball :name])))))) + +(deftest test-data-clean-renames-empty-plugin-key + (testing "Data cleaning renames empty string plugin key" + (let [result (import-val/validate-import multi-plugin-with-empty-key + {:strategy :progressive + :auto-clean true})] + (is (:success result)) + ;; Empty key should be renamed to "Unnamed Content" + (let [data (:data result)] + (is (not (contains? data ""))) + (is (contains? data "Unnamed Content")) + (is (contains? data "Existing Pack")))))) + +(deftest test-data-clean-fixes-empty-option-pack + (testing "Data cleaning fixes empty option-pack strings" + (let [result (import-val/validate-import multi-plugin-with-empty-key + {:strategy :progressive + :auto-clean true})] + (is (:success result)) + ;; Empty option-pack should be replaced with "Unnamed Content" + (let [elf (get-in result [:data "Unnamed Content" :orcpub.dnd.e5/races :elf])] + (is (= "Unnamed Content" (:option-pack elf))))))) \ No newline at end of file