Skip to content

Symfony Migration#2145

Open
ryanrath wants to merge 86 commits intomainfrom
symfony_migration
Open

Symfony Migration#2145
ryanrath wants to merge 86 commits intomainfrom
symfony_migration

Conversation

@ryanrath
Copy link
Copy Markdown
Contributor

Description

This is the main migration PR for Symfony.

Motivation and Context

Tests performed

Checklist:

  • The pull request description is suitable for a Changelog entry
  • The milestone is set correctly on the pull request
  • The appropriate labels have been added to the pull request

These changes provide the initial migration of XDMoD's rest framework to Symfony
For whatever reason the `/about/*.php` routes would not match, even
though we have other routes w/ `.php` in the url. I've updated the about
url's that included `.php` to `.html`. I will revisit these changes in
the future.
These changes update so that Symfony code logs to
/var/log/xdmod/exceptions.log
These are readability changes meant to make it clear to the reader that
the roles are in fact xdmod roles which are different then Symfony
Roles.
These changes ensures that we have PHP 8.2 installed w/ all of the dependencies
in the XDMoD docker container. Once the XDMoD image is updated to PHP 8.2 by
default then these can be removed.
This change ensures that we only copy the playwright related files into the
playwright container. While it may only save a second or two, it also makes it
clear to anybody who works / reads this later that the full xdmod src is not
necessary for the Playwright Tests.
It turns out that we needed a few more directories for playwright tests to be
happy, these are them.
These are all changes related to account for PHP 8.2 being more strict about
types, as well as moving away from having our own logging levels to just using
Monolog Level values.

`classes/CCR/CCRDBFormatter`:
- Updated the type of `$record` to `LogRecord` from `array` so that it matches
`Monolog\Formatter\NormalizerFormatter::format`.

`classes/CCR/CCRDBHandler`:
- Updated the `$level` default value to be `Monolog\Level:Debug` as opposed to
out custom `DEBUG` level.
- Removed extraneous code / variables from the `write` function.

`classes/CCR/CCRLineFormatter`:
- Updated the type of `$record` to `LogRecord` from `array` so that it matches
`Monolog\Formatter\LineFormatter::format`.

`classes/CCR/Log.php`:
- Changed from using custom int log levels (0-7) to the int levels that Monolog
provides.
- Removed the `convertToMonologLevel` and `convertToCCRLevel` functions and
their usage since they are no longer needed ( because we're standardizing on
Monolog levels now ).

`classes/CCR/Loggable.php`:
- Removed usage of the `convertToMonologLevel` function.

`tests/component/lib/ETL/IngestorTest.php`:
- By default Monolog outputs it's log levels in upper case and since we've
chosen to go with Monolog defaults elsewhere we are here as well.

`tests/component/lib/LoggerTest.php`:
- Updated to use default Monolog log levels ( e.g. Upper Case ).
- Updated to use the default integer values for Monolog log levels.
`tests/post/lib/Controllers/MetricExplorerControllerTest.php`:
- Just removing the `/` from the begining of the URLs to play nice with Symfony
routing.

`tests/post/lib/Rest/JobViewerTest.php`:
- PHP 8.2 does not like dynamically created properties, this change fixes that.
These changes update `src/Security/Helpers/Tokens.php` with the latest logic
from `classes/Models/Services/Tokens.php` with one minor modification. The
callers of `src/Security/Helpers/Tokens::authenticate` expect that if there is no token
present, null will be returned and they can fall back to standard user /
password authentication, but the code in `classes/Models/Services/Tokens.php`
only ever threw exceptions. I added a `$strict` arguemnt to the `Helpers/Tokens::authenticate`
function that is true by default ( which replicates the original behavior of
`Services/Tokens` ) but set it to false for all the new rest endpoints that
utilized the old `authenticateToken` function.

I also removed the `classes/Models/Services/Tokens.php` class as it's no longer
used.
This reverts commit a3c558a.
These changes are the minimum required to get our logging to work w/ PHP 8.2 and
the latest version of Monolog. They consist of the following:

`classes/CCR/CCRDBFormatter.php`:
- Updating the type of the `$record` format to `Monolog\LogRecord`. This is to
  match the function signature of `Monolog\Formatter\NormalizerFormatter`

`classes/CCR/CCRLineFormatter.php`:
- Updating the type of the `$record` format to `Monolog\LogRecord`. This is to
  match the function signature of `Monolog\Formatter\LineFormatter`.

`classes/CCR/CCRDBHandler.php`:
- This code is no longer needed as we do all the formatting for db log records
in `CCRDBFormatter`.

`classes/CCR/Log.php`:
- Since we handle the conversion of the log level in the constructor of
  `CCRDBHandler`, this code can be removed.

`tests/component/lib/ETL/IngestorTest.php`:
- The default format for log level in Monolog messages is to be upper case. We
  previously expected them to be lower case which is likely a hold over from old
  PSR logging. We've decided to use standards where ever it makes sense / can, hence this
  change.

`tests/component/lib/LoggerTest.php`:
- Same reason / changes as for `IngestorTest`.
The functionality contained in this file has been moved to
HomeController / index.html.twig.
Comment thread classes/Realm/Realm.php
}
$factoryClassName = $configObj->class;
} elseif ( false === strpos($factoryClassName, '\\') && 'static' != $factoryClassName ) {
} elseif (false === strpos($factoryClassName, '\\') && 'static' != $factoryClassName) {
Copy link
Copy Markdown
Contributor

@eiffel777 eiffel777 Jan 15, 2026

Choose a reason for hiding this comment

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

Because of the change on line 377, changing what values $factoryClassName might be, should 'static' != $factoryClassName test a value different then static? If I am understanding the code correctly, maybe Realm::class?

Copy link
Copy Markdown
Contributor Author

@ryanrath ryanrath Jan 15, 2026

Choose a reason for hiding this comment

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

So I think it can actually be removed. Some quick context:

This function is called in three places / functions:

  • Realm::getGroupByObjects: return static::getSortedObjectList('GroupBy', ...);
  • Realm::getRealmObjects: return static::getSortedObjectList('Realm', ...);
  • Realm::getStatisticObjects: return static::getSortedObjectList('Statistic', ...);

The value of $factory (a string, the function that's going to ultimately be executed ) w/ the original code is:

static::factory           # $className = 'Realm';
\Realm\GroupBy::factory   # $className = 'GroupBy';
\Realm\Statistic::factory # $className = 'Statistic';

Now, I'm not sure exactly why whoever wrote this subbed in static for Realm, but I do know that it / that line isn't actually needed and can be simplified to the following:

function getSortedObjectList(
    string $className,
    object $configObj,
    ...
)
{
    ...
    // No need for `$factoryClassName = ('Realm' == $className ? 'static' : $className);` 
    // Allows for the configuring of a different class to handle GroupBys or Statistics in datawarehouse.json
    if ( 'Realm' != $className && isset($configObj->class) ) {
        if ( ! class_exists($configObj->class) ) {
            $msg = sprintf("Attempt to instantiate undefined %s class %s", $className, $configObj->class);
            throw new \Exception($msg);
        }
        $factoryClassName = $configObj->class;
    // NOTE: this is an `else` instead of an `elseif` 
    } else {
        // Add the __NAMESPACE__ to the provided $className.
        $factoryClassName = sprintf('\\%s\\%s', __NAMESPACE__, $className);
    }
    $factoryCallable = [$factoryClassName, 'factory'];

    ...
}

If we look at $factory / $factoryCallable for the original code and my proposed simplified code above it looks something like:

Original:
"static::factory"
"\\Realm\\GroupBy::factory"
"\\Realm\\Statistic::factory"
***
New Updated:
["\\Realm\\Realm","factory"]
["\\Realm\\GroupBy","factory"]
["\\Realm\\Statistic","factory"]

Thoughts?

ryanrath and others added 26 commits January 20, 2026 10:14
* Syncing contents w/ main

* reverting change to `deleteRequests`
* Standardizing Exceptions thrown

* Minimizing changes for DashboardController
* Reverting unneeded space change

* Updating comment to be more intelligible
* updating playwright version to 1.58.0 (#2153)

* updated xdmod-admin to remove jobs for a resource in shredder (#2155)

updated to allow removal of jobs for a given resource in xdmod-admin

---------

Co-authored-by: Rose Tovar <15618284+rvtovar@users.noreply.github.com>
- Added the error codes from `\XDError::getErrorCodes` to the internal dashboard
controller / twig template and removed refernces to `Error.js.php` as it no
longer exists.
- Updated the version of jquery for the internal dashboard login page to the
version that XDMoD provides.
- Moved the `ext-stop-flash-check.js` to the bottom of `body` so that
`document.body` is populated before it runs.
Whatever reason I had to change the way `$orgConfig` was processed was
is no longer relevant so these changes revert that ( no need for the
array_pop ). and Updated the name to just `$org`.
* updating playwright version to 1.58.0 (#2153)

* updated xdmod-admin to remove jobs for a resource in shredder (#2155)

updated to allow removal of jobs for a given resource in xdmod-admin

* Updating Commands and Faq to explain new Command for Xdmod-admin (#2157)

Updates to docs for new command to delete jobs for a given resource. 

Authored-by: Alex Tovar

Co-authored-by: Aaron Weeden <31246768+aaronweeden@users.noreply.github.com>

* Updates to support the Allocations Realm (#2122)

* Updates to support the allocations realm changes.

* Fix typo in journal helper

* Add migration and tests.

* Create scheam for upgrade path

* Linting fixes

* Add documentation for new functions

---------

Co-authored-by: ryanrath <ryanrath@buffalo.edu>
Co-authored-by: Rose Tovar <15618284+rvtovar@users.noreply.github.com>
Co-authored-by: Aaron Weeden <31246768+aaronweeden@users.noreply.github.com>
These errors were in the original non-symfony code, but the file
move caused them to be alerted on. Might as well fix now we know
about them (low risk due to being in test code).
* Making Symfony Command Execution Programmatic

These changes allow us to execute Symfony commands programmatically as
opposed to using `exec` to start a new process. This means that we are
not dependent on there being a `bin/console` file being present.
These changes remove comments that previously mentioned a now removed
url path `gui/general/login.php`. The functions that these comments were
above have been removed as they are not used.
* Fixing routes that use a {prefix}

Routes with `{prefix}/` were broken in some cases due to `/` also being
matched by `.*`. Replacing `{prefix}/` with just `{prefix}` means these
routes function as intended.

* Fixing log dir generation during xdmod-setup

* Updating to use Symfony vs PHP Super Global

* Removing unneeded code

* Adding ReportBuilder route

This is a route that's used by selecting the "Download" button from the
"My Reports" section of the Report Generator.

* Added missing else-case

This else-case was missed in initial migration.

* Updating jwt-redirect to work with prefix

* Adding simplesamlphp oauth2/oidc module

* Adding simplesamlphp authorize module

* Renaming login_link to idp_host for clarity

So what the old `services.yaml#sso::login_link` property really was
meant to be was the referrer value that is included when the Identity
Provider makes a call to XDMoD's callback url so that we can
specifically identify the "entry point" of SSO authentication as opposed
to attempting to authenticate each and every request.

* Symfony related httpd changes

* Updates to SSO Setup / Config

These changes clarify what's actually being done to configure SSO for XDMoD and
allow our current setup to work w/ how we actually tell people to setup
SimpleSAMLPhp in production as opposed to how we do it for our CI Pipeline. An
update to our CI SSO Setup will be forthcoming.

* Adding a command to update sso::auth_referrer

* Needed to pass the url parameter correctly

* Command needs to be the first entry in options

* removing the sso parametrers and using portal_settings.ini

* removing XDMOD_LOG_DIR from .env

* removing Error.js.php reference

* Replacing ErrorController w/ RestExceptionListner

So not only was ErrorController essentially swallowing exceptions, it
didn't actually do what we needed it to ( catch thrown exceptions and
return 200OK w/ a json body ).

To actually implement the handling of Exceptions I've added a
RestExceptionListener that hooks into Symfony Kernel's life-cycle by listening for
 kernel.exception events, which occur when an exception is thrown while
processing a request.

Manual testing was done by manually throwing a `BadRequestHttpException`
in UserInterfaceController.php `index` function. The expected outcome
is that an error message is presented to the user in a js modal window.

* Being explicit about calling functions in twig

* Turns out ErrorController was needed

It looks like ErrorController really is needed, after removing it and
adding RestExceptionListener roughly 40-50 integration tests failed due
to http status code differences. It looks like identifying the endpoints
that provide error messages to a user need to be identified and
specifically handled as opposed to a blanket approach.

* Removing last ref to gui/general/login.php

* Removing usage of PHP Super Globals

These changes are in reference to:
https://github.com/orgs/ubccr/projects/5/views/1?pane=issue&itemId=142956534&issue=ubccr%7Cxdmod-xsede%7C1048
 which replaces usage of the PHP Super Global `$_REQUEST` w/ Symfony's Request.

* re-removing a change

This change had been removed in a commit previously, but must have been lost
somewhere along the way.

* Updating preload to use prod instead of dev

* Normalize processing of `operation` parameters.

These changes came about when investigating what was causing symfony-dev to
"hang" with no error thrown.

The root cause of the problem was a new group by
that had been added to xdmod-dev / mysql-dev, but was not present in
symfony-dev's config files. But, it did surface a bug in the Symfony
implementation for the old controller endpoints. Specfically how they were ( or
were not ) dealing with exceptions.

By default Symfony will return a non-200 HTTP Status Code when an exception is
thrown. This is all well and good for "modern" front end's, but ExtJS 3.4 was
written in a by gone era where apparently web servers were supposed to return
200 OK regardless of exceptions.

These changes make it so that we explicitely wrap these old controller routes in
try-catch blocks and put a good face on things so that ExtJS can merrily get on
with life.

* Changes per @aestoltm

* changes per @aestoltm

* Fixing InternalDashboard login/logout

In this commit I've updated the internal dashboard login page so that it
is visually consistent with our current internal dashboard login page
and fixed the InternalDashboard logout functionality to work as it does
now.

Manual testing was done to confirm these changes work as intended.

* missed actually including internal_dashboard twig tmplate

* Adding missing controller

This controller provides the end point for user password resets.

* Actual fixes for ErrorController

So with the ultimate goal of probably removing linker.php this commit
migrates the `HttpCodeMessages` class to `src/Helper/` and implements
the same logic as the unhandled exception handler in `linker.php`.

Testing was done via the automated tests in docker.

* Adding CORS listener

* Adding portal_settings access in Symfony Parameter

This commit adds the the contents of portal_settings.[ini,d] to
Symfony's parameters. This only occurs once during cacheing, not on
every request. This was verified by adding a logging statement to the
function and clearing the cache. This generated a log message as opposed
to when I created a test endpoint that referenced
`xdmod.portal_settings.general.title` and returned it in a json
response and repeatedly requested the endpoint 5-10 times and no log
messages were generated.

* Simplifying the handling of .env

This commit's changes take into account that while we don't actually need the
contents of the `.env` file thanks to the new EnvVarLoaders, it does still need
to exist for Symfony to function properly.

* Removing references to dead code

This class no longer exists and having it referenced here causes Symfony to
throw an exception.

* Updates to support Symfony portal_settings

- These changes are related to the .env removal and accessing
  portal_settings in Symfony code.
- Updated controllers so that they now use `$this->parameters->get` as
  opposed to `\xd_utilities\getConfiguration`.
- Updated `index.html.twig` to remove the integrity property from the
  recaptch script element as it's not needed.
- Added `src/EnvVar/*` classes that populate the needed environment
  variables for Symfony and for ReCaptcha thus letting us get around the
need for a .env file that is ever modified.
- Now that we don't need to modify the .env file, we just need it to
  exist, `xdmod-setup-start.tcl` has been updated since we no longer
  prompt to write to it.

* just adding context with comments

* Fixing SimpleSamlPhp for use in our docker env

* Removing debugging code

* Updates per code review comments by @connersaeli

This change is based on recmmendations by @connersaeli in ubccr/xdmod-xsede#1182

* changes per @connersaeli

* Updates per @aestotlm

These changes are in regards to: ubccr/xdmod-xsede#1194

* Updates per @connersaeli

This is to address @connersali 's comment in ubccr/xdmod-xsede#1195

* Removing per @aestoltm

* removing unneeded code

* This goes with the removal of CCR\Logger

* removing unneeded code

* removing unneeded code

* removing unneeded config

* Removing debug logger

* Updating samlSetup

* Appeasing the style gods

* Adding file logging to samlSetup.sh

* Normalizing network name and playwright version

* Fixing up password_reset route and emails

This commit reduces duplicate code by refactoring the user password
reset emailing to a helper service, which is then used by both the
resetting of a password in internal_dashboard and a regular user
requesting a password reset.

I also fixed problems w/ the `mode` and the image path that it's based
on.

* More appeasing

* Fixing updates

* Making SonarQube happy

* Add the auth_referer

* Updating to fix logic
* Refactoring dotenv setup and migration.

* Fixing incorrect import.
* Reviewing for Symfony Migration

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants