mvc: Phalcon framework dependency #6389

fichtner · 2023-03-03T12:52:01Z

Important notices

Before you add a new report, we ask you kindly to acknowledge the following:

I have read the contributing guide lines at https://github.com/opnsense/core/blob/master/CONTRIBUTING.md
I am convinced that my issue is new after having checked both open and closed issues at https://github.com/opnsense/core/issues?q=is%3Aissue

Is your feature request related to a problem? Please describe.

Given the state of Phalcon, it’s probably a good idea to start planning for losing more dependencies on the framework.

Describe the solution you like

A good start would probably be to start with our model implementation, which seems to depend on the following classes for which we should build native php alternatives in that case:

~~Phalcon\Filter\Validation\Validator\Email~~
~~Phalcon\Filter\Validation\Validator\ExclusionIn~~
~~Phalcon\Filter\Validation\Validator\InclusionIn~~
~~Phalcon\Filter\Validation\Validator\Numericality~~
~~Phalcon\Filter\Validation\Validator\PresenceOf~~
~~Phalcon\Filter\Validation\Validator\Regex~~
~~Phalcon\Filter\Validation\Validator\Url~~
~~OPNsense\Phalcon\Logger\Logger~~
~~Phalcon\Filter\Validation\Exception~~
Phalcon\Di\FactoryDefault
~~Phalcon\Logger\Adapter\Syslog~~
~~Phalcon\Messages\Message~~
~~Phalcon\Messages\Messages~~
~~Phalcon\Config\Config~~

Describe alternatives you considered

N/A

Additional context

N/A

…r own. for #6389

Add simple Message class and remove "Messages" dependancy in Validation.php, which should be backwards compatible with all existing validations. By moving \Phalcon\Filter\Validation() into validate() we're making the validation paths more explicit, if an objects implements ValidatorInterface, it uses phalcon, otherwise it's a simple BaseValidator passing messages back to $this->appendMessage(). The original phalcon Message class has additional fields we don't use, we only use fieldname for tracking purposes and the message itself.

fix regression in c2ea9aa and amend unit tests.

Properties should be copied 1-to-1 before we apply the required defaults if necessary. In the previous situation this caused required properties to be set to an empty value after the default value had already been written to it. In the failure case we attempt to be a bit more explicit and refer to the crash reporter. While here, the master branch has dropped the Phalcon Messages class, so switch to count() since this seems to inherit array type and is therefore backwards compatible: $msgs = new \Phalcon\Messages\Messages(); $count = count($msgs); // $count == 0 See #6389

Properties should be copied 1-to-1 before we apply the required defaults if necessary. In the previous situation this caused required properties to be set to an empty value after the default value had already been written to it. In the failure case we attempt to be a bit more explicit and refer to the crash reporter. While here, the master branch has dropped the Phalcon Messages class, so switch to count() since this seems to inherit array type and is therefore backwards compatible: $msgs = new \Phalcon\Messages\Messages(); $count = count($msgs); // $count == 0 See #6389 (cherry picked from commit fdc8a8f) (cherry picked from commit 3cb2f3d)

…for backwards compatible OPNsense\Base\Messages\Message class. for #6389

Fix regression in c2ea9aa, performValidation() should return an object which is able to add new messages (using appendMessage()) as the previous Messages class did.

…r own. for #6389

Add simple Message class and remove "Messages" dependancy in Validation.php, which should be backwards compatible with all existing validations. By moving \Phalcon\Filter\Validation() into validate() we're making the validation paths more explicit, if an objects implements ValidatorInterface, it uses phalcon, otherwise it's a simple BaseValidator passing messages back to $this->appendMessage(). The original phalcon Message class has additional fields we don't use, we only use fieldname for tracking purposes and the message itself.

fix regression in c2ea9aa and amend unit tests.

Properties should be copied 1-to-1 before we apply the required defaults if necessary. In the previous situation this caused required properties to be set to an empty value after the default value had already been written to it. In the failure case we attempt to be a bit more explicit and refer to the crash reporter. While here, the master branch has dropped the Phalcon Messages class, so switch to count() since this seems to inherit array type and is therefore backwards compatible: $msgs = new \Phalcon\Messages\Messages(); $count = count($msgs); // $count == 0 See #6389

…for backwards compatible OPNsense\Base\Messages\Message class. for #6389

Fix regression in c2ea9aa, performValidation() should return an object which is able to add new messages (using appendMessage()) as the previous Messages class did.

…e to offer multiple model paths in the phalcon configuration, in practice this was never used (all models live in /usr/local/opnsense/mvc/app/models). for #6389

…nse\Core\AppConfig class. for #6389

…e\ValidationException (both simple empty Exception classes). for #6389 (cherry picked from commit 0dc6089)

#6389

The original intend was to be able to reuse the Csrf class, but as this requires direct access to the session object, it's likely not a good idea for the goals of #6389 Since the legacy pages need a lock on session anyway, keeping it doesn't make a difference.

#6389

…sses with a routing class of our own. (#6389) This removes most phalcon code currently being used on our end, except the Volt templates (which are re-wrapped) and a translation class we can easisly replace later in a separate commit. Consumers of our controller classes shouldn't notice a difference as the used objects and methods are named the same. The most notable changes are the following ones: * Exceptions about not being able to find a requested path now break down into different exceptions inheriting from DispatchException, which makes it easier from the entrypoint (api.php, index.php) to catch and handle accordingly. * When not in development mode, raw exceptions are not being returned anymore, which increases security * The Dispatcher class is reponsible for object construction and mapping validation (valid uri, but no object found) * The Router class replaces previous Application class, it disects offered uri's into namespaces, classnames and methods to call.

This removes most phalcon code currently being used on our end, except the Volt templates (which are re-wrapped) and a translation class we can easily replace later in a separate commit. Consumers of our controller classes shouldn't notice a difference as the used objects and methods are named the same. The most notable changes are the following ones: * Exceptions about not being able to find a requested path now break down into different exceptions inheriting from DispatchException, which makes it easier from the entrypoint (api.php, index.php) to catch and handle accordingly. * When not in development mode, raw exceptions are not being returned anymore, which increases security * The Dispatcher class is reponsible for object construction and mapping validation (valid uri, but no object found) * The Router class replaces previous Application class, it disects offered uri's into namespaces, classnames and methods to call. In the long run there should be a seperate controller for controllers using volt templates or api calls, but as the existing ones don't distinct between this and the output handling is different now, we can park this for a later moment in time (the performance penalty should be rather low). Some unused functionality has been removed, for example support for the X-HTTP-Method-Override header in Request->getMethod() (see https://github.com/phalcon/cphalcon/blob/44243c07658d060cd8a21761743b0f4fc01641aa/phalcon/Http/Request.zep#L599-L609).

…uments returned $_POST previsouly. (same likely the case for get())

…ntAddress

fichtner added the cleanup Low impact changes label Mar 3, 2023

fichtner added this to the 23.7 milestone Mar 3, 2023

fichtner assigned fichtner, AdSchellevis and swhite2 Mar 3, 2023

AdSchellevis mentioned this issue Mar 11, 2023

Demo of volt framework using Dnsmasq #6410

Closed

fichtner modified the milestones: 23.7, 24.1 Jun 29, 2023

AdSchellevis added a commit that referenced this issue Jan 9, 2024

MVC: ditch phalcon's syslog implementation for a simple wrapper of ou…

4d8c9d1

…r own. for #6389

fichtner modified the milestones: 24.1, 24.7 Jan 11, 2024

AdSchellevis added a commit that referenced this issue Feb 13, 2024

mvc: phalcon - drop unused "use Phalcon\Messages\Messages" (#6389)

fa6d093

AdSchellevis added a commit that referenced this issue Feb 13, 2024

mvc: Phalcon framework dependency (#6389)

75bb361

fix regression in c2ea9aa and amend unit tests.

AdSchellevis added a commit that referenced this issue Feb 15, 2024

mvc / CallbackValidator - remove Phalcon dependency for #6389

913fa71

AdSchellevis added a commit that referenced this issue Feb 15, 2024

mvc : Phalcon framework dependency, replace Phalcon\Messages\Message …

32d19cc

…for backwards compatible OPNsense\Base\Messages\Message class. for #6389

AdSchellevis added a commit that referenced this issue Feb 18, 2024

mvc: Phalcon framework dependency (#6389)

5fc54cf

Fix regression in c2ea9aa, performValidation() should return an object which is able to add new messages (using appendMessage()) as the previous Messages class did.

fichtner pushed a commit that referenced this issue Mar 12, 2024

MVC: ditch phalcon's syslog implementation for a simple wrapper of ou…

0d3211f

…r own. for #6389

fichtner pushed a commit that referenced this issue Mar 12, 2024

mvc: phalcon - drop unused "use Phalcon\Messages\Messages" (#6389)

c914f94

fichtner pushed a commit that referenced this issue Mar 12, 2024

mvc: Phalcon framework dependency (#6389)

5476170

fix regression in c2ea9aa and amend unit tests.

fichtner pushed a commit that referenced this issue Mar 12, 2024

mvc / CallbackValidator - remove Phalcon dependency for #6389

4441a2d

fichtner pushed a commit that referenced this issue Mar 12, 2024

mvc : Phalcon framework dependency, replace Phalcon\Messages\Message …

74f8a82

…for backwards compatible OPNsense\Base\Messages\Message class. for #6389

fichtner pushed a commit that referenced this issue Mar 12, 2024

mvc: Phalcon framework dependency (#6389)

7103ebc

Fix regression in c2ea9aa, performValidation() should return an object which is able to add new messages (using appendMessage()) as the previous Messages class did.

AdSchellevis added a commit that referenced this issue Apr 30, 2024

mvc: replace Phalcon\Config\Config usage for new and simplified OPNse…

cbc01cb

…nse\Core\AppConfig class. for #6389

fichtner pushed a commit that referenced this issue May 6, 2024

mvc - replace \Phalcon\Filter\Validation\Exception with \OPNsense\Bas…

e7f0488

…e\ValidationException (both simple empty Exception classes). for #6389 (cherry picked from commit 0dc6089)

AdSchellevis added a commit that referenced this issue May 6, 2024

mvc - work in progress to replace Phalcon Application()

c9d46d3

#6389

AdSchellevis added a commit that referenced this issue May 6, 2024

mvc - work in progress to replace Phalcon Application()

050068d

#6389

AdSchellevis added a commit that referenced this issue May 6, 2024

mvc - work in progress to replace Phalcon Application()

84e2438

#6389

AdSchellevis added a commit that referenced this issue May 6, 2024

mvc - work in progress to replace Phalcon Application()

93ecc7d

#6389

AdSchellevis added a commit that referenced this issue May 7, 2024

mvc - work in progress to replace Phalcon Application()

eecfa04

#6389

AdSchellevis added a commit that referenced this issue May 7, 2024

mvc - work in progress to replace Phalcon Application()

07bfc94

#6389

AdSchellevis added a commit that referenced this issue May 7, 2024

mvc - work in progress to replace Phalcon Application()

0f8cdd5

#6389

AdSchellevis added a commit that referenced this issue May 7, 2024

mvc - work in progress to replace Phalcon Application()

6084f72

#6389

AdSchellevis added a commit that referenced this issue May 8, 2024

mvc - work in progress to replace Phalcon Application()

5bb7456

#6389

AdSchellevis added a commit that referenced this issue May 8, 2024

mvc - work in progress to replace Phalcon Application()

36496e0

#6389

AdSchellevis added a commit that referenced this issue May 11, 2024

mvc - work in progress to replace Phalcon Application()

38e5e44

#6389

AdSchellevis added a commit that referenced this issue May 11, 2024

mvc - work in progress to replace Phalcon Application()

9f3ece3

#6389

AdSchellevis added a commit that referenced this issue May 11, 2024

mvc - work in progress to replace Phalcon Application()

d6b3298

#6389

AdSchellevis added a commit that referenced this issue May 11, 2024

mvc - work in progress to replace Phalcon Application()

542d49b

#6389

AdSchellevis added a commit that referenced this issue May 11, 2024

mvc - work in progress to replace Phalcon Application()

6251010

#6389

AdSchellevis added a commit that referenced this issue May 12, 2024

mvc - work in progress to replace Phalcon Application()

7bc1233

#6389

AdSchellevis mentioned this issue May 12, 2024

mvc - larger overhaul replacing Phalcon Application #6389 #7455

Merged

fichtner linked a pull request May 13, 2024 that will close this issue

mvc - larger overhaul replacing Phalcon Application #6389 #7455

Merged

fichtner closed this as completed in #7455 May 13, 2024

AdSchellevis added a commit that referenced this issue May 13, 2024

mvc - Response add setRawHeader for backwards compatibility (for #6389)

cf7300e

AdSchellevis added a commit that referenced this issue May 21, 2024

mvc: backwards compatibility for Phalcon (#6389), getPost without arg…

d5566b8

…uments returned $_POST previsouly. (same likely the case for get())

AdSchellevis added a commit that referenced this issue May 21, 2024

mvc: backwards compatibility for Phalcon (#6389), add missing getClie…

72120a6

…ntAddress

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mvc: Phalcon framework dependency #6389

mvc: Phalcon framework dependency #6389

fichtner commented Mar 3, 2023 •

edited by AdSchellevis

mvc: Phalcon framework dependency #6389

mvc: Phalcon framework dependency #6389

Comments

fichtner commented Mar 3, 2023 • edited by AdSchellevis

fichtner commented Mar 3, 2023 •

edited by AdSchellevis