Adding PSR-4 support

[barakiva]

Addressing #1501 , this PR adds PSR-4 to the project. I've went in greater depth with regards to the necessity of PSR-4 in the issue. But in general, PSR-4 gives us the following:

1. Sane imports. even the strict require_once still silently executes files, introducing many code smells like polluting the global namespace
2. Better performance as imports are explicit. Even with smart compiler tree shaking, using barel files such as namespace.php with dozen of files fires off unecessary system calls in the file lookups.
3. Stricter allows us to reveal expensive runtime errors in compile time using static analysis.

[JohnVillalovos]

This PR is seeming to undo recent commits. Please fix that. Thanks.

[barakiva]

This PR is seeming to undo recent commits. Please fix that. Thanks.

It seems I have mistakenly branched off from a feature branch I was working on instead of from develop. Cleaned this up today with a rebase. Sorry for the mess.

[Copilot]

Pull request overview

This PR introduces initial PSR-4 autoloading support via Composer and begins migrating the Microsoft OAuth callback handler to a proper namespace-based import workflow, reducing reliance on require_once barrel files.

Changes:

• Add PSR-4 autoload mapping for the lib\ namespace in composer.json.
• Namespace MicrosoftOAuthCallback (and related types) and switch call sites to use imports.
• Update the Microsoft OAuth callback web entrypoint to bootstrap via Composer autoload.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
Web/microsoft-auth.php	Switches bootstrap and imports for the Microsoft OAuth callback handler.
tests/Application/Authentication/MicrosoftOAuthCallbackTest.php	Updates test to use namespaced MicrosoftOAuthCallback import.
lib/Application/Authentication/MicrosoftOAuthCallback.php	Adds namespace declaration to support PSR-4 autoloading.
composer.json	Introduces PSR-4 autoload configuration for lib\ => lib/.

[JohnVillalovos]

@barakiva What do you think of these Codex review comments?

PR 1552 review

Recommendation

Do not merge PR 1552 yet. Request changes for the production OAuth
regression and the PSR-4 design problems below.

Findings

P1: Microsoft OAuth error callbacks now return HTTP 500

Web/microsoft-auth.php replaces lib/Common/namespace.php with only the
Composer autoloader and lib/Common/Logging/Log.php. Log::Error() depends on
global classes such as Configuration, ConfigKeys, BooleanConverter, and
ServiceLocator, which are no longer loaded.

Reproducing an Azure error callback fails before the intended redirect:

Fatal error: Class "Configuration" not found

User cancellation and provider errors therefore return an HTTP 500 instead of
redirecting home. Keep the existing common bootstrap until all of its required
dependencies are migrated, or explicitly bootstrap every required dependency.
Add an entry-point test that exercises the OAuth error and logging path; the
handler unit test does not cover this integration.

P2: The auth entry point no longer registers the global exception handler

lib/Common/namespace.php also required
lib/Common/Logging/ExceptionHandler.php, which registers
set_exception_handler(['ExceptionHandler', 'Handle']). Loading only Log.php
drops that registration, so this endpoint no longer logs uncaught exceptions
the same way as sibling auth entry points such as google-auth.php and
oauth2-auth.php.

If the common bootstrap is not restored, the exception handler and all of its
dependencies must be loaded explicitly.

P2: Mapping the entire legacy lib/ tree breaks strict optimized autoload generation

composer.json declares "lib\\": "lib/", although nearly everything under
lib/ remains in the global namespace or uses an unrelated namespace. The
mapped tree also contains bundled third-party Slim code.

The following command exits with status 1 and reports 841 PSR-4 violations:

composer dump-autoload --optimize --strict-psr

This undermines the intended gradual migration. Put migrated code in an
isolated PSR-4 source subtree, such as src/, rather than declaring the whole
legacy tree PSR-4-compliant.

P2: Two newly namespaced types are not independently autoloadable

lib/Application/Authentication/MicrosoftOAuthCallback.php declares all three
of these types:

• MicrosoftOAuthCallbackOutcome
• MicrosoftOAuthCallbackResult
• MicrosoftOAuthCallback

Only MicrosoftOAuthCallback matches the filename. With a normal,
non-optimized Composer autoloader, directly resolving the result type fails:

class_exists('lib\\Application\\Authentication\\MicrosoftOAuthCallbackResult');
// false

Split the three types into files matching their class or enum names.

P3: PR and commit metadata needs cleanup

The PR title is not in conventional-commit format. Commit subjects such as
fixed typo and passed unit tests use past tense and do not clearly describe
the changes. The PR also remains blocked by an existing CHANGES_REQUESTED
review.

Before merge, use an imperative conventional title such as:

refactor(autoload): add initial PSR-4 support

[JohnVillalovos]

Here is Claude's code review

PR 1552 code review

Merge recommendation

Do not merge yet. The PR is mergeable, currently has a maintainer approval,
and its GitHub checks are green, but it introduces a reproducible production
regression in the Microsoft OAuth error path. The initial PSR-4 layout also
needs revision before it becomes the pattern for later migrations.

Findings

P1: Microsoft OAuth provider errors now cause an HTTP 500

Location: Web/microsoft-auth.php:5-17

The entry point previously loaded lib/Common/namespace.php. The PR replaces
that bootstrap with only vendor/autoload.php and
lib/Common/Logging/Log.php. Composer cannot autoload the application's legacy
global classes, but Log::Error() immediately uses Configuration,
ConfigKeys, BooleanConverter, and ServiceLocator.

The normal provider-error path can be reproduced with:

cd Web
php -d display_errors=1 -r '$_GET = ["error" => "access_denied"]; include "microsoft-auth.php";'

It terminates with:

Fatal error: Class "Configuration" not found in lib/Common/Logging/Log.php:63

An Azure error or user cancellation therefore produces a server error instead
of the intended redirect to Web. Restore the common application bootstrap
until all logging dependencies are autoloadable, or explicitly load the full
dependency set. Add an entry-point-level regression test that exercises the
error and logging branch; the handler unit test does not execute it.

P2: The auth entry point silently loses the global exception handler

Location: Web/microsoft-auth.php:5-6

lib/Common/namespace.php also loads
lib/Common/Logging/ExceptionHandler.php. That file registers
set_exception_handler(['ExceptionHandler', 'Handle']) and logs uncaught
exceptions. Loading only Log.php drops this behavior, unlike sibling callback
entry points such as Web/google-auth.php and Web/oauth2-auth.php.

Restoring the common bootstrap fixes both this problem and the missing logging
dependencies. If the bootstrap removal is deliberate, explicitly preserve the
exception-handler registration and its dependencies.

P2: The PSR-4 mapping covers the entire non-PSR-4 legacy tree

Location: composer.json:5-8

The mapping "lib\\": "lib/" tells Composer that all of lib/ is a PSR-4
base directory. That tree still contains hundreds of global classes, files
with multiple classes, the unrelated Booked namespace, and bundled Slim
sources under lib/external/.

After a fresh composer install, this check exits with status 1 and reports
841 noncompliant classes:

composer dump-autoload --optimize --strict-psr

This makes strict optimized autoload generation unusable and creates a noisy
foundation for a gradual migration. Put new PSR-4 code in an isolated source
tree such as src/ (using a project namespace such as LibreBooking) or in
another directory containing only migrated classes.

P2: Two migrated types cannot be autoloaded by their class names

Location: lib/Application/Authentication/MicrosoftOAuthCallback.php:10-74

The file declares three namespaced types:

• MicrosoftOAuthCallbackOutcome
• MicrosoftOAuthCallbackResult
• MicrosoftOAuthCallback

Only MicrosoftOAuthCallback matches the filename. In a fresh, normal Composer
installation, resolving the result type directly fails:

require 'vendor/autoload.php';
class_exists('lib\\Application\\Authentication\\MicrosoftOAuthCallbackResult');
// false

The current callback happens to work because loading MicrosoftOAuthCallback
executes the whole file and defines the companion types as a side effect. That
is the same implicit loading behavior this migration is intended to remove.
Give each class and enum a matching file.

P3: PR title and commit subjects do not follow repository conventions

The PR title, Adding PSR-4 support, is not a conventional-commit header. The
three commit subjects use gerunds or past tense, and
fix(psr): passed unit tests. Fixed typos in imports. describes test status
rather than one clear change. All three commits also have empty bodies despite
the architectural significance of the change.

If this will be squash-merged, rename the PR to an imperative conventional
title, for example:

refactor(autoload): add initial PSR-4 support

Otherwise, clean up the individual commit messages before merge.

Validation

The following were run at head commit
47327ce1e4caa7efddcdd06a137b9f76811311c0 after deleting vendor/ and
performing a fresh composer install:

• Full PHPUnit suite: passed, 1,645 tests and 7,438 assertions.
• PHPStan base configuration: passed with zero errors.
• PHPStan next configuration: passed with zero errors.
• Focused PHP-CS-Fixer check: passed for all three changed PHP files.
• Composer manifest validation: passed.
• git diff --check: passed.
• Microsoft OAuth error-path reproduction: failed with missing Configuration.
• Direct MicrosoftOAuthCallbackResult autoload probe: returned false.
• Strict optimized Composer autoload generation: failed with 841 violations.

GitHub CI is green across PHP 8.2-8.5, linting, static analysis,
documentation, frontend linting, and integration smoke tests. Those checks do
not exercise the web entry point's provider-error/logging branch or strict
optimized Composer autoload generation.

[barakiva]

Here is Claude's code review

PR 1552 code review

Merge recommendation

Do not merge yet. The PR is mergeable, currently has a maintainer approval, and its GitHub checks are green, but it introduces a reproducible production regression in the Microsoft OAuth error path. The initial PSR-4 layout also needs revision before it becomes the pattern for later migrations.

Findings

P1: Microsoft OAuth provider errors now cause an HTTP 500

Location: Web/microsoft-auth.php:5-17

The entry point previously loaded lib/Common/namespace.php. The PR replaces that bootstrap with only vendor/autoload.php and lib/Common/Logging/Log.php. Composer cannot autoload the application's legacy global classes, but Log::Error() immediately uses Configuration, ConfigKeys, BooleanConverter, and ServiceLocator.

The normal provider-error path can be reproduced with:

cd Web
php -d display_errors=1 -r '$_GET = ["error" => "access_denied"]; include "microsoft-auth.php";'

It terminates with:

Fatal error: Class "Configuration" not found in lib/Common/Logging/Log.php:63

An Azure error or user cancellation therefore produces a server error instead of the intended redirect to Web. Restore the common application bootstrap until all logging dependencies are autoloadable, or explicitly load the full dependency set. Add an entry-point-level regression test that exercises the error and logging branch; the handler unit test does not execute it.

P2: The auth entry point silently loses the global exception handler

Location: Web/microsoft-auth.php:5-6

lib/Common/namespace.php also loads lib/Common/Logging/ExceptionHandler.php. That file registers set_exception_handler(['ExceptionHandler', 'Handle']) and logs uncaught exceptions. Loading only Log.php drops this behavior, unlike sibling callback entry points such as Web/google-auth.php and Web/oauth2-auth.php.

Restoring the common bootstrap fixes both this problem and the missing logging dependencies. If the bootstrap removal is deliberate, explicitly preserve the exception-handler registration and its dependencies.

P2: The PSR-4 mapping covers the entire non-PSR-4 legacy tree

Location: composer.json:5-8

The mapping "lib\\": "lib/" tells Composer that all of lib/ is a PSR-4 base directory. That tree still contains hundreds of global classes, files with multiple classes, the unrelated Booked namespace, and bundled Slim sources under lib/external/.

After a fresh composer install, this check exits with status 1 and reports 841 noncompliant classes:

composer dump-autoload --optimize --strict-psr

This makes strict optimized autoload generation unusable and creates a noisy foundation for a gradual migration. Put new PSR-4 code in an isolated source tree such as src/ (using a project namespace such as LibreBooking) or in another directory containing only migrated classes.

P2: Two migrated types cannot be autoloaded by their class names

Location: lib/Application/Authentication/MicrosoftOAuthCallback.php:10-74

The file declares three namespaced types:

• MicrosoftOAuthCallbackOutcome
• MicrosoftOAuthCallbackResult
• MicrosoftOAuthCallback

Only MicrosoftOAuthCallback matches the filename. In a fresh, normal Composer installation, resolving the result type directly fails:

require 'vendor/autoload.php';
class_exists('lib\\Application\\Authentication\\MicrosoftOAuthCallbackResult');
// false

The current callback happens to work because loading MicrosoftOAuthCallback executes the whole file and defines the companion types as a side effect. That is the same implicit loading behavior this migration is intended to remove. Give each class and enum a matching file.

P3: PR title and commit subjects do not follow repository conventions

The PR title, Adding PSR-4 support, is not a conventional-commit header. The three commit subjects use gerunds or past tense, and fix(psr): passed unit tests. Fixed typos in imports. describes test status rather than one clear change. All three commits also have empty bodies despite the architectural significance of the change.

If this will be squash-merged, rename the PR to an imperative conventional title, for example:

refactor(autoload): add initial PSR-4 support

Otherwise, clean up the individual commit messages before merge.

Validation

The following were run at head commit 47327ce1e4caa7efddcdd06a137b9f76811311c0 after deleting vendor/ and performing a fresh composer install:

• Full PHPUnit suite: passed, 1,645 tests and 7,438 assertions.
• PHPStan base configuration: passed with zero errors.
• PHPStan next configuration: passed with zero errors.
• Focused PHP-CS-Fixer check: passed for all three changed PHP files.
• Composer manifest validation: passed.
• git diff --check: passed.
• Microsoft OAuth error-path reproduction: failed with missing Configuration.
• Direct MicrosoftOAuthCallbackResult autoload probe: returned false.
• Strict optimized Composer autoload generation: failed with 841 violations.

GitHub CI is green across PHP 8.2-8.5, linting, static analysis, documentation, frontend linting, and integration smoke tests. Those checks do not exercise the web entry point's provider-error/logging branch or strict optimized Composer autoload generation.

Can you please explain P2 to me? I've went over a few PSR-4 articles as well as composer docs, and had claude explain it to me. I was under the impression migrations are gradual. So why is it that adding 'lib' breaks 800+ files if I didn't add use and namepsace to them?

I've prompted the AI over and over to explain p2 to me but it's all just nonsense I need a human to explain this to me cause literally the reason I've made the PR was due to multiple articles explaning PSR-4 code and require can codexist.

[JohnVillalovos]

@barakiva I'll attempt to take a look at it more this weekend. No guarantees as the family might have other plans 😄

Thanks.

[JohnVillalovos]

Can you please explain P2 to me? I've went over a few PSR-4 articles as well as composer docs, and had claude explain it to me. I was under the impression migrations are gradual. So why is it that adding 'lib' breaks 800+ files if I didn't add use and namepsace to them?

I used Codex and Claude to help me research this. This is my initial research but I need to do more.

Run this command with the current code in this PR: composer dump-autoload --optimize and you can see all the errors.

So doing some research it sounds like we might should do is to create a new directory off of the root called src/ (it could be called something else but 'src' sounds good to me). And then that will be what will go into composer.json in the psr-4 section. Then files will get moved from lib/ to src/

For example would take lib/Application/Authentication/MicrosoftOAuthCallback.php and create

  src/Application/Authentication/
  ├── MicrosoftOAuthCallback.php
  ├── MicrosoftOAuthCallbackOutcome.php
  └── MicrosoftOAuthCallbackResult.php

The one file becomes three files because each class is supposed to be in its own file.

[JohnVillalovos]

Codex PR 1552 PSR-4 Migration Plan

Prompt

Let's say we are starting from scratch before this PR. And we want to setup
PSR-4 support as this PR wants to do. And we will first migrate
'lib/Application/Authentication/MicrosoftOAuthCallback.php' to be PSR-4.
Explain the steps that should do this.

Objective

Configure a clean PSR-4 source root and migrate the Microsoft OAuth callback
component without disrupting LibreBooking's legacy include system.

PSR-4 migrates named PHP types rather than arbitrary files. The existing
lib/Application/Authentication/MicrosoftOAuthCallback.php file contains one
enum and two classes, so all three types need to be considered as part of this
migration.

1. Establish the PSR-4 source root

Create a top-level src/ directory and map the LibreBooking\ project
namespace to it in composer.json:

"autoload": {
    "psr-4": {
        "LibreBooking\\": "src/"
    }
}

The mapping means:

LibreBooking\Foo\Bar -> src/Foo/Bar.php

Do not map the legacy lib/ directory. Most of that directory does not follow
PSR-4 and must continue using the existing include mechanism during the
incremental migration.

2. Split and move the three types

The existing file declares:

• MicrosoftOAuthCallback
• MicrosoftOAuthCallbackResult
• MicrosoftOAuthCallbackOutcome

Move those declarations into separate files:

src/
└── Application/
    └── Authentication/
        ├── MicrosoftOAuthCallback.php
        ├── MicrosoftOAuthCallbackOutcome.php
        └── MicrosoftOAuthCallbackResult.php

Each file should contain declare(strict_types=1) and use this namespace:

namespace LibreBooking\Application\Authentication;

Because the types share a namespace, their references to one another do not
need additional use statements.

After the declarations have moved, delete:

lib/Application/Authentication/MicrosoftOAuthCallback.php

3. Generate the Composer autoloader

Run:

composer dump-autoload

This regenerates Composer's derived autoload configuration with the new
mapping. Do not commit generated files under vendor/.

4. Update the web entry point

Update Web/microsoft-auth.php to import the migrated class:

use LibreBooking\Application\Authentication\MicrosoftOAuthCallback;

The entry point should explicitly load both autoloading systems:

require_once ROOT_DIR . 'vendor/autoload.php';
require_once ROOT_DIR . 'lib/Common/namespace.php';

They have separate responsibilities:

• vendor/autoload.php loads the new PSR-4 classes.
• lib/Common/namespace.php loads the legacy global classes required by
  Log and registers the existing global exception handler.

Remove the old direct include:

require_once ROOT_DIR . 'lib/Application/Authentication/MicrosoftOAuthCallback.php';

That file no longer exists after the migration.

5. Update the unit test

Replace the test's direct require_once with:

use LibreBooking\Application\Authentication\MicrosoftOAuthCallback;

Confirm that the test runner loads vendor/autoload.php. The repository's
Composer-installed PHPUnit runner already does this. Making the dependency
explicit in the shared test bootstrap can be considered separately if the
project needs to support other test runners.

The existing callback tests should naturally exercise autoloading of all three
types:

MicrosoftOAuthCallback
        ↓ returns
MicrosoftOAuthCallbackResult
        ↓ contains
MicrosoftOAuthCallbackOutcome

6. Add a bootstrap regression test

Add coverage for the actual Web/microsoft-auth.php error path, not only the
callback class.

The test should demonstrate that an input such as:

?error=access_denied

can:

• construct the callback handler through Composer;
• use the legacy logging system;
• redirect to the failure destination;
• avoid a fatal missing-class error.

This test would catch the current PR's Configuration not found regression.

7. Validate the new boundary

First verify PSR-4 compliance:

composer dump-autoload --optimize --strict-psr

Because Composer now scans only the clean src/ tree, this should not produce
the hundreds of legacy lib/ violations.

Then run the normal project checks:

composer validate
composer phpcsfixer:lint
composer phpstan
composer phpstan_next
composer phpunit

Expected result

The Microsoft OAuth callback component uses PSR-4, while the rest of lib/
continues working through the existing include system. Future pull requests can
migrate additional components into src/ one logical area at a time.

[JohnVillalovos]

I added the composer dump-autoload --optimize to the CI. And now the CI is failing for this PR.

[JohnVillalovos]

@barakiva I re-worked this and created PR #1610

Thanks for this. I marked you as a co author on PR #1610

[barakiva]

@barakiva I re-worked this and created PR #1610

Thanks for this. I marked you as a co author on PR #1610

What a rabbit hole! I thought I could iterate on PSR-4 as the /src move seemed too ambitious for 1 PR.