Add optional setting to use system temp directory for H5P temporary files

[Lin87]

Hello,

This pull request introduces an optional admin setting that allows H5P to store temporary/working files in the server’s system temporary directory (via PHP's sys_get_temp_dir() function) instead of the H5P plugin directory.

Background

In our environment, WordPress content is stored on AWS Elastic File System (EFS). While EFS works well for persistent content, it performs poorly when handling large numbers of small, frequently changing files—such as those created during H5P uploads and processing.

By using the server's system temp directory for these temporary files, we've observed:

• Significantly faster upload and processing times
• Reduced I/O overhead on networked file systems like EFS
• Improved performance on resource-constrained servers

This aligns with common best practices of using local system temp storage for short-lived working files.

What This PR Does

• Adds a new admin-configurable setting to enable use of the system temp directory for H5P temporary files
• Keeps the setting disabled by default, preserving existing behavior
• Ensures the change is fully opt-in and does not impact existing installations unless explicitly enabled by an administrator

Backward Compatibility & Safety

• No changes to default behavior
• No migration required
• Existing installs will continue to use the current plugin directory unless the setting is enabled
• The feature is isolated to temporary file handling only

This pull request is based on the earlier work from PR #130, originally submitted several years ago. That PR was created by my colleague and did not receive any follow-up, so this submission reintroduces the feature with minor updates to align with the current H5P codebase.

Thank you for considering this improvement. I'm happy to adjust the implementation or documentation if needed.

[otacke]

@Lin87 Hi Ethan! As you may have heard, I am now a community maintainer, and I started to browse some of the issues and pull-requests across H5P Group's repositories, so they will eventually be handled.

I think your pull request makes sense in general! I cannot decide whether it will be accepted or rejected, however. Nevertheless did a review and noted some findings that you might want to check, see comments.

Unfortunately, you did not pre-qualify the change, because I think optionally writing to the server's /tmp directory instead would not only be a smart idea to have for WordPress, but for any H5P integration. Then getTmpPath in H5P core (https://github.com/h5p/h5p-php-library/blob/5e6058c88b5641bdda9167f25cbee4881cf7b03b/h5p-default-storage.class.php#L103-L107) would need to get the switch, and H5P itegrations would pass that either in the function call or via an environment variable (that might be set on the server even without an option in the settings).

I'll /cc @icc here to comment on whether optionally using /tmp would be considered by H5P Group and whether it should be implemented in H5P core or per H5P integration.

Cheers,
Oliver

p.s.: You should better not develop on your master branch and create pull requests from it. It prevents you from properly syncing changes, and if your pull request gets rejected, you'd have to clean up the repository to bring it to a valid state again. Can be done, sure, but it's probably better to prevent something like that by simply working on a dedicated branch.

[otacke]

@Lin87 Do you plan to take care of the comments I added?

[Lin87]

@otacke So sorry. Work has been hectic. I will set some time aside next week and tackle this.

[otacke]

@Lin87 No worries. I am not trying to push. But I closed other pull requests today, because the original contributor was not interested in doing the changes requested. That's why I asked.

[otacke]

@Lin87 I think not all of my previous comments have been taken into consideration. But it may be best to wait for @icc here, as this may stretch beyond just the H5P integration for WordPress and perhaps should involve H5P core, too.

[otacke]

As I said, having the option to have this in other H5P integrations, too, would be benefitial. I am pinging @icc here (currently on vacation AFAIK) as I cannot tell whether that's in the interest of H5P Group.

Then getTmpPath in H5P core should return the path and either get an argument to switch between integration/temp or learn about the setting in a different fashion.

[Lin87]

See my latest comments below.

[otacke]

@Lin87 I have seen these patterns of operation before: Is this you who is coding this or an AI? Just asking ... Either way, I'll wait for @icc to comment on the general approach before pre-reviewing any further.

[Lin87]

@otacke Thank you for reviewing my pull request and sorry for the delayed response. Great catches on my mistakes. I skipped a couple of steps when I copied the code from my colleague's pull request. Good thing the code is correct on my live websites. Anyway, I made a few changes to the code.

I rewrote the getUploadedH5pFolderPath and getUploadedH5pPath functions in class-h5p-wordpress.php part and a few helper functions to do the following:

• If the Use System Temp Directory option is enabled, the plugin creates a unique parent H5P folder in the system temp directory for the installation. Instead of using the root of the system temp directory, all H5P uploads are stored in this parent folder, with each upload placed in its own unique subfolder.
• The parent temp directory is removed during uninstall if it is empty. This is an extra cleanup step, since folders in the system temp directory are typically cleared by the server anyway.
• All H5P uploads are stored in the parent directory, each within its own unique subdirectory.
• If the system temp directory is unavailable or not writable, the plugin falls back to its original temp folder.

https://github.com/Lin87/h5p-wordpress-plugin/blob/a868b4cfb929016cf4d107be438c29f2cfbd47cc/public/class-h5p-wordpress.php#L97-L183

https://github.com/Lin87/h5p-wordpress-plugin/blob/a868b4cfb929016cf4d107be438c29f2cfbd47cc/public/class-h5p-plugin.php#L1761-L1768

The h5p_use_system_temp_dir option is now removed during uninstall as well.

https://github.com/Lin87/h5p-wordpress-plugin/blob/12540941c0e00c4e463ca36eb996be10c1e812fb/public/class-h5p-plugin.php#L1738

When I took over this task some time ago, I considered proposing that these changes be made in the H5P PHP core rather than in the WordPress integration.

I decided against that approach for three reasons:

1. I am not sure where else the H5P PHP core is used.
2. The system temp directory may not be available in every environment.
3. I wanted to stay within the area I am most familiar with, which is WordPress.

That said, if the H5P Group believes using the system temp directory across all integrations would be beneficial, I would fully support that approach.

For now, however, I need this functionality to remain in the WordPress integration for the websites I currently manage at my organization. We host our WordPress sites on AWS, and user-uploaded files, including plugins, themes, and media uploads, are stored in an Amazon S3 bucket. That setup is slower and not well suited for temporary files, so using the system temp directory is important for our environment for the following benefits:

• Temporary files remain on the local filesystem instead of being sent to remote object storage.
• File operations are generally faster and have lower latency.
• It avoids unnecessary uploads, downloads, and storage requests to Amazon S3.
• Temporary files can be cleaned up automatically by the operating system or server.

P.S. I understand that I should not use the master branch for changes and pull requests. I took the easier route this time, but I was able to sync my fork of the H5P WordPress plugin with the latest code from the H5P Group repository without much difficulty. I will create a new branch for future modifications.

Regardless of whether this pull request is approved, I need to use my modified version of the plugin so that it works better in my environment. Having this option available natively in the official plugin would make things much easier, since I would not need to port or resync the changes every time a new version is released.

Please let me know if I need to make any further changes or if you have questions.

Thanks again for reviewing my changes.

Ethan

[Lin87]

@Lin87 I have seen these patterns of operation before: Is this you who is coding this or an AI? Just asking ... Either way, I'll wait for @icc to comment on the general approach before pre-reviewing any further.

Both. I coded what I wanted it to do first and then have AI code review my changes to make sure my code does not break anything else, and the format remain consistent with the rest of the plugin code. Sometimes, AI suggests better coding logic than mine, which I reviewed and agreed to the change.

[otacke]

@Lin87 sys_get_temp_dir is a general PHP function (https://www.php.net/manual/en/function.sys-get-temp-dir.php) and not WordPress specific - and you would not need to implement anything for Drupal, Moodle or whatever.

As mentioned in my initial comment, sys_get_temp_dir could as well be put into getTmpPath() in H5P core which is meant to serve the H5P integration with the temporary path. There's no need every H5P Integration would have to determine that itself, and that's why having it there would make sense. The respective H5P Integration would still need to allow choosing the target, here WordPress, but there's no need to implement this for every H5P integration at the same time.

Here's how this could look:

public function getTmpPath($target = 'host-platform') {
  $tmpPath = '';

  if ($target === 'system') {
     $tmpPath = self::getSystemTmpPath(); // needs to be implemented, e.g. like you have, but in H5P core you could simply use `self::dirReady()` to ensure a directory exists and is writable, so this could probably be refactored.
  }

  if ($target === 'host-platform' || $tmpPath === '') {
    $temp = "{$this->path}/temp";
    self::dirReady($temp);
    $tmpPath = "{$temp}/" . uniqid('h5p-');
  }

  return $tmpPath;
}

This way, nothing would change for other H5P integrations when calling getTmpPath as the argument is optional and defaults to the current logic. All that an H5P integration needs to do is to pass system if somewhere in the settings an admin chose to store tmp files on the system.

1. "I am not sure where else the H5P PHP core is used." => You would not need to know or care.
2. "The system temp directory may not be available in every environment." => PHP takes care of that for you. It returns an empty string if it cannot determine a temp directory. Has nothing to do with WordPress.
3. "I wanted to stay within the area I am most familiar with, which is WordPress." => The request was not to implement settings for Moodle, Drupal, ILIAS or other H5P integrations.

Long story short: I am not the final reviewer anyway, so @icc or someone else from H5P Group should comment further.

Oh, and my question about the use of AI was triggered by the overly verbose commenting, a common footprint.

[Lin87]

@otacke Thank you for the additional information and code sample. Is the H5P core repository open to public contributions? I noticed that it has its own public GitHub repository, but I am not sure whether pull requests there receive the same attention as contributions to the WordPress and other platform integrations.

As you said, I will wait for someone from the H5P Group to comment on this request and provide guidance on the next steps. I am fine with either approach as long as the option to use the system temp directory is available.

I intentionally forced AI to generate/keep verbose comments. They help me verify that the code does what it said it does.

Thank you again for reviewing and verifying this pull request.

[otacke]

@Lin87 All public H5P repositories are open for contributions.

Regarding code verbosity, if you need it for understanding the code, fine. But you can in fact comment too much. If you need comments to understand what the code is doing, then the code might not be up to highest standards. I can recommend to take a look at the book "Clean Code" by Robert C. Martin (or at least https://blog.cleancoder.com/uncle-bob/2017/02/23/NecessaryComments.html).

I'd request to boil down comments, but I'll leave that to H5P Group.