Skip to content

fix: respect explicit spreadsheet preview type#772

Open
SpencerZhang wants to merge 1 commit into
kekingcn:masterfrom
SpencerZhang:fix/xlsx-pdf-preview-routing
Open

fix: respect explicit spreadsheet preview type#772
SpencerZhang wants to merge 1 commit into
kekingcn:masterfrom
SpencerZhang:fix/xlsx-pdf-preview-routing

Conversation

@SpencerZhang

Copy link
Copy Markdown
Contributor

What changed

  • Track whether officePreviewType was explicitly supplied by the request or caller.
  • Respect explicit pdf, html, xlsx, and csv preview routing for spreadsheet files.
  • Preserve the existing default Web-preview behavior when no preview type is explicitly supplied.
  • Add focused unit tests for cache-target selection and Web-preview routing.

Why

When office.type.web=web, an explicit officePreviewType=pdf request for an xlsx file could still be routed to the xlsx Web preview. Spreadsheet cache naming was also decided before the request-level preview type was applied, so the generated artifact could disagree with the requested mode.

This change distinguishes an explicit request override from the configured default and applies it before selecting the cache target and preview page.

Impact

  • Explicit xlsx PDF requests now use the Office-to-PDF flow.
  • Explicit HTML requests continue to generate HTML previews.
  • Requests without an explicit preview type retain the existing xlsx/csv Web-preview behavior.

Validation

  • git diff --check
  • Added FileHandlerServiceTests and OfficeFilePreviewImplTests.
  • Targeted Maven tests could not run because com.aspose:aspose-cad:25.10 was unavailable from the configured Hzero Maven repository.

@klboke klboke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for one routing blocker.

OfficeFilePreviewImpl.filePreviewHandle() calls checkAndHandleConvertStatus() before shouldUseWebPreview(). With this patch, an explicit officePreviewType=xlsx or csv uses isHtmlView=false, so it receives the same .pdf cache/status key used by PDF/image conversion. If that spreadsheet already has an image conversion in progress, failed, or timed out, the explicit Web-preview request is intercepted by the conversion-status check and returns the waiting/error page instead of the xlsx/csv Web preview.

I reproduced this against the full filePreviewHandle() path: after marking the shared .pdf cache key as converting, an explicit xlsx request returned waiting rather than officeweb.

Please handle shouldUseWebPreview(fileAttribute) before checking server-side conversion status, since the Web preview does not depend on that conversion. Please also add a regression test that exercises the full filePreviewHandle() ordering; the current helper-only tests do not catch this case.

The four GitHub checks are green, and the eight newly added focused tests pass locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants