diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 30e907c..a29cf77 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -177,6 +177,22 @@ jobs: echo "$out" | grep -q "MinInclusive constraint failed" || { echo "::error::missing expected Zip range error"; exit 1; } echo "$out" | grep -q -- "-> " || { echo "::error::error row did not map to the node"; exit 1; } + # The failure direction that matters (#36): a document nothing could be checked against + # used to exit 0 and print "0 errors", so a CI job read "the schema host is down" as "the + # document is fine". The fixture's hint is a missing sibling file rather than a URL, so + # this asserts the behaviour and not the runner's network. + - name: Fux validation smoke (unloadable schema -> exit 3, never a pass) + shell: bash + run: | + printf '\n\n \n\n' > "$RUNNER_TEMP/noschema.xml" + set +e + out=$(dotnet run --project src/Fux -c Release -- --validate "$RUNNER_TEMP/noschema.xml") + code=$? + set -e + echo "$out" + [ "$code" -eq 3 ] || { echo "::error::a document whose schema would not load should exit 3 (got $code)"; exit 1; } + echo "$out" | grep -q "Not validated" || { echo "::error::an unchecked document reported a validation result"; exit 1; } + # Drives the real TUI under a PTY: --drill builds the actual UI, injects key events # (F6 focus ring, Enter-to-jump, F9 menu, ^Q), and asserts against the driver's output # buffer — borders/titles render, every cell painted, only theme-palette backgrounds. diff --git a/readme.md b/readme.md index f52a87d..a7f0c90 100644 --- a/readme.md +++ b/readme.md @@ -57,7 +57,7 @@ pass `RID=...`. `make help` lists everything. fux document.xml # open the editor fux --no-backup doc.xml # edit without keeping backups fux --dump document.xml # headless structure dump -fux --validate doc.xml # headless XSD validation (exit 1 if there are errors) +fux --validate doc.xml # headless XSD validation (1 = errors, 3 = no schema) fux --help # usage summary fux --version # version only ``` @@ -77,6 +77,28 @@ XML Notepad's conversion conventions. An import never overwrites its own source: saving writes XML, so `fux data.csv` will not silently turn `data.csv` into an XML file. +## Schemas + +fux validates against whatever the document's `xsi:schemaLocation` and +`xsi:noNamespaceSchemaLocation` hints point at — a URL as readily as a file sitting +next to the document. A schema published once on the web and referenced by every +document beats a copy beside each file, so remote hints are meant to be used. + +Being briefly unable to reach one is then routine rather than exotic, and fux is built +for that. Remote schemas are fetched on a background thread, never on the one drawing +the screen, so a host that swallows packets — VPN down, captive portal, firewall — does +not freeze the editor; a fetch that fails is remembered for the session instead of +being retried after every keystroke; and one fetch is capped at five seconds, or +whatever `--schema-timeout=N` says. + +When a schema cannot be fetched, or turns out not to be a schema, fux says so instead +of reporting a document nothing has checked as clean. On open you get a dialog — +**Retry**, **Continue**, or **Quit**, with Quit last so that a reflexive Enter cannot +leave you editing unvalidated — and for the rest of the session the validation pane +reads `Not validated: 1 schema unavailable` rather than `0 errors`. Headless, +`fux --validate` exits **3** for the same condition, so a CI job cannot mistake "the +schema host is down" for "the document is fine". + ## Keys | Key | Action | diff --git a/src/Fux/Drill.cs b/src/Fux/Drill.cs index 327d16d..1831086 100644 --- a/src/Fux/Drill.cs +++ b/src/Fux/Drill.cs @@ -1920,6 +1920,189 @@ string onClipboard() Check(Program.TryOpen(ui, file) == null, "block drill: the document reopens clean"); } + // --- 14c. A schema that cannot be loaded: fux says so, keeps saying so, and + // never waits for the network on this thread (#35, #36, #37). Three issues, one + // section, because they are one condition seen from three places — the pane, the + // prompt, and the fetch that must not happen here. + // + // Local fixtures throughout, deliberately. The point of #35's fix is that the UI + // thread does not go to the network at all, so a check that needed a route out + // would be measuring the runner rather than the code. The one remote hint below is + // never fetched from here, which is exactly what it is present to prove. + { + var schemaScratch = System.IO.Path.GetDirectoryName(file); + var resolver = (SchemaResolver)Program.Model.SchemaResolver; + + // (a) A hint that resolves to nothing. "fux-drill-missing.xsd" and not + // "emp.xsd": Main copies every sibling .xsd into this directory, so the + // obvious name would resolve and the fixture would prove nothing. + var orphan = System.IO.Path.Combine(schemaScratch, "fux_drill_orphan.xml"); + System.IO.File.WriteAllText(orphan, + "\n" + + "\n" + + " \n" + + "\n"); + Check(Program.TryOpen(ui, orphan) == null, "the unresolvable-schema document opens"); + var failed = Schemas.Settled(Program.Model.SchemaFailures); + Check(failed.Count == 1, $"the hint that would not load is recorded ({failed.Count})"); + Check(failed.Count == 1 && failed[0].Location == "fux-drill-missing.xsd", + "...naming the hint as the document writes it"); + + // (b) The persistent half of #37. The dialog fires once; this is what is still + // on screen for the rest of the session, and "0 errors" is precisely the thing + // it must not say. Read from the error pane's own title row rather than from + // the whole screen: the diagnostics quote the file name, so a screen-wide + // search would find almost anything almost anywhere. + app.LayoutAndDraw(true); + int titleRow = ui.ErrorList.Frame.Y; + var paneTitle = ScreenRow(app, titleRow); + Check(paneTitle.Contains("Not validated"), + $"the pane says the document was not validated [row {titleRow}: {paneTitle.Trim()}]"); + // Not "does not say 0 errors" — that stayed green under a mutation that put + // the old count back, because this fixture happens to have real errors to + // count. The property is stronger and simpler: a document nothing checked + // must not report a validation result at all. + Check(!paneTitle.Contains("Validation:"), + "...and reports no validation result, having performed none"); + + // (c) The button order, asserted from the array. MessageBox's Dialog exposes no + // SubViews in 2.4.17 and injected keys reach only app-scope bindings, so the + // buttons cannot be pressed — same reason DeleteButtons is checked this way. + // Enter lands on the LAST button (#21), and of the two reflexes the safe one is + // quitting: dismissing leaves the user editing a document nothing is checking. + Check(Program.SchemaButtons[Program.SchemaButtons.Length - 1] == "Quit" + && Program.SchemaQuit == Program.SchemaButtons.Length - 1, + "the schema prompt puts Quit last, where MessageBox's Enter lands"); + Check(Program.SchemaButtons[Program.SchemaRetry] == "Retry" + && Program.SchemaButtons[Program.SchemaContinue] == "Continue", + "...and Retry and Continue index the buttons they name"); + + // ...and that it actually opens, on the running UI, naming the schema. + int depth = -1, ticks = 0; + string modalText = null; + var tok = app.AddTimeout(TimeSpan.FromMilliseconds(100), () => + { + ticks++; + if (depth < 0 && ui.ModalDepth > 0) + { + depth = ui.ModalDepth; + app.LayoutAndDraw(true); + modalText = ScreenText(app); + } + app.Keyboard.RaiseKeyDownEvent(Key.Esc); + if (ticks > 20 && !ReferenceEquals(app.TopRunnable, ui.Top)) + app.RequestStop(app.TopRunnable); + return ticks <= 30; + }); + Program.WarnIfSchemaUnavailable(ui); + app.RemoveTimeout(tok); // never leave one armed for the next modal — see §14a + Check(depth > 0, $"the schema prompt opens on the running UI [ticks={ticks}]"); + Check(modalText != null && modalText.Contains("fux-drill-missing.xsd"), + "...naming the schema it could not load"); + + // (d) Once per condition, not once per validation — validation runs after every + // command, and a box that reopened on each of them would be unusable. + Check(!string.IsNullOrEmpty(ui.SchemaAckKey), "the prompt is recorded as acknowledged"); + int depth2 = -1, ticks2 = 0; + var tok2 = app.AddTimeout(TimeSpan.FromMilliseconds(100), () => + { + ticks2++; + if (depth2 < 0 && ui.ModalDepth > 0) depth2 = ui.ModalDepth; + app.Keyboard.RaiseKeyDownEvent(Key.Esc); + if (ticks2 > 20 && !ReferenceEquals(app.TopRunnable, ui.Top)) + app.RequestStop(app.TopRunnable); + return ticks2 <= 30; + }); + Program.WarnIfSchemaUnavailable(ui); + app.RemoveTimeout(tok2); + Check(depth2 < 0, $"the same failure does not prompt a second time [ticks={ticks2}]"); + + // (e) The retry storm, and the way out of it. Without the memory this is the + // 120-second `--validate` and the freeze-per-keystroke of #35; without the + // forgetting, a transient failure would outlive the wifi coming back. + var missing = new Uri(new Uri(orphan), "fux-drill-missing.xsd"); + Check(resolver.HasFailed(missing), + "the failure is remembered, so the next validation does not repeat the load"); + Program.RetrySchemas(ui); + Check(!resolver.HasFailed(missing), + "Retry forgets it, so the next load goes back to the source"); + Check(ui.SchemaAckKey == null, + "...and un-acknowledges, so the same failure is worth reporting again"); + + // (f) #35 itself: this thread does not go to the network. Structural, not + // timed — a remote hint comes back recorded as *pending*, which can only + // happen if the fetch was declined before a socket was opened. + Check(XmlProxyResolver.OfflineThread, "the UI thread is barred from fetching a schema"); + var blackhole = System.IO.Path.Combine(schemaScratch, "fux_drill_blackhole.xml"); + System.IO.File.WriteAllText(blackhole, + "\n" + + "\n" + + " \n" + + "\n"); + var timeoutWas = XmlProxyResolver.Timeout; + try + { + // Opening it starts a real background fetch, and 192.0.2.1 is unroutable by + // RFC 5737 — packets are dropped, not refused, so it costs the full timeout. + // Cap what that is worth to a CI runner. Restored in the finally: a mutation + // of the timeout left standing would be paid by every later section. + // Three seconds, not fifty milliseconds: the elapsed-time check below has + // to be able to fail. Nothing pays this in a passing run — the UI thread + // declines the fetch outright — so it is only spent by the background + // prefetch, off this thread, and by a regression that lets the fetch back + // onto it, which is the whole point. + XmlProxyResolver.Timeout = TimeSpan.FromSeconds(3); + var sw = System.Diagnostics.Stopwatch.StartNew(); + Check(Program.TryOpen(ui, blackhole) == null, "the black-holed-schema document opens"); + sw.Stop(); + var recorded = Program.Model.SchemaFailures; + Check(recorded.Count == 1 && recorded[0].Pending, + $"the remote hint is declined on this thread, not fetched ({recorded.Count} recorded)"); + Check(Schemas.Settled(recorded).Count == 0, + "...and a declined fetch is not reported to the user as a broken schema"); + Check(ui.SchemaPending, "the document is marked as still waiting for its schema"); + app.LayoutAndDraw(true); + var waiting = ScreenRow(app, ui.ErrorList.Frame.Y); + Check(waiting.Contains("loading schema"), + $"the pane says the schema is still loading [{waiting.Trim()}]"); + // Again not "does not say 0 errors": a mutation that removed the pending + // branch put back "2 errors, 0 warnings", which contains neither of those + // phrases and left the check green. What must not be here is a count of + // anything, since counting is exactly what has not happened yet. + Check(!waiting.Contains("error") && !waiting.Contains("no issues"), + "...rather than a tally of what a pass without the schema happened to find"); + // The structural checks above are the oracle; this is the number the user + // actually felt — 60s per hint, twice per validation pass, before the fix. + Check(sw.Elapsed < TimeSpan.FromSeconds(2), + $"opening it does not block on the network ({sw.ElapsedMilliseconds} ms)"); + } + finally + { + XmlProxyResolver.Timeout = timeoutWas; + } + + // (g) #37 asks for this as an assertion rather than an assumption, because a + // regression here would hang CI instead of failing it: --validate and --dump + // never build a Ui, and ModalQuery is what makes that safe. + Check(Program.ModalQuery(null, "t", "m", "a", "b") == null, + "a headless caller is never prompted"); + + // Back to the drill's own document, clean, for §15 — and the last check is + // about the two documents just opened as much as this one. All three declare + // the same target namespace, so the schema cache holds an entry for each of + // their hints; a document whose own schema loads must not inherit the failures + // of whatever was open before it. + Check(Program.TryOpen(ui, file) == null, "schema drill: the document reopens clean"); + Check(!ui.SchemaPending, "...and is not left waiting on a schema"); + var reopened = Schemas.Settled(Program.Model.SchemaFailures); + Check(reopened.Count == 0, + $"...and does not inherit the failed hints of the documents before it ({reopened.Count})"); + } + // --- 15. F9 focuses the menu bar; ^Q requests stop. The document is clean by // now (§14a reopened it), so ^Q must not prompt — and with no timeout armed to // answer one, a prompt here would hang the run rather than quietly pass. diff --git a/src/Fux/Program.cs b/src/Fux/Program.cs index 39648b8..f1ce979 100644 --- a/src/Fux/Program.cs +++ b/src/Fux/Program.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Collections.ObjectModel; +using System.Globalization; using System.Xml; using Terminal.Gui.App; using Terminal.Gui.Drawing; @@ -42,6 +43,21 @@ private static int Main(string[] args) { if (a.Length > 0 && a[0] == '-') { + // The one option that carries a value, so it cannot be an exact match + // against KnownFlags. Checked before that list, or every use of it would + // be rejected as an unknown option. + if (a.StartsWith(SchemaTimeoutFlag, StringComparison.Ordinal)) + { + var v = a.Substring(SchemaTimeoutFlag.Length); + if (!double.TryParse(v, NumberStyles.Float, CultureInfo.InvariantCulture, out var secs) + || secs <= 0 || double.IsInfinity(secs)) + { + Console.Error.WriteLine("fux: " + SchemaTimeoutFlag + " wants a positive number of seconds, not '" + v + "'"); + return 2; + } + XmlProxyResolver.Timeout = TimeSpan.FromSeconds(secs); + continue; + } if (Array.IndexOf(KnownFlags, a) < 0) { Console.Error.WriteLine("fux: unknown option '" + a + "'"); @@ -150,7 +166,12 @@ private static int Validate() { var hasFile = _model.Document?.DocumentElement != null; var items = RunValidation(); - Console.WriteLine(SummarizeValidation(items, hasFile).Trim()); + // Synchronous and on this thread, deliberately: OfflineThread is set in BuildUi and + // this path never builds a Ui, so a schema is fetched here and now. --validate is a + // CI entry point and has to be deterministic — the same document must give the same + // answer and the same exit code every run, which a background fetch cannot promise. + var schemaFailures = Schemas.Settled(_model.SchemaFailures); + Console.WriteLine(SummarizeValidation(items, hasFile, schemaFailures, false).Trim()); int errors = 0; foreach (var it in items) { @@ -162,7 +183,11 @@ private static int Validate() Console.WriteLine(" -> " + (node == null ? "(no node)" : GetLabel(node))); } } - return errors == 0 ? 0 : 1; + // Three answers, not two. A document nothing could be checked against used to exit + // 0, indistinguishable from one that passed, so a CI job read "the schema host is + // down" as "the document is fine" (#36). Errors still win the exit code when there + // are any: something did validate, and what it found is the more actionable news. + return errors > 0 ? 1 : schemaFailures.Count > 0 ? 3 : 0; } // -------------------------------------------------------------------- @@ -205,6 +230,13 @@ private static int RunUi(string file) ui.App.AddTimeout(TimeSpan.FromMilliseconds(1), () => { TerminalTitle.Set(_model.FileName, _model.Dirty); + // From here and not earlier, for two reasons. A dialog needs a driver that has + // learned the terminal size — a MessageBox laid out before that dies on a + // negative width — and Main loads the document before the app exists at all, + // so nothing raised from the load path would have had a UI to appear in (#37). + // A timeout cannot fire before the loop runs, which is exactly the ordering + // this needs. + StartSchemaPrefetch(ui); return false; // once }); @@ -265,6 +297,18 @@ internal sealed class Ui public XmlNode EditNode; // node whose value is being edited public int ModalDepth; // >0 while a dialog/message box runs: app-wide keys stay inert + // A background schema fetch is in flight. While it is, Revalidate does nothing: + // that is the whole of the mutual exclusion keeping the two threads off the shared + // schema cache at once (see Schemas). It is also what the pane title reports, so + // "loading" is never mistaken for "checked and clean". + public bool SchemaPending; + + // The set of schema failures the user has already been shown, as Schemas.FailureKey + // renders it. The prompt fires once per condition, not once per validation — and + // again when the condition changes, because a schema that has started failing for + // a new reason is news. Null means nothing has been acknowledged. + public string SchemaAckKey; + // The standing find, so F3 can repeat it. Kept as the raw query rather than a // built Query: an XPath one caches the nodes it selected, which the next edit // would invalidate (see the note on Find). @@ -287,6 +331,15 @@ internal sealed class Ui internal static Ui BuildUi(string file) { + // From here to the end of the process, this thread does not go to the network. + // A schema fetch on the UI thread is a freeze however short its timeout, and + // validation runs after every command, so the only safe rule is the absolute one: + // the UI thread sees what is already in the schema cache and nothing else, and a + // background thread is what puts things there (#35, see Schemas). Set here rather + // than in RunUi so that --drill gets exactly the same UI thread the editor does — + // and so CI cannot reach the network through the front end at all. + XmlProxyResolver.OfflineThread = true; + // v2 is instance-based (the static Application facade is marked obsolete). var app = Application.Create(null); app.Init(null); @@ -328,6 +381,10 @@ internal static Ui BuildUi(string file) Inert(new MenuItem("_Open…", "^O", () => StartOpen(ui), new Key())), Live(new MenuItem("_Save", "^S", () => SaveFile(ui), new Key())), Inert(new MenuItem("Save _As…", "", () => StartSaveAs(ui), new Key())), // menu-only: see the key handler + // The dialog's Retry is a one-shot: dismiss it and there would otherwise be + // no way back, so a session that started offline would stay unvalidated + // even after the VPN came up. Same call, reachable for the rest of the run. + Inert(new MenuItem("_Reload Schemas", "", () => RetrySchemas(ui), new Key())), Live(new MenuItem("_Quit", "^Q", () => RequestQuit(ui), new Key())), }), new MenuBarItem("_Edit", new View[] @@ -578,6 +635,10 @@ void CycleFocus() App = app, Top = top, Menu = menu, Status = status, Tree = tree, ValueView = valueView, ErrorList = errorList, Errors = errors, Undo = undo, EditInertMenu = editInert.ToArray(), EditLiveMenu = editLive.ToArray(), + // Set before the first Revalidate, not after: with a remote hint outstanding + // that pass could only report what it cannot yet know. The pane says "loading + // schema" until StartSchemaPrefetch's callback has a real answer. + SchemaPending = Schemas.RemoteHints(_model).Count > 0, }; Revalidate(ui); UpdateTitle(ui); @@ -819,6 +880,9 @@ private static bool TrySetClipboard(Ui ui, string text) private static readonly string[] KnownFlags = { "--dump", "--validate", "--drill", "--no-backup", "--help", "-h", "--version" }; + // Not in KnownFlags: it takes a value, so it is matched by prefix in Main instead. + private const string SchemaTimeoutFlag = "--schema-timeout="; + // What `fux --help` prints. Kept to plain stdout on purpose: the first thing anyone // does with an unfamiliar binary is ask it what it is, and that answer has to arrive // without a TTY, a document, or a running UI. Version comes from the assembly, so it @@ -857,7 +921,11 @@ internal static string UsageText() + " fux --version print the version and exit\n" + " fux --help print this message and exit\n" + "\n" - + "--validate exits 1 if the document has validation errors, else 0.\n" + + " --schema-timeout=N seconds to wait for a schema fetch (default 5)\n" + + "\n" + + "--validate exits 1 if the document has validation errors, 3 if a schema it\n" + + "declares could not be fetched or was not a schema — so nothing checked it —\n" + + "and 0 only when the document was validated and found clean.\n" + "\n" + "Before overwriting a file, fux copies its previous contents next to it as\n" + "..bak. A save that changes nothing writes no backup,\n" @@ -886,7 +954,7 @@ internal static string AboutText() + "See LICENSE and THIRD-PARTY-NOTICES.md."; } - private static int? ModalQuery(Ui ui, string title, string message, params string[] buttons) + internal static int? ModalQuery(Ui ui, string title, string message, params string[] buttons) { if (ui == null) return null; ui.ModalDepth++; @@ -1580,13 +1648,123 @@ private static void ExpandSubtree(Ui ui, XmlNode node) // Mutates ui.Errors in place: the RowRender/Accepting closures hold the list reference. private static void Revalidate(Ui ui) { + bool hasFile = _model.Document?.DocumentElement != null; + + // A fetch is in flight. Validating now would be both wrong and unsafe: wrong + // because the schemas it would report on are the ones not yet loaded, and unsafe + // because the background thread is writing to the schema cache this pass reads. + // Not validating is the mutual exclusion — see the concurrency rule in Schemas. + // The previous pass's rows stay put; only the title changes, so the pane cannot + // read "0 errors" while the answer is still being fetched. + if (ui.SchemaPending) + { + ui.ErrorList.Title = SummarizeValidation(ui.Errors, hasFile, null, true).Trim(); + return; + } + var items = RunValidation(); ui.Errors.Clear(); ui.Errors.AddRange(items); - ui.ErrorList.Title = SummarizeValidation(ui.Errors, _model.Document?.DocumentElement != null).Trim(); + ui.ErrorList.Title = SummarizeValidation(ui.Errors, hasFile, Schemas.Settled(_model.SchemaFailures), false).Trim(); ui.ErrorList.SetSource(new ObservableCollection(BuildErrorLines(ui.Errors))); } + // Start (or restart) resolution of the document's schemas, and report on the result. + // + // The single entry point for both halves of "is this document being checked at all": + // the remote hints go to a background thread, and when that settles — immediately, if + // there are none — the pane and the prompt are brought up to date. Called on open, on + // opening another document, and on Retry. + private static void StartSchemaPrefetch(Ui ui) + { + if (ui == null) return; + var remote = Schemas.RemoteHints(_model); + ui.SchemaPending = remote.Count > 0; + if (ui.SchemaPending) Revalidate(ui); // repaint as "loading" before the wait starts + Schemas.Prefetch(_model, remote, ui.App, () => + { + ui.SchemaPending = false; + Revalidate(ui); + WarnIfSchemaUnavailable(ui); + }); + } + + // Tell the user, once, that this document is not being validated and let them decide + // what to do about it. + // + // The dialog is half the answer and the pane title is the other half (#37). A modal + // fires once; the condition lasts the session, and after this is dismissed the title + // is the only thing still saying the document is unchecked — which is why + // SummarizeValidation must not go back to reading "0 errors". + // + // Headless callers never reach this: ModalQuery returns null when ui is null, and + // --validate and --dump never build a Ui at all. §16 of the drill asserts that rather + // than assuming it, since a regression here would hang CI instead of failing it. + internal static void WarnIfSchemaUnavailable(Ui ui) + { + if (ui == null || ui.SchemaPending) return; + var failures = Schemas.Settled(_model.SchemaFailures); + if (failures.Count == 0) + { + ui.SchemaAckKey = null; // the schemas resolved; a later failure is news again + return; + } + // A fetch settles on the main loop, and a modal runs a nested one — so this can + // arrive while the user is in the middle of another dialog. Stacking a second box + // on top of it would be both rude and unreadable. Come back when the screen is + // theirs again; the acknowledgement is deliberately not recorded yet, so nothing + // is lost by waiting. + if (ui.ModalDepth > 0) + { + ui.App.AddTimeout(TimeSpan.FromMilliseconds(250), () => + { + WarnIfSchemaUnavailable(ui); + return false; // once; if a dialog is still up this re-arms from the top + }); + return; + } + var key = Schemas.FailureKey(failures); + if (key == ui.SchemaAckKey) return; // already said so, and nothing has changed + ui.SchemaAckKey = key; + + // Esc closes a MessageBox with -1, which lands here as neither Retry nor Quit — + // i.e. as Continue, the same as the button. That is the right reading of Esc: it + // dismisses the dialog, it does not quit the editor and it does not refetch. + int choice = ModalQuery(ui, "Schema unavailable", Schemas.Describe(failures), + SchemaButtons) ?? SchemaContinue; + if (choice == SchemaRetry) RetrySchemas(ui); + else if (choice == SchemaQuit) RequestQuit(ui); + } + + // Quit last, and therefore the default: MessageBox binds Enter to the LAST button, the + // trap #21 walked into. Of the two reflexes that is the safe one — a reflexive Enter + // that quits costs a relaunch, while a reflexive Enter that dismisses lands the user + // editing a document nothing is checking, which is the exact state this prompt exists + // to prevent. Retry is first because the usual causes are transient: wifi not up yet, + // VPN not connected, captive portal not signed into. + // + // The indices sit next to the array for the same reason DeleteButtons' do: reorder one + // without the other and the prompt inverts. The drill asserts the pairing from here, + // because MessageBox's Dialog exposes no SubViews in 2.4.17 and its buttons cannot be + // pressed by an injected key. + internal static readonly string[] SchemaButtons = { "Retry", "Continue", "Quit" }; + internal const int SchemaRetry = 0; + internal const int SchemaContinue = 1; + internal const int SchemaQuit = 2; + + // Forget every remembered failure and resolve the document's schemas again. + // + // Clearing the acknowledgement too: the user asked for a fresh answer, so a fresh + // answer — including the same failure a second time — is worth showing. The prompt + // that follows is raised from the prefetch callback, never from inside this call, so + // repeated retries unwind one dialog before opening the next instead of nesting. + internal static void RetrySchemas(Ui ui) + { + (_model.SchemaResolver as SchemaResolver)?.ClearFailures(); + ui.SchemaAckKey = null; + StartSchemaPrefetch(ui); + } + // The tree pane title doubles as the document title: file name + dirty marker. private static void UpdateTitle(Ui ui) { @@ -1752,7 +1930,12 @@ private static void RebindDocument(Ui ui) } ui.Tree.SelectedObject = root; // null when the load left nothing behind ui.ValueView.Text = root == null ? "" : GetValue(root) ?? ""; + // Whatever was acknowledged was about the document being replaced. Cleared before + // the prefetch, so the new document's schemas get their own prompt if they need it. + ui.SchemaAckKey = null; + ui.SchemaPending = false; Revalidate(ui); + StartSchemaPrefetch(ui); UpdateTitle(ui); ui.Tree.SetFocus(); } @@ -1794,17 +1977,43 @@ private static List RunValidation() return collector.Items; } - private static string SummarizeValidation(List items, bool hasFile) + // The error pane's title, and the first line of `--validate`'s output. + // + // Its job is not to count things, it is to answer "was this document checked?". It + // used to conflate the two: with an unreachable schema it said "0 errors", which reads + // as a pass and is the wrong direction to fail in (#36). A document nothing validated + // now says so, here and for as long as the condition lasts — the dialog fires once, + // this is what is still on screen afterwards (#37). + private static string SummarizeValidation(List items, bool hasFile, + IList schemaFailures, bool pending) { if (!hasFile) return " (no file loaded)"; - if (items.Count == 0) return " Validation: no issues"; + if (pending) return " Validation: loading schema…"; + + int failed = schemaFailures == null ? 0 : schemaFailures.Count; + string Plural(int n, string w) => $"{n} {w}{(n == 1 ? "" : "s")}"; + int errors = 0, warnings = 0; foreach (var it in items) { if (it.Severity == Severity.Error) errors++; else if (it.Severity == Severity.Warning) warnings++; } - string Plural(int n, string w) => $"{n} {w}{(n == 1 ? "" : "s")}"; + + if (failed > 0) + { + // Every hint the document declares failed, so nothing was checked against + // anything: lead with that instead of a count, which would only describe the + // handful of things validation can find without a schema. + int hints = Schemas.HintCount(_model); + string what = failed >= hints + ? $" Not validated: {Plural(failed, "schema")} unavailable" + : $" Validation: {Plural(errors, "error")}, {Plural(warnings, "warning")}" + + $" — {Plural(failed, "schema")} unavailable"; + return what + " (Enter: go to node)"; + } + + if (items.Count == 0) return " Validation: no issues"; return $" Validation: {Plural(errors, "error")}, {Plural(warnings, "warning")} (Enter: go to node)"; } diff --git a/src/Fux/Schemas.cs b/src/Fux/Schemas.cs new file mode 100644 index 0000000..2dfba31 --- /dev/null +++ b/src/Fux/Schemas.cs @@ -0,0 +1,240 @@ +using System; +using System.Collections.Generic; +using System.Text; +using System.Threading.Tasks; +using System.Xml.Schema; +using Terminal.Gui.App; +using XmlNotepad; + +namespace Fux +{ + /// + /// What fux does about a document's schemas that the engine does not: resolve the remote + /// ones off the UI thread, and describe what came back in terms a person can act on. + /// + /// + /// + /// The engine resolves a schema lazily, from inside the validation pass, on whatever + /// thread is validating. In the editor that is the UI thread and validation runs after + /// every command, so a hint pointing at a host that swallows packets — VPN down, captive + /// portal, corporate firewall — froze fux for the length of the fetch, per keystroke + /// (#35). Remote schemas are worth using: one published once and referenced by every + /// document beats a copy sitting beside each file. Being transiently unable to reach one + /// is therefore a routine state, not an exotic one, and it has to be survivable. + /// + /// + /// So: is set on the UI thread for the life + /// of the process, and the UI thread sees only what is already in the schema cache. + /// Warming that cache is this class's job, on a background thread, once per document. + /// + /// + /// The concurrency rule, and it is the whole safety argument: a warm run and a + /// validation pass never overlap. Both write to the shared — + /// plain dictionaries, no locking — so overlapping them would be a data race. They are + /// kept apart by construction rather than by a lock: Program.Revalidate does + /// nothing at all while Ui.SchemaPending is set, and the flag is cleared on the UI + /// thread by the completion callback below, after the background thread is finished with + /// the cache. Nothing else may touch the schema cache off the UI thread. + /// + /// + internal static class Schemas + { + /// + /// The http/https schema hints on the document's root element, resolved and deduped. + /// + /// + /// Only the remote ones: a sibling .xsd is a file read, and moving that off the + /// UI thread would buy a document-open race in exchange for nothing measurable. + /// + internal static List RemoteHints(XmlCache model) + { + var uris = new List(); + var doc = model?.Document; + if (doc == null) return uris; + + // The document's directory, exactly as Checker.ValidateContext derives it — a + // relative hint has to resolve to the same place here as it will there, or the + // warm run would populate the cache under a URI the validation pass never asks for. + Uri baseUri = null; + if (!string.IsNullOrEmpty(model.FileName)) + { + baseUri = new Uri(new Uri(model.FileName), new Uri(".", UriKind.Relative)); + } + + foreach (SchemaHint hint in Checker.GetSchemaHints(doc)) + { + Uri resolved; + try + { + resolved = Checker.ResolveSchemaLocation(hint.Context, baseUri, hint.Location); + } + catch (UriFormatException) + { + continue; // a malformed hint; the validation pass is what reports it + } + if (!resolved.IsAbsoluteUri) continue; + if (resolved.Scheme != Uri.UriSchemeHttp && resolved.Scheme != Uri.UriSchemeHttps) continue; + if (!uris.Contains(resolved)) uris.Add(resolved); + } + return uris; + } + + /// + /// How many schema hints the document declares, remote and local alike. + /// + /// + /// Only ever compared against the number that failed, to tell "some of this document's + /// schemas are missing" from "none of them loaded, so nothing checked it". + /// + internal static int HintCount(XmlCache model) + { + int n = 0; + foreach (SchemaHint unused in Checker.GetSchemaHints(model?.Document)) n++; + return n; + } + + /// + /// Fetch into the shared schema cache on a background thread, + /// then run on the UI thread. Never throws to the caller. + /// + /// + /// Results are not returned: success lands in the schema cache and failure in the + /// resolver's session memory, which is where the next validation pass looks anyway. + /// The callback's job is only to lift the pending flag and revalidate. + /// The callback never runs inline, even when there is nothing to fetch — see below. + /// + internal static void Prefetch(XmlCache model, IList uris, IApplication app, Action onDone) + { + var resolver = model?.SchemaResolver as SchemaResolver; + if (resolver == null || uris == null || uris.Count == 0) + { + // Deferred to the next loop iteration rather than called here. The callback + // can open a dialog, and one of that dialog's buttons is Retry, which comes + // straight back through this method: called inline, a run of retries would + // nest one message box inside the last and grow the stack until the user + // stopped pressing it. A timeout lets each dialog unwind before the next. + // (IApplication.Invoke would not do — from the main thread it runs inline.) + app.AddTimeout(TimeSpan.Zero, () => { onDone(); return false; }); + return; + } + + Task.Run(() => + { + try + { + Warm(model, resolver, uris); + } + catch + { + // Every reason a schema did not load is already recorded — in the schema + // cache as an absence, in the resolver as a remembered failure. Nothing + // here is worth killing a background thread over, and the validation pass + // the callback triggers is what reports it. + } + try + { + app.Invoke(onDone); + } + catch + { + // The app can be torn down while a fetch is in flight — quit during the + // five seconds this is allowed to take. There is then no UI to update. + } + }); + } + + // Resolve each hint and compile it, on the calling (background) thread. + // + // Compile, not just resolve: and inside a fetched schema are + // resolved by XmlSchemaSet at compile time, through this same resolver. Skipping it + // would leave those nested fetches to be discovered by the validation pass on the UI + // thread, which may not fetch — so a schema whose includes are remote would never + // resolve at all, however many times it was retried. + private static void Warm(XmlCache model, SchemaResolver resolver, IList uris) + { + // Neither the reader nor the compiler may raise anything into fux's error pane + // from here: this run's diagnostics are thrown away, and the validation pass that + // follows produces the real ones against the real document. Without a handler the + // engine throws on the first schema warning instead. + resolver.Handler = (s, e) => { }; + + var set = new XmlSchemaSet { XmlResolver = resolver }; + set.ValidationEventHandler += (s, e) => { }; + foreach (Uri uri in uris) + { + try + { + if (resolver.GetEntity(uri, "", typeof(XmlSchema)) is XmlSchema schema) + { + set.Add(schema); + } + } + catch + { + // Recorded by the resolver; the next hint still deserves its chance. + } + } + try { set.Compile(); } catch { } + } + + /// + /// A stable identity for a set of load failures, so that a prompt can fire once per + /// condition instead of once per validation — and again when the condition changes. + /// + internal static string FailureKey(IList failures) + { + if (failures == null || failures.Count == 0) return ""; + var uris = new List(); + foreach (var f in failures) + { + if (!f.Pending) uris.Add(f.ResolvedUri + "|" + f.Message); + } + uris.Sort(StringComparer.Ordinal); + return string.Join("\n", uris.ToArray()); + } + + /// + /// The failures worth telling the user about: everything that actually failed. A + /// pending record is a fetch in flight, not an answer, and never reaches the user. + /// + internal static List Settled(IList failures) + { + var settled = new List(); + if (failures != null) + { + foreach (var f in failures) if (!f.Pending) settled.Add(f); + } + return settled; + } + + /// + /// The body of the "schema unavailable" prompt: what could not be loaded and why. + /// + internal static string Describe(IList failures) + { + var sb = new StringBuilder(); + sb.Append(failures.Count == 1 + ? "This document declares a schema that could not be loaded,\nso nothing is validating it.\n" + : $"This document declares {failures.Count} schemas that could not be\nloaded, so nothing is validating it.\n"); + foreach (var f in failures) + { + sb.Append('\n').Append(Ellipsize(f.Location, 60)).Append('\n'); + sb.Append(" ").Append(Ellipsize(f.Message, 60)).Append('\n'); + } + return sb.ToString(); + } + + // A URL is exactly the kind of string that is both essential and arbitrarily long, and + // MessageBox does not wrap: an untrimmed one silently widens the dialog past the + // terminal and takes its buttons off-screen with it. Elide the middle — the host at the + // front and the file name at the end are the two halves that identify it. + internal static string Ellipsize(string s, int max) + { + if (string.IsNullOrEmpty(s)) return ""; + s = s.Replace("\r", " ").Replace("\n", " "); + if (s.Length <= max) return s; + int head = (max - 3) / 2; + return s.Substring(0, head) + "..." + s.Substring(s.Length - (max - 3 - head)); + } + } +} diff --git a/src/Model/Checker.cs b/src/Model/Checker.cs index 331dc9a..90e29a8 100644 --- a/src/Model/Checker.cs +++ b/src/Model/Checker.cs @@ -12,6 +12,54 @@ namespace XmlNotepad { public enum Severity { None, Hint, Warning, Error } + /// + /// One xsi:schemaLocation / xsi:noNamespaceSchemaLocation entry, as written. + /// + /// + /// Enumerated separately from loading them so that a caller who wants to resolve the + /// document's schemas somewhere other than inside a validation pass — off the UI thread, + /// say — reads the hints the same way does rather than reimplementing + /// the parse and drifting from it. + /// + public sealed class SchemaHint + { + /// The xsi:* attribute the hint was written on; the error's position. + public XmlAttribute Context; + /// Target namespace, or "" for noNamespaceSchemaLocation. + public string Namespace; + /// The location exactly as written — relative path or absolute URL. + public string Location; + } + + /// + /// A schema hint the document declares that did not produce a usable schema. + /// + /// + /// The reason this is a record and not just the warning text: "nothing validated this + /// document" and "this document is valid" have to be distinguishable by the caller, and a + /// warning in a list of warnings is not (#36). Collected per validation pass, deduplicated + /// by resolved URI — LoadSchemas reaches the same hint twice, once by URI and once by + /// namespace. + /// + public sealed class SchemaLoadFailure + { + /// The hint as written in the document. + public string Location; + /// What it resolved to, or the location itself when it would not resolve. + public string ResolvedUri; + /// Why it failed, in the terms the user should see. + public string Message; + /// Where the hint is, for the error pane's jump-to-node. + public int Line, Col; + + /// + /// The fetch was declined, not attempted: this thread may not block on the network + /// and a background one is resolving it. Nothing is yet known about the URL, so this + /// is neither a diagnostic nor grounds for telling the user the schema is broken. + /// + public bool Pending; + } + public abstract class ErrorHandler { public abstract void HandleError(Severity sev, string reason, string filename, int line, int col, object data); @@ -33,6 +81,7 @@ public class Checker : IDisposable private XmlElement _node; private Hashtable _parents; private IntellisensePosition _position; + private readonly List _schemaFailures = new List(); internal const int SurHighStart = 0xd800; internal const int SurHighEnd = 0xdbff; @@ -58,6 +107,19 @@ public Checker(ErrorHandler eh) this._eh = eh; } + /// + /// The document's schema hints that did not load, from the last validation pass. + /// + /// + /// Empty is the only thing that means "everything the document asked for was + /// checked". A caller reporting "0 errors" without consulting this is reporting that + /// an unvalidated document passed. + /// + public IList SchemaFailures + { + get { return this._schemaFailures; } + } + public XmlSchemaAttribute[] GetExpectedAttributes() { return this._expectedAttributes; @@ -246,59 +308,78 @@ private bool LoadSchemasForNamespace(XmlSchemaSet set, SchemaResolver resolver, return result; } - bool LoadXsiSchemas(XmlDocument doc, XmlSchemaSet set, SchemaResolver resolver) + /// + /// The schema hints on a document's root element, in document order. + /// + /// + /// Public and static so that resolving a document's schemas outside a validation pass + /// reads the hints through this, not through a second copy of the parse. Pair it with + /// to get the URI a load would actually go to. + /// + public static IEnumerable GetSchemaHints(XmlDocument doc) { - if (doc.DocumentElement == null) return false; - bool result = false; + if (doc == null || doc.DocumentElement == null) yield break; foreach (XmlAttribute a in doc.DocumentElement.Attributes) { - if (a.NamespaceURI == "http://www.w3.org/2001/XMLSchema-instance") + if (a.NamespaceURI != "http://www.w3.org/2001/XMLSchema-instance") continue; + if (a.LocalName == "noNamespaceSchemaLocation") { - if (a.LocalName == "noNamespaceSchemaLocation") + if (!string.IsNullOrEmpty(a.Value)) { - string path = a.Value; - if (!string.IsNullOrEmpty(path)) - { - result = LoadSchema(set, resolver, a, "", a.Value); - } + yield return new SchemaHint { Context = a, Namespace = "", Location = a.Value }; } - else if (a.LocalName == "schemaLocation") + } + else if (a.LocalName == "schemaLocation") + { + // Whitespace-separated namespace/location pairs. An odd trailing word is + // a namespace with no location and is skipped, as `i + 1 < n` says. + string[] words = a.Value.Split(new char[] { ' ', '\t', '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); + for (int i = 0, n = words.Length; i + 1 < n; i++) { - string[] words = a.Value.Split(new char[] { ' ', '\t', '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); - for (int i = 0, n = words.Length; i + 1 < n; i++) - { - string nsuri = words[i]; - string location = words[++i]; - result |= LoadSchema(set, resolver, a, nsuri, location); - } + string nsuri = words[i]; + string location = words[++i]; + yield return new SchemaHint { Context = a, Namespace = nsuri, Location = location }; } } } + } + + /// + /// Where a hint's location resolves to: against the context node's own base URI if it + /// has one, else against (the document's directory). + /// + public static Uri ResolveSchemaLocation(XmlNode ctx, Uri fallbackBase, string location) + { + Uri baseUri = fallbackBase; + if (ctx != null && !string.IsNullOrEmpty(ctx.BaseURI)) + { + baseUri = new Uri(ctx.BaseURI); + } + return baseUri != null + ? new Uri(baseUri, location) + : new Uri(location, UriKind.RelativeOrAbsolute); + } + + bool LoadXsiSchemas(XmlDocument doc, XmlSchemaSet set, SchemaResolver resolver) + { + bool result = false; + foreach (SchemaHint hint in GetSchemaHints(doc)) + { + result |= LoadSchema(set, resolver, hint.Context, hint.Namespace, hint.Location); + } return result; } bool LoadSchema(XmlSchemaSet set, SchemaResolver resolver, XmlNode ctx, string nsuri, string filename) { + Uri resolved = null; try { if (set.Contains(nsuri)) { return false; } - Uri baseUri = this._baseUri; - if (!string.IsNullOrEmpty(ctx.BaseURI)) - { - baseUri = new Uri(ctx.BaseURI); - } - Uri resolved; - if (baseUri != null) - { - resolved = new Uri(baseUri, filename); - } - else - { - resolved = new Uri(filename, UriKind.RelativeOrAbsolute); - } + resolved = ResolveSchemaLocation(ctx, this._baseUri, filename); XmlSchema s = null; SchemaCache sc = this._cache.SchemaCache; var ce = sc.FindSchemaByUri(resolved.AbsoluteUri); @@ -320,13 +401,73 @@ bool LoadSchema(XmlSchemaSet set, SchemaResolver resolver, XmlNode ctx, string n return true; } } + catch (Exception e) when (SchemaOfflineException.IsIn(e)) + { + // Not a failure: this thread may not block on the network and something else + // is fetching it. Recorded so the caller can say "loading" rather than "0 + // errors", but deliberately not reported as a diagnostic — the fetch has not + // happened yet, so there is nothing to tell the user about the schema. + RecordSchemaFailure(ctx, filename, resolved, null, true); + } catch (Exception e) { - ReportError(Severity.Warning, string.Format(Strings.SchemaLoadError, filename, e.Message), ctx); + string reason = Unwrap(e).Message; + ReportError(Severity.Warning, string.Format(Strings.SchemaLoadError, filename, reason), ctx); + RecordSchemaFailure(ctx, filename, resolved, reason, false); } return false; } + // A hint that produced no schema. Kept alongside the warning rather than instead of it: + // the warning is what the user reads, this is what the caller can act on — a count of + // warnings cannot answer "was this document actually checked against anything?". + // + // Deduplicated by resolved URI because LoadSchemas reaches the same hint twice, once + // through LoadXsiSchemas and once through LoadSchemasForNamespace; a pending record is + // upgraded to a real failure if the second attempt gets a real answer. + void RecordSchemaFailure(XmlNode ctx, string location, Uri resolved, string message, bool pending) + { + string uri = resolved == null ? location : resolved.AbsoluteUri; + foreach (var existing in this._schemaFailures) + { + if (existing.ResolvedUri == uri) + { + if (existing.Pending && !pending) + { + existing.Pending = false; + existing.Message = message; + } + return; + } + } + int line = 0, col = 0; + LineInfo li = _cache == null ? null : _cache.GetLineInfo(ctx); + if (li != null) + { + line = li.LineNumber; + col = li.LinePosition; + } + this._schemaFailures.Add(new SchemaLoadFailure + { + Location = location, + ResolvedUri = uri, + Message = message, + Pending = pending, + Line = line, + Col = col, + }); + } + + // What HttpClient actually said, not AggregateException's "One or more errors occurred." + static Exception Unwrap(Exception e) + { + while (e is AggregateException agg && agg.InnerExceptions.Count == 1) + { + e = agg.InnerExceptions[0]; + } + return e; + } + void ReportError(Severity sev, string msg, XmlNode ctx) { if (_eh == null) return; diff --git a/src/Model/SchemaCache.cs b/src/Model/SchemaCache.cs index 57afa51..b75abf3 100644 --- a/src/Model/SchemaCache.cs +++ b/src/Model/SchemaCache.cs @@ -820,6 +820,19 @@ public class SchemaResolver : XmlProxyResolver SchemaCache cache; ValidationEventHandler handler; + // Schemas that could not be loaded, by absolute URI, with the reason. The positive + // cache above only ever remembers successes, so before this every failed hint was + // re-fetched from scratch: twice within one validation pass (LoadXsiSchemas resolves + // it by URI, LoadSchemasForNamespace again by namespace), and then again after every + // command, since validation runs after every edit, undo and redo. Against an + // unroutable host that measured 120 seconds per `--validate` and a freeze per + // keystroke in the editor (#35). + // + // A failure is remembered for the session and no longer, because the common causes + // are transient — wifi not up yet, VPN not connected, captive portal not signed into. + // ClearFailures is what a Retry is: forget, and let the next resolve go to the wire. + private readonly Dictionary _failures = new Dictionary(); + public SchemaResolver(IServiceProvider site, SchemaCache cache) : base(site) { this.cache = cache; @@ -831,42 +844,112 @@ public ValidationEventHandler Handler set { handler = value; } } + /// + /// Forget every remembered failure, so the next resolve attempts the fetch again. + /// + public void ClearFailures() + { + lock (_failures) { _failures.Clear(); } + } + + /// + /// Whether a load of this URI has already been tried and failed this session. + /// + public bool HasFailed(Uri uri) + { + lock (_failures) { return _failures.ContainsKey(uri.AbsoluteUri); } + } + public override object GetEntity(Uri absoluteUri, string role, Type ofObjectToReturn) { CacheEntry ce = cache.FindSchemaByUri(absoluteUri); if (ce != null && ce.HasUpToDateSchema) return ce.Schema; + // Already known bad: fail now, at the speed of a dictionary lookup, with the + // reason the real attempt gave. Callers cannot tell this from the first attempt, + // which is the point — every one of them reports the same diagnostic. + string remembered; + lock (_failures) + { + _failures.TryGetValue(absoluteUri.AbsoluteUri, out remembered); + } + if (remembered != null) throw new SchemaLoadException(absoluteUri, remembered); + XmlSchema s = null; if (ofObjectToReturn == typeof(XmlSchema)) { - using (XmlReader r = XmlHelpers.ReadXml(absoluteUri.AbsoluteUri, this, handler)) + try { - if (r != null) + using (XmlReader r = XmlHelpers.ReadXml(absoluteUri.AbsoluteUri, this, handler)) { - s = XmlSchema.Read(r, handler); - if (s != null) + if (r != null) { - s.SourceUri = absoluteUri.AbsoluteUri; - if (ce != null) - { - ce.Schema = s; - } - else + s = XmlSchema.Read(r, handler); + if (s != null) { - cache.Add(s); + s.SourceUri = absoluteUri.AbsoluteUri; + if (ce != null) + { + ce.Schema = s; + } + else + { + cache.Add(s); + } + return s; } - return s; } } + // Read returned null without throwing. This is the quiet half of "cannot + // be fetched or is invalid": something that is not a schema arrived where + // one was asked for — a captive portal's sign-in page, an HTML 404 body, a + // repo browser's UI. It parses as markup and simply is not an XSD. Left as + // a null return it came out of validation as "no schema, no errors, all is + // well", which is the wrong direction to fail in (#36). + throw new SchemaLoadException(absoluteUri, "not a valid XML schema"); + } + catch (Exception e) + { + // A declined fetch is not an answer about the URL — the caller was simply + // not allowed to wait. Remembering it would make a deferred fetch look + // like a broken schema for the rest of the session. + if (!SchemaOfflineException.IsIn(e)) Remember(absoluteUri, Unwrap(e)); + throw; } } - else + + return base.GetEntity(absoluteUri, role, typeof(Stream)) as Stream; + } + + private void Remember(Uri uri, string reason) + { + lock (_failures) { _failures[uri.AbsoluteUri] = reason; } + } + + // AggregateException wraps whatever HttpClient actually said, so the message a user + // saw began "One or more errors occurred." and buried the DNS or status text behind it. + private static string Unwrap(Exception e) + { + while (e is AggregateException agg && agg.InnerExceptions.Count == 1) { - return base.GetEntity(absoluteUri, role, typeof(Stream)) as Stream; + e = agg.InnerExceptions[0]; } + return e.Message; + } + } - return null; + /// + /// A schema load that has already been tried this session and failed, replayed from + /// 's memory rather than attempted again. + /// + public class SchemaLoadException : Exception + { + public SchemaLoadException(Uri uri, string message) : base(message) + { + this.Uri = uri; } + + public Uri Uri { get; } } } diff --git a/src/Model/XmlCache.cs b/src/Model/XmlCache.cs index 7c4e600..45fa303 100644 --- a/src/Model/XmlCache.cs +++ b/src/Model/XmlCache.cs @@ -108,6 +108,19 @@ public void ValidateModel(ErrorHandler handler) _checker.Validate(this); } + /// + /// The schema hints the last pass could not load. + /// + /// + /// Non-empty means the document was not fully checked, whatever the error count says. + /// The two are independent answers and the caller needs both: a document with no + /// errors and an unloaded schema has not passed, it has not been examined (#36). + /// + public IList SchemaFailures + { + get { return _checker == null ? new List() : _checker.SchemaFailures; } + } + public XmlDocument Document { diff --git a/src/Model/proxy.cs b/src/Model/proxy.cs index 695e80f..e2ee162 100644 --- a/src/Model/proxy.cs +++ b/src/Model/proxy.cs @@ -8,6 +8,7 @@ using System.Runtime.InteropServices; using System.Xml; using System.Net.Http; +using System.Threading; namespace XmlNotepad @@ -23,6 +24,48 @@ public XmlProxyResolver(IServiceProvider site) Proxy = HttpWebRequest.DefaultWebProxy; } + // One client for the process. A new HttpClient per fetch leaks its socket pool for the + // duration of TIME_WAIT, and the old code disposed it while still holding the response + // stream it had just returned. The timeout lives on a per-request token instead of on + // the client, because HttpClient.Timeout cannot be changed once a request has gone out + // and Timeout below is deliberately settable. + private static readonly HttpClient _client = + new HttpClient { Timeout = System.Threading.Timeout.InfiniteTimeSpan }; + + /// + /// How long a single schema fetch may take before it is abandoned. + /// + /// + /// XmlNotepad used 60 seconds. Against a host that swallows packets rather than + /// refusing — VPN down, captive portal, corporate firewall — that is not "the document + /// goes unvalidated", it is a frozen editor: a validation pass resolves each hint twice + /// (once by URI in LoadXsiSchemas, again by namespace in LoadSchemasForNamespace), so + /// `--validate` against a black-holed host measured 120 seconds, and validation runs + /// after every command (#35). Single-digit seconds is the whole budget a person waiting + /// at a keyboard has. Settable so a batch caller with a slow link can raise it — + /// `fux --schema-timeout=N`. + /// + public static TimeSpan Timeout { get; set; } = TimeSpan.FromSeconds(5); + + /// + /// Set on a thread that must never block on the network, and read only here. + /// + /// + /// The UI thread sets it for the life of the process: a fetch on it is a freeze no + /// matter how short the timeout, so the interactive front end resolves remote schemas + /// on a background thread instead and lets the UI thread see only what is already + /// cached (see Fux.Schemas). Thread-scoped rather than a flag on the resolver because + /// the resolver instance is shared between the two threads — the property answers + /// "may *I* wait here", which is the question each caller actually has. + /// The headless paths (--validate, --dump) never set it and stay fully synchronous. + /// + [ThreadStatic] private static bool _offlineThread; + public static bool OfflineThread + { + get { return _offlineThread; } + set { _offlineThread = value; } + } + public override object GetEntity(Uri absoluteUri, string role, Type ofObjectToReturn) { if (absoluteUri == null) @@ -61,12 +104,33 @@ public override object GetEntity(Uri absoluteUri, string role, Type ofObjectToRe Stream GetResponse(Uri uri) { Debug.WriteLine($"Loading Uri {uri}"); - using (var client = new HttpClient()) + if (OfflineThread) { - client.Timeout = TimeSpan.FromSeconds(60); - var result = client.GetAsync(uri).Result; - result.EnsureSuccessStatusCode(); - return result.Content.ReadAsStreamAsync().Result; + // Nothing has been learned about this URL — the caller simply may not wait + // here. Distinct from a failure, so callers can tell "not yet" from "no". + throw new SchemaOfflineException(uri); + } + using (var cts = new CancellationTokenSource(Timeout)) + { + HttpResponseMessage result; + try + { + result = _client.GetAsync(uri, cts.Token).GetAwaiter().GetResult(); + } + catch (OperationCanceledException) when (cts.IsCancellationRequested) + { + // The framework's own text for this is "A task was canceled", which tells + // the person reading the error pane nothing at all. Say what happened. + throw new TimeoutException($"Timed out after {Timeout.TotalSeconds:0.#}s"); + } + using (result) + { + result.EnsureSuccessStatusCode(); + // Buffered, not handed out live: the response has to be disposed, and the + // previous code returned a stream owned by an HttpClient it had already + // disposed on the way out of the using block. A schema is small. + return new MemoryStream(result.Content.ReadAsByteArrayAsync().GetAwaiter().GetResult()); + } } } @@ -76,6 +140,52 @@ IWebProxy GetProxy() } } + /// + /// A remote schema fetch that was declined rather than attempted, because the calling + /// thread may not block on the network (see ). + /// + /// + /// Deliberately not a failure: nothing has been learned about the URL, so it must not be + /// remembered as unreachable and must not be reported to the user as a broken schema. The + /// answer is "not yet" — a background fetch is what settles it. + /// + public class SchemaOfflineException : Exception + { + public SchemaOfflineException(Uri uri) + : base($"Schema not fetched yet: {uri}") + { + this.Uri = uri; + } + + public Uri Uri { get; } + + /// + /// Whether is, or wraps, a declined fetch. + /// + /// + /// The chain walk is the point. XmlReader.Create resolves the URI from inside its own + /// construction, so what comes back out is whatever that path chose to wrap the + /// resolver's exception in — testing the outermost type would silently start treating + /// "not fetched yet" as "schema is broken", which is exactly the wrong answer to + /// remember for the session and to put in front of the user. + /// + public static bool IsIn(Exception e) + { + for (; e != null; e = e.InnerException) + { + if (e is SchemaOfflineException) return true; + if (e is AggregateException agg) + { + foreach (var inner in agg.InnerExceptions) + { + if (IsIn(inner)) return true; + } + } + } + return false; + } + } + public enum WebProxyState { NoCredentials = 0,