Skip to content

Migrate Silex to Symfony #2030

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 108 commits into
base: main
Choose a base branch
from
Draft

Conversation

ryanrath
Copy link
Contributor

Description

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

Ryan Rathsam and others added 30 commits February 9, 2024 10:30
Included in this commit is:
  - Added the symfony directories / files this includees:
    - config/
    - src/
    - templates/
  - In an effort to keep the Symfony infrastructure seperate from the
    Silex / historical system I've set the "Symfony App" to be served
    from the public directory. This is why public is essentially the
    same as html just without the `controllers/` and
    `internal_dashboard` directories.
  - Rest Endpoints migrated so far ( OpenXDMoD ):
    - Summary Tab
    - Usage Tab
    - Metric Explorer Tab
    - Data Export Tab
    - Job Viewer Tab
Included in this commit is:
  - Added the symfony directories / files this includees:
    - config/
    - src/
    - templates/
  - In an effort to keep the Symfony infrastructure seperate from the
    Silex / historical system I've set the "Symfony App" to be served
    from the public directory. This is why public is essentially the
    same as html just without the `controllers/` and
    `internal_dashboard` directories.
  - Rest Endpoints migrated so far ( OpenXDMoD ):
    - Summary Tab
    - Usage Tab
    - Metric Explorer Tab
    - Data Export Tab
    - Job Viewer Tab
I just want to checkpoint the changes I've made so far so as not to lose
anything in the case of a problem w/ my development block.
- Migrated internal_dashboard/controllers/mailer.php to a Symfony
  Controller
  - for enum_target_addresses and send_plain_mail operations
- Updated the expected output for the enum_target_addresses tests so
  that they're in line with the new controller.
Since this is going in to xdmod11.0 we will only be supporting  el8 builds and
as such only need one composer.json file again ( and there was much rejoicing! )
These changes add the ability to use API tokens to the new REST stack.
  - BaseController.php: Added a new `$tokenHelper` member variable so that child classes can support
    token authentication
  - WarehouseExportController.php:
    - Utilized the new `$tokenHelper` instance to implement token authentication
      for the `getRealms` function.
    - Updated the way that authentication was handled so that tests passed for:
      - `getRequests`
      - `createRequests`
    - Updated `deleteRequests` so that the request ids are provided as a
      parameter vs in the body of the requests due to DELETE requests not having
      a body.
This function needs to be called to ensure that user login counts are properly
recorded ( via the SessionManager ).
Somewhat Automated error handling that helps keep the same behavior from the old
rest stack.
empty will allow us to also capture when the userid value is an empty string as
opposed to just null.
These changes allow us to mimic the same responses as were received from the old
rest framework.
The previous function `CCR.invokePost` would do a pre-flight check to ensure
that a user was authenticated / authorized to make the call before actually
making it. That file is no longer needed / exists so this function call was
changed to be the same as what occurs when a user was determined to be
authorized. The authentication / authorization is now handled on the backend for
each rest call.
These changes were just needed beacuse of the new version of PHPUnit.
Due to the way that the new rest stack interprets urls the leading `/` needed to
be removed from these tests.
Since we can now return status codes other than 200, this let's us have an
expected http_code.
This form of `assertEquals` has been deprecated in the newer versions of
PHPUnit, this commit just updates them to the currently supported version called
`assertEqualsWithDelta`.
The error messages for missing or malformed parameter values has been updated
slightly in the new REST stack, the message still indicate the same occurence,
but the wording is different.
These changes should have been in the previous commit.
This reflects the new REST url. A quick note, this change is accounted for on
the front end by setting the two REST properties in portal_settings.ini to an
empty string. Which is then js variable `XDMoD.rest.url`.
The current rest stack defaults to setting the content-type to application/json
when it returns json so where before we would be getting back "json" but
expected the content-type to be `text/html` we can now update it to reflect the
new content-type.

Included in this commit are also some massaging of the expected http code, most
often it's expecting a 4** error code where before we had a 200.

There were also some places where we expected the returned value to be
stringified json that needed to be `json_decode`d, but now that it's actually
returning json it's decoded for us and we're presented with an array as opposed
to a string.

Some additional debugging was also added.
Added some assertions so that the new version of PHPUnit would run the tests and
added a return type definition to keep PHP 7.2 happy.
PHPUnit deprecated this function call but thankfully some nice person sucked out
the implementation from PHPUnit and put it in a new composer library. This
change is just accounting for that change.
ryanrath and others added 29 commits February 16, 2024 14:44
We'll expect PHP Deprecated errors until we have everything fully upgraded.
Some entries for 'PHP Notice' are added to the php_errors.log file for
attempting to fread a directory. These should be ignored.
- Deleted remnants of change in `html/`
- Removed Highcharts libraries from `public/gui/lib/highcharts` as we're
  using plotly now.
- Updated versions of plotly and jQuery
- updated public/highchart_template.html to public/plotly_template.html
- updated public/index.php so that it works w/ organizations being an
  array vs. an object.
In PHP8.2 it's required that to reference a variable from within a string you
now need to do `{$variable}` as opposed to `${variable}`.
this code was previously referencing `i` which isn't a php variable.
Classes that implement functions to comply with PHP Iterators need to specify function return types.
@jpwhite4 jpwhite4 marked this pull request as draft April 29, 2025 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants