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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions submit_ce/domain/event/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ def _common_file_change_execute(api: SubmitApi, submission: Submission) -> None:
this is also stale.
* **preview** -- the compiled PDF that was produced from the
previous source.
* **compile_log** -- the TeX compiler log produced alongside that
preview. It belongs to the previous source, so leaving it behind
lets the Process page render a stale log (from a compile of files
that no longer exist) even though the preview it described is gone.
[SUBMISSION-75]

Submit 1.5 had separate ``clear_preflight`` and
``clear_directives_data`` routines in ``Submit.pm`` that were
Expand All @@ -72,6 +77,7 @@ def _common_file_change_execute(api: SubmitApi, submission: Submission) -> None:
file_store.delete_user_decisions(sid)
file_store.delete_directives(sid)
file_store.delete_preview(sid)
file_store.delete_compile_log(sid)


def _add_evaluate_oversize(api: SubmitApi,
Expand Down
18 changes: 17 additions & 1 deletion submit_ce/domain/event/tests/test_file_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
from pytz import UTC

from submit_ce.domain import submission as submod, agent
from submit_ce.domain.event.file import UploadFiles, RemoveFiles, RemoveAllFiles
from submit_ce.domain.event.file import (
UploadFiles, RemoveFiles, RemoveAllFiles, _common_file_change_execute,
)

def _now():
return datetime.now(UTC)
Expand Down Expand Up @@ -59,3 +61,17 @@ def test_remove_all_files_clears_package():
assert s.source_format is None
assert s.uncompressed_size == 0
assert s.submitter_confirmed_preview is False


def test_file_change_deletes_compile_log(mocker):
"""Any source file change invalidates the compile log alongside the other
derived artifacts, so a stale log can't linger on the Process page after the
source it described has changed. [SUBMISSION-75]"""
api = mocker.MagicMock()
store = api.get_file_store.return_value
submission = mocker.MagicMock()
submission.submission_id = "123"

_common_file_change_execute(api, submission)

store.delete_compile_log.assert_called_once_with("123")
3 changes: 3 additions & 0 deletions submit_ce/domain/event/tests/test_oversize_detection.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ def delete_directives(self, sid):
def delete_preview(self, sid):
pass

def delete_compile_log(self, sid):
pass


class _FakeApi:
def __init__(self, workspace=None, limits=None, unpacked=None):
Expand Down
70 changes: 70 additions & 0 deletions submit_ce/ui/controllers/new/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from submit_ce.ui.routes.flow_control import (
ready_for_next, stay_on_this_stage, advance_to_current,
)
from submit_ce.ui.workflow import conditions
from submit_ce.ui.backend import get_submission


Expand Down Expand Up @@ -74,6 +75,7 @@ def file_process(method: str, params: MultiDict, session: Session,
return advance_to_current(({}, status.OK, {}))

if method == "GET":
_maybe_autocompile(params, session, submission_id, token)
return compile_status(params, session, submission_id, token)
elif method == "POST":
if params.get('action') in ['previous', 'next', 'save_exit']:
Expand Down Expand Up @@ -155,6 +157,65 @@ def _check_status(params: MultiDict, session: Session, submission_id: str,
return ready_for_next(({}, status.OK, {}))


def _maybe_autocompile(params: MultiDict, session: Session, submission_id: str,
token: str) -> None:
"""Compile the source on arrival at the Process page, only when needed.

Implements SUBMISSION-75: the submitter no longer has to click "Process
submission files" to start compilation. On GET we initiate a compile when
the source is (La)TeX and there is no current compile to reuse, and we skip
it otherwise so revisiting the page (or refreshing) doesn't burn compute.

Compilation is (re)triggered when *both*:

* no valid preview exists -- either nothing has been compiled yet, or the
previous preview was invalidated by a file change (see
``_common_file_change_execute``); and
* no compile has already been attempted against the current source
(:func:`conditions.has_compiled_current_source`). This guards the failure
case: a compile that failed on TeX errors leaves no preview, but its
``StartCompileSource`` event means we must *not* silently recompile the
same broken source on every refresh -- the submitter sees the failure and
retries via the Reprocess button (a POST).

Only TeX source is auto-compiled here; PDF-only is handled earlier in
:func:`file_process`, and non-processing formats (e.g. HTML) need no
compile. The compile is dispatched as a server-initiated event rather than
through :func:`start_compilation` because the latter requires a CSRF token
that a GET request does not carry.

A failure to reach the compile service is logged and flashed but not raised:
the page still renders (with no preview), and a manual refresh or Reprocess
retries -- better than 500-ing on arrival.
"""
submission, events = get_submission(submission_id)
if submission.source_format != SourceFormat.TEX:
return

file_store: SubmissionFileStore = current_app.api.get_file_store()
if file_store.does_preview_exist(str(submission_id)):
return # A current compile already exists; reuse it.
if conditions.has_compiled_current_source(submission, events):
return # Already attempted for this source (e.g. it failed); don't loop.

submitter, client = user_and_client_from_session(session)
command = StartCompileSource(creator=submitter, client=client,
source_content_id="BOGUS")
try:
current_app.api.save(command, submission_id=submission_id)
file_store.uncompress_compile_tarball(submission_id)
except InvalidEvent as e:
# Precondition failed under the lock (e.g. empty source). Nothing to
# compile; leave the page to render its not-started state.
logger.info('Skipped auto-compile for %s: %s', submission_id, e)
except SaveError as e:
logger.error('Auto-compile failed for %s: %s', submission_id, e)
alerts.flash_failure(
f"We couldn't process your submission automatically. Use the"
f" Process button to try again. {SUPPORT}",
title="Processing failed")


def compile_status(params: MultiDict, session: Session, submission_id: str,
token: str, **kwargs: Any) -> Response:
"""
Expand Down Expand Up @@ -198,6 +259,15 @@ def compile_status(params: MultiDict, session: Session, submission_id: str,
if file and file.exists():
response_data['status']="succeeded"

# Show the compile log whenever one exists. Any source file change deletes
# it (see `_common_file_change_execute`), so a log that is present is always
# current -- this surfaces the compiler summary on a successful compile and
# the errors on a failed one, while a stale log from a since-changed source
# can't appear because it has been removed. (Note we must not gate this on
# the request-cached event history: `_maybe_autocompile` records the compile
# event within this same GET, but `get_submission` caches the pre-compile
# snapshot in `g`, so an event-based check would wrongly hide a just-created
# current log until the next refresh.) [SUBMISSION-75]
log = file_store.get_compile_log(str(submission_id))
if log and log.exists():
response_data['compile_log'] = log.download_as_text()
Expand Down
152 changes: 150 additions & 2 deletions submit_ce/ui/controllers/new/tests/test_process.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,49 @@
"""Tests for :mod:`submit_ce.controllers.process`."""

import io
from http import HTTPStatus as status

from werkzeug.datastructures import MultiDict
from flask import current_app
from flask import current_app, request

from submit_ce.domain.agent import InternalClient
from submit_ce.domain.event import SetSourceFormat
from submit_ce.domain.event.process import StartCompileSource
from submit_ce.domain.uploads import SourceFormat
from submit_ce.implementations.compile.mock_compile_mimesis_pdf import MockCompileMimesisPdf
from submit_ce.implementations.file_store.mock_file_store import MockFileStore
from submit_ce.ui.controllers.new.process import file_process
from submit_ce.ui.controllers.new.process import compile_status, file_process


class _CountingCompiler(MockCompileMimesisPdf):
"""Counts start_compile calls and writes a preview only when asked.

With ``produce_preview=False`` it models a compile that ran but produced no
usable PDF (e.g. TeX errors) -- a ``StartCompileSource`` event is still
recorded, but no preview lands, so we can assert the Process page does not
silently recompile the same broken source on every arrival. [SUBMISSION-75]
"""

def __init__(self, produce_preview: bool = True) -> None:
super().__init__()
self.compile_calls = 0
self._produce_preview = produce_preview

def start_compile(self, submission, user, client, api, source_package_id=None):
self.compile_calls += 1
if self._produce_preview:
return super().start_compile(submission, user, client, api,
source_package_id)
# Ran, but produced no preview.
from datetime import datetime, timezone
from submit_ce.domain.event.process import Result
from submit_ce.domain.process import ProcessStatus
return Result(
status=ProcessStatus(status=ProcessStatus.Status.SUCCEEDED,
creator=user, created=datetime.now(timezone.utc),
details={}),
duration_sec=0, utc_start_time=datetime.now(timezone.utc),
url="FAKE")


class _StampFails(MockCompileMimesisPdf):
Expand Down Expand Up @@ -41,6 +74,121 @@ def test_process(app, authorized_client, sub_reviewfiles):
and b"<form " in resp.data


def test_file_process_tex_autocompiles_on_arrival(
app, authorized_user_session, sub_files_tex):
"""TeX source with no prior compile: arriving at Process (GET) initiates
compilation automatically -- no button click needed. [SUBMISSION-75]"""
session, _ = authorized_user_session
with app.test_request_context("/"):
# compile_status instantiates a CSRF form, which needs an active
# session on the request. We call the controller directly (rather than
# via the test client) so we can inject the mock store/compiler and
# assert on them, so set request.auth ourselves. [SUBMISSION-75]
request.auth = session
sid = str(sub_files_tex.submission_id)
store = MockFileStore()
current_app.api.store = store
current_app.api.compiler = MockCompileMimesisPdf()

assert not store.does_preview_exist(sid)

rdata, code, _ = file_process("GET", MultiDict(), session, sid, token="")

assert code == status.OK
# A preview was produced and the page reflects success ...
assert store.does_preview_exist(sid)
assert rdata.get('status') == 'succeeded'
# ... and a compile event is now recorded for the current source.
_, events = current_app.api.get_with_history(sid)
assert any(isinstance(e, StartCompileSource) for e in events)


def test_file_process_tex_reuses_existing_preview(
app, authorized_user_session, sub_files_tex):
"""A valid preview already exists and the source is unchanged: arriving at
Process must NOT recompile -- the existing result is reused. [SUBMISSION-75]"""
session, _ = authorized_user_session
with app.test_request_context("/"):
# compile_status instantiates a CSRF form, which needs an active
# session on the request. We call the controller directly (rather than
# via the test client) so we can inject the mock store/compiler and
# assert on them, so set request.auth ourselves. [SUBMISSION-75]
request.auth = session
sid = str(sub_files_tex.submission_id)
store = MockFileStore()
current_app.api.store = store
counting = _CountingCompiler()
current_app.api.compiler = counting
# Seed a current preview as if a previous compile had succeeded.
store.store_preview(sid, io.BytesIO(b"%PDF-EXISTING\n%%EOF\n"))

_, code, _ = file_process("GET", MultiDict(), session, sid, token="")

assert code == status.OK
assert counting.compile_calls == 0 # reused, not recompiled


def test_file_process_tex_does_not_retry_failed_compile_on_refresh(
app, authorized_user_session, sub_files_tex):
"""A compile that produced no preview (e.g. TeX errors) is attempted once;
revisiting/refreshing Process must not silently recompile the same broken
source. The submitter retries explicitly via Reprocess. [SUBMISSION-75]"""
session, _ = authorized_user_session
sid = str(sub_files_tex.submission_id)
# Store/compiler live on the app-level api so they persist across the two
# separate request contexts below. Each context is a distinct arrival with
# a fresh `g`, so the second GET reloads the submission from the DB and sees
# the StartCompileSource event the first one recorded (mirroring two real
# HTTP requests, rather than reusing a cached snapshot). [SUBMISSION-75]
store = MockFileStore()
app.api.store = store
counting = _CountingCompiler(produce_preview=False)
app.api.compiler = counting

# First arrival: auto-compile runs, but yields no preview.
with app.test_request_context("/"):
request.auth = session # compile_status builds a CSRF form -> needs session
file_process("GET", MultiDict(), session, sid, token="")
assert counting.compile_calls == 1
assert not store.does_preview_exist(sid)

# Second arrival (fresh request/g cache): must not re-trigger.
with app.test_request_context("/"):
request.auth = session
file_process("GET", MultiDict(), session, sid, token="")
assert counting.compile_calls == 1


def test_compile_status_shows_current_log(
app, authorized_user_session, sub_files_tex, mocker):
"""A compile log that exists is current -- any file change deletes it (see
``_common_file_change_execute``) -- so ``compile_status`` surfaces it on the
Process page, whether the compile succeeded or failed. This also guards
against the request-cached-event pitfall: the log shows on first arrival,
not only after a refresh. [SUBMISSION-75]"""
session, _ = authorized_user_session
with app.test_request_context("/"):
# compile_status instantiates a CSRF form, which needs an active
# session on the request. We call the controller directly (rather than
# via the test client) so we can inject the mock store and assert on the
# rendered data, so set request.auth ourselves. [SUBMISSION-75]
request.auth = session
sid = str(sub_files_tex.submission_id)

fake = mocker.MagicMock()
fake.get_preview.return_value.exists.return_value = True
log = mocker.MagicMock()
log.exists.return_value = True
log.download_as_text.return_value = "COMPILER LOG OUTPUT"
fake.get_compile_log.return_value = log
mocker.patch.object(app.api, 'get_file_store', return_value=fake)

rdata, code, _ = compile_status(MultiDict(), session, sid, token="")

assert code == status.OK
assert rdata.get('compile_log') == "COMPILER LOG OUTPUT"


def test_file_process_pdf_only_installs_stamped_preview(
app, authorized_user, authorized_user_session, sub_primary):
"""PDF-only: transiting Process stamps the uploaded PDF and installs the
Expand Down
33 changes: 33 additions & 0 deletions submit_ce/ui/workflow/conditions.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
)
from submit_ce.domain.event.process import (
PreflightStatus,
StartCompileSource,
StartDirectives,
StartPreflight,
)
Expand Down Expand Up @@ -139,6 +140,38 @@ def has_current_directives(submission: Submission, events: List[Event]) -> bool:
return not any(isinstance(event, _FILE_CHANGE_EVENTS)
for event in events[last_directives + 1:])

def has_compiled_current_source(submission: Submission, events: List[Event]) -> bool:
"""Determine whether a compile has been attempted against the *current* source.

Used by the Process stage to decide whether compilation needs to be
(re)triggered on arrival. A compile counts as "current" only if no
file-change event has occurred since the most recent ``StartCompileSource``:
any file change invalidates the prior compile (and deletes its preview and
log via ``_common_file_change_execute`` in event/file.py), so the source
must be recompiled.

Returns True when the latest compile is current, meaning no new compile is
needed. Returns False when there has never been a compile, or when the
source has changed since the last one -- either way, compilation should be
initiated. ``events`` is in chronological order (oldest first).

Note this reflects that a compile was *attempted*, not that it produced a
usable PDF: a compile that fails on TeX errors still records a
``StartCompileSource`` event, so the submitter is shown the failure/log and
can retry rather than having the Process page silently recompile broken
source on every refresh. Whether a valid PDF exists is a separate check
(``does_preview_exist``). [SUBMISSION-75]
"""
last_compile = None
for i, event in enumerate(events):
if isinstance(event, StartCompileSource):
last_compile = i
if last_compile is None:
return False
return not any(isinstance(event, _FILE_CHANGE_EVENTS)
for event in events[last_compile + 1:])


def source_format_pdf(submission: Submission, events: List[Event]) -> bool:
return submission.source_format == SourceFormat.PDF

Expand Down
Loading
Loading