Skip to content

[5.0] Allow JPATH constants overridable and make SiteRouter JPATH_PUBLIC aware#41570

Merged
HLeithner merged 2 commits intojoomla:5.0-devfrom
HLeithner:5.0/feature/public
Sep 4, 2023
Merged

[5.0] Allow JPATH constants overridable and make SiteRouter JPATH_PUBLIC aware#41570
HLeithner merged 2 commits intojoomla:5.0-devfrom
HLeithner:5.0/feature/public

Conversation

@HLeithner
Copy link
Member

Useful for public folder for example or only partial overrides.

Followup Pull Request for #40509 and prepare Pull Request for #41446.

Summary of Changes

Allow each JPATH constant to be overridden individually which allow us to only define 3 constants in the public folder index.php or any other bootstrap file.

Remove the need of _JDEFINED if custom constants have been defined.

SiteRouter uses now the JPATH_PUBLIC constant instead of the JPATH_SITE constant to find the index.php in the path to removes it from the request uri object.

@dgrammatiko @wilsonge

Testing Instructions

joomla frontend, backend and api should work normally, with and without public folder

Actual result BEFORE applying this Pull Request

Works
More files in public folder

Expected result AFTER applying this Pull Request

Works
Only index.php and extract.php in public folder.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

Useful for public folder for example or only partial overrides.
Co-authored-by: Dimitris Grammatikogiannis <dg@dgrammatiko.dev>
@Fedik
Copy link
Member

Fedik commented Sep 4, 2023

TBH, looks dirty to me.

For #41446 we can do similar to if (file_exists(dirname(__DIR__) . '/defines.php')):

if (file_exists(__DIR__ . '/defines-public-folder.php')) {
    include_once __DIR__ . '/defines-public-folder.php';
}

And you only check for

defined('JPATH_ROOT') || define('JPATH_ROOT', implode(DIRECTORY_SEPARATOR, $parts));
defined('JPATH_SITE') || define('JPATH_SITE', JPATH_ROOT);
defined('JPATH_PUBLIC') || define('JPATH_PUBLIC', JPATH_ROOT);

Then CLI script should generate only defines-public-folder.php not index.php

app.php then will be like:

...
if (file_exists(dirname(__DIR__) . '/defines.php')) {
    include_once dirname(__DIR__) . '/defines.php';
}
if (file_exists(__DIR__ . '/defines-public-folder.php')) {
    include_once __DIR__ . '/defines-public-folder.php';
}

if (!defined('_JDEFINES')) {
    define('JPATH_BASE', dirname(__DIR__));
    require_once JPATH_BASE . '/includes/defines.php';
}
...

Or we better call it defines-generated.php or defines-custom.php

@HLeithner
Copy link
Member Author

I never understood way I only can override one variable or all in defines...
Because this means we broke all sites that uses it own defines file when we added the JPATH_API constant.

That's the reason I simple allow to override any constant for simplicity.

@dgrammatiko
Copy link
Contributor

Then CLI script should generate only defines-public-folder.php not index.php

That was my initial approach to have only a custom defines (well I actually did a bit more) but I also like this take. It's simpler IMHO

@Fedik
Copy link
Member

Fedik commented Sep 4, 2023

Well, I wrote what I not like 😉

At least please avoid hacking of index.php. create a separated file for generated defines.

@dgrammatiko
Copy link
Contributor

I'm ok with either approaches, just make up your mind and let me know what will it be 😁

@HLeithner HLeithner merged commit 56ae4d0 into joomla:5.0-dev Sep 4, 2023
@HLeithner
Copy link
Member Author

I'm merging this for beta1 so we see any site effects and as it is needed as preparation for #41446

@HLeithner HLeithner deleted the 5.0/feature/public branch September 4, 2023 09:36
@HLeithner HLeithner added this to the Joomla! 5.0 milestone Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants