Skip to content

feat(collectives-publish) build targz - #2839

Merged
code4candy merged 9 commits into
feature/collectives-publishfrom
publish-feature/build-targz
Oct 7, 2026
Merged

code4candy merged 9 commits into
feature/collectives-publishfrom
publish-feature/build-targz

Conversation

@code4candy

@code4candy code4candy commented Oct 5, 2026 •

Copy link
Copy Markdown

📝 Summary

Goal

Provide a tar.gz archive for the Publish Service to retrieve.

Description

After selecting the data to be published, it needs to be prepared for the Publish Service.
After storing the archive in appData the external Publish Service gets
This involves several steps:

  • Collect all files (not from the Files app)
  • Preserve the folder hierarchy
  • Package the files as a tar.gz archive
  • Temporarily store the tar.gz archive so that it can be retrieved by the Publish Service

Acceptance Criteria

  • 1. The tar.gz archive contains all Collective files, including assets, preserving the same hierarchy as in the Collective.
  • 2. The archive does not contain the .templates folder or any of its subfolders.
  • 3. The tar.gz archive is available for retrieval at a defined path.
  • 4. The total archive size is limited to 100 MB (+ some Byte?).

Limitation

A large Collective with many images may hit max_execution_time or proxy/browser timeouts when the archive is built synchronously.

Dev Notes:

  • vgl. Download Collective.
Test in console (nextcloud-docker-dev):
# direkt über Terminal:
docker exec master-database-mysql-1 mysql -unextcloud -pnextcloud nextcloud -e "SELECT id, slug, title, status FROM oc_collectives_st_sites;"

# tar.gz list
docker exec master-nextcloud-1 sh -c 'ls -la /var/www/html/data/appdata_*/collectives/static_sites/'

# Inhalt tar.gz
docker exec master-nextcloud-1 sh -c 'tar tzvf /var/www/html/data/appdata_*/collectives/static_sites/<MY_TAR_NAME>.tar.gz'

🖼️ Screenshots

Successful storage of tar.gz:
Bildschirmfoto vom 2026-10-05 16-25-40

Failed - on second create with same slug (not yet published, so no update possible):
Bildschirmfoto vom 2026-10-05 16-25-19-1

🚧 TODO

  • ...

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation (README or documentation) has been updated or is not required

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI tools
  • The AI-generated content was reviewed, comprehended and tested by a human

@code4candy
code4candy requested a review from kjoscha October 5, 2026 14:38
@kjoscha kjoscha changed the title Publish feature/build targz feat(collectives-publish) build targz Oct 6, 2026
@kjoscha

kjoscha commented Oct 6, 2026 •

Copy link
Copy Markdown
  • Make a temp remark in the UI, that page de-selection has no effect yet

Diskussion:
Halte ich für überflüssige Arbeit - wir sind nicht in Prod und die Selection muss ja demnächst kommen. Wir wissen ja, dass das noch nicht funktioniert.
I think that is unnecessary work - its not on PROD yet and the selection will come soon. Also we know it does not work.

@kjoscha

kjoscha commented Oct 6, 2026 •

Copy link
Copy Markdown
  • Deleting a collective now leaks appdata blobs

Nothing in CollectiveService or the listeners touches collectives_st_sites. Before this commit that was an orphaned DB row; now it’s also a permanent static_sites/.tar.gz in appdata, plus a permanently occupied slug. Worth a follow-up issue at minimum.

Comment:
True but also the publish service needs to delete the provided websites. And there is currently no route for that.

So this PR does not get to big I am going to create an extra issue for this topic:
https://github.com/orgs/nextcloud-publish/projects/1/views/1?filterQuery=collectives&pane=issue&itemId=264748223&issue=nextcloud-publish%7Cgeneral%7C72

@kjoscha

kjoscha commented Oct 6, 2026 •

Copy link
Copy Markdown
  • An empty file set produces a confusing 500

PharData::compress() on an archive with zero entries succeeds but writes no site.tar.gz (verified). buildArchive() returns the path anyway, writeToAppData()’s fopen fails, and the user gets ServiceException('Failed to open static site archive') → bare 500. Hard to reach in practice (every collective has a landing page), but a guard — if ($files === []) → UnprocessableEntityException, or a file_exists check on the compressed path — is cheaper than the mystery. Untested either way.

Comment:
Done.
Prevent tar packing when no files to pack.

@kjoscha

kjoscha commented Oct 6, 2026 •

Copy link
Copy Markdown

Readability

  • isInProgress() (lib/Db/StaticSite.php:70-75): the early-return-false + in_array over a one-element array reads as if the timeout applies to all in-progress statuses. It reduces to:

    return $this->status === self::STATUS_PENDING
        && $this->updatedAt >= $now - self::PENDING_TIMEOUT;

    If the array is kept for the TODO, a single boolean expression still reads better.

Comment: unnecessary work - changing now and changing it back tomorrow.

  • validateSize() (lib/Service/StaticSiteService.php:194-199) slices $sizes twice, once preserving keys and once not. One slice feeds both:

    $largest = array_slice($sizes, 0, self::LARGEST_FILES_IN_MESSAGE, true);
    $largestFiles = array_map($format, array_keys($largest), $largest);

Comment: OK

  • getSelectedPageIds() only guards JsonException. A corrupted-but-valid scalar ("5") decodes to int and hits a TypeError against the : array return, not the documented UnexpectedValueException — and it's reached from jsonSerialize(), so it 500s the whole index endpoint.

Comment: OK

  • ServiceException isn't mapped in OCSExceptionHelper, so archive failures return an unannotated 500 and the wrapped root cause is never logged by the app itself (buildArchive() only attaches it as previous). An explicit $this->logger->error(..., ['exception' => $e]) before rethrowing would make failures diagnosable.

Comment: OK - add info to log entry and catch exception for readable toast message. But no second logging of the same information.

  • playwright/e2e/collective-publish.spec.ts: the no-unsafe-optional-chaining disable hides real nullability — expect(response).not.toBeNull() and then a non-optional access is clearer than silencing the rule.

Comment: OK - slicing the rule

  • static_site_max_size is a new admin-facing knob with no documentation.

Comment: Add to new documentation isssue: https://github.com/orgs/nextcloud-publish/projects/1/views/1?filterQuery=collectives&pane=issue&itemId=264855052&issue=nextcloud-publish%7Cgeneral%7C73

Comment thread lib/Service/StaticSiteArchiver.php Outdated
foreach ($files as $path => $file) {
$localPath = $filesFolder . '/' . $index++;
$this->copyToLocal($file, $localPath);
$tar->addFile($localPath, (string)$path);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

According to Claude, this is a severe problem:

PharData doesn’t append — every addFile() re-serializes the whole tar to disk.

So N files means N full rewrites: the work grows with N², and doubling the page count quadruples the publish time.

Measured locally (100 KB per file): 200 files → 1.4 s, 400 → 5.7 s, 800 → 24 s. A real wiki can blow the request timeout here and leave a pending row behind.

buildFromIterator() writes the archive once and takes exactly the mapping we already have (archivePath => localPath): same 800 files drop to 0.1 s. Would mean collecting the copies into an array in the loop and then:

$tar->buildFromIterator(new ArrayIterator($localPaths));

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good finding!
Ist umgesetzt.

@code4candy
code4candy force-pushed the feature/collectives-publish branch from 9ca1098 to 37ee4ef Compare October 7, 2026 21:03
Melpo added 8 commits October 7, 2026 23:28
Signed-off-by: Melpo <melpomene@posteo.net>
Assisted-by: ClaudeCode:claude-sonnet-5
Signed-off-by: Melpo <melpomene@posteo.net>
Assisted-by: ClaudeCode:claude-sonnet-5
Signed-off-by: Melpo <melpomene@posteo.net>
Assisted-by: ClaudeCode:claude-sonnet-5
Signed-off-by: Melpo <melpomene@posteo.net>
Assisted-by: ClaudeCode:claude-sonnet-5
Signed-off-by: Melpo <melpomene@posteo.net>
Signed-off-by: Melpo <melpomene@posteo.net>
Assisted-by: ClaudeCode:claude-sonnet-5
Signed-off-by: Melpo <melpomene@posteo.net>
Assisted-by: ClaudeCode:claude-sonnet-5
Signed-off-by: Melpo <melpomene@posteo.net>
Assisted-by: ClaudeCode:claude-sonnet-5
@code4candy
code4candy force-pushed the publish-feature/build-targz branch from 34b44c6 to 731b7a0 Compare October 7, 2026 21:31
Signed-off-by: Melpo <melpomene@posteo.net>
Assisted-by: ClaudeCode:claude-sonnet-5
@code4candy
code4candy marked this pull request as ready for review October 7, 2026 22:46
@code4candy
code4candy merged commit 7d82ad6 into feature/collectives-publish Oct 7, 2026
47 of 53 checks passed
@code4candy
code4candy deleted the publish-feature/build-targz branch October 7, 2026 22:56
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