Skip to content

Conversation

@ryanrath
Copy link
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.
ryanrath and others added 15 commits December 1, 2025 14:00
APP_SECRET population will be moved so that it's handled as part of the
General Setup ( option 1 in xdmod-setup ).
These changes update the way that the `.env` file is generated.
- `.env` is now excluded from being included when building an rpm or source
install.
- `.env` is now created using a new template `templates/env.template` at the end
of the `1. General Setup` process in `xdmod-setup`.
- We also use Symfony to pre-compile the `.env` file so that it's not read on
every request.

The xdmod-setup-start.tcl file has been updated to take this second file
generation into account.
This change allows for the `bin/console` file to be used while installed
via RPM or Source *and* while doing development work within the XDMoD
git repo.
SRI is subresource integrity and informs the browser to check the
requested script resource against the provided cryptographic hash. Here
is the page used as a reference for changes: https://developer.mozilla.org/en-US/docs/Web/Security/Defenses/Subresource_Integrity
and here is the url where the error can be seen: https://sonarcloud.io/project/security_hotspots?id=ubccr_xdmod&pullRequest=2097&sinceLeakPeriod=true
Fixing issue found here: https://app.circleci.com/pipelines/github/ubccr/xdmod/4752/workflows/29b1a380-8ab1-4cb5-872d-943b90d9ca53/jobs/12063
by replacing `strpos` usage ( which does not allow null haystacks ) to
`str_contains` which does.
* This is handle a deprecation

* removing unneeded security config

* not having this set is a deprecation

* Cleaning up the authentication Controller

* Updating doc blocks and removing unneeded logging

* just adding comments on why the line was changed

* Changes to make review less insanity inducing
* Simplify the export process for XDMoD. Also fixes a bug with timezones not being considered for image export
}
$factoryClassName = $configObj->class;
} elseif ( false === strpos($factoryClassName, '\\') && 'static' != $factoryClassName ) {
} elseif (false === strpos($factoryClassName, '\\') && 'static' != $factoryClassName) {
Copy link
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
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 6 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>
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