Skip to content
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

Error on removing default connection #581

Closed
marcelojaloto opened this issue Oct 21, 2022 · 21 comments
Closed

Error on removing default connection #581

marcelojaloto opened this issue Oct 21, 2022 · 21 comments
Assignees
Labels
open Some team member is working on this
Milestone

Comments

@marcelojaloto
Copy link
Contributor

marcelojaloto commented Oct 21, 2022

On the event OnAfterRouting in the class TMVCActiveRecordMiddleware the DMVC framework try to remove default connection, but when you haven't the "default" connection in the file connection.ini then occurr the erro below:


Debugger Exception Notification

Project Server.exe raised exception class Exception with message 'Unknown connection default'.

The solution for this problem it is just to deactivate the param 'RaiseExceptionIfNotAvailable' in the method RemoveDefaultConnection.

Unit: MVCFramework.Middleware.ActiveRecord
Line: 131

procedure TMVCActiveRecordMiddleware.OnAfterRouting(AContext: TWebContext; const AHandled: Boolean);
begin
  ActiveRecordConnectionsRegistry.RemoveDefaultConnection(False);
end;

@danieleteti
Can I send a pull request for you with the fix?

@danieleteti
Copy link
Owner

Thank you, I did the change. However, how you reached that situation? The default connection is added in the OnBeforeRouting event. Did you remove it manually?

@marcelojaloto
Copy link
Contributor Author

My connection stay in file Connection.ini with other name, then when I adds support to Active Record, the connection name it is set with the other name.

FMVC.AddMiddleware(TMVCActiveRecordMiddleware.Create('other_connection_name', ''));

@marcelojaloto
Copy link
Contributor Author

Another point when you remove a connection, it is not necessary to raise an exception case the connection name does not exist.

marcelojaloto added a commit to marcelojaloto/delphimvcframework that referenced this issue Oct 21, 2022
marcelojaloto added a commit to marcelojaloto/delphimvcframework that referenced this issue Oct 21, 2022
danieleteti#581 - Fix error on removing default connection;
danieleteti pushed a commit that referenced this issue Oct 22, 2022
* Issue #546 was missing from the merge.

* #581 - Fix error on removing default connection;
@danieleteti
Copy link
Owner

I think there is a misunderstanding here.

When you define your firedac connection name (let's say "myconnection"), this connection become your "default" connection in the ActiveRecordConnectionRegistry. So, it is compulsory to remove the connection with the default name, because you actually added it.

@marcelojaloto
Copy link
Contributor Author

@danieleteti
Yes, exactly. However in the configuration file (Configuration.ini) there is no connection with the name "default" and so the error occurs. I just muted the message that occurs in case this issue happens.

ok?

@danieleteti
Copy link
Owner

No, the ConnectionDefName in the FireDAC connections file is not related in any way with the "default" name in the ActiveRecordConnectionRegistry, it is just a FireDAC connection definition. You can have a name i the file and another name in the connection registry.
RemoveDefaultConnection must raise a connection if it is not available.

Check the code, and the samples, it should be clear.

Let me know

@marcelojaloto
Copy link
Contributor Author

@danieleteti
There is an error with the default connection name "default", so if the connection named "default" is not found it will trigger an unnecessary error message. I just disabled the error message which is already predicted in the ActiveRecordConnectionsRegistry.RemoveDefaultConnection method.

I have tested it several times and error always occurs. Disabling unnecessary message error on just calling this method works fine because I have another connection with another name and everything works fine.

@marcelojaloto
Copy link
Contributor Author

If there is a connection with the name "default", then the system will remove this connection normally, but if it does not exist there is no reason to display an error message, because another connection will be activated as in my case.

And in the last case a more specific error in other methods will occur because there is no available connection to the database.

@marcelojaloto
Copy link
Contributor Author

marcelojaloto commented Oct 24, 2022

ok?

@danieleteti
Copy link
Owner

Sorry, I still don't get the point. Can you send me a self contained sample which show the problem?

@Microcom-Bjarne
Copy link
Contributor

@marcelojaloto
Do you have a BASE controller? (one from which all other decend from)?
If you do - you could try this approach via tha OnBeforeAction event.

Skip adding the ActiveRecord Middleware

FMVC.AddMiddleware(TMVCActiveRecordMiddleware.Create('important_connection', ExtractFilePath(GetModuleName(HInstance)) + 'MyConnections.ini'));
and simply maintain the ActiveRecordConnectionsRegistry manually.

  if not FDManager.ConnectionDefFileLoaded then
  begin
    FDManager.ConnectionDefFileAutoLoad := false;
    FDManager.ConnectionDefFileName := ExtractFilePath(GetModuleName(HInstance)) + 'MyConnections.ini';
    FDManager.LoadConnectionDefFile;
  end;

  if FDManager.IsConnectionDef('important_connection') then // needed for some reason!
  begin
    ActiveRecordConnectionsRegistry.AddDefaultConnection('important_connection');
  end;

And then remember to remove default connection in OnAfterAction
ActiveRecordConnectionsRegistry.RemoveDefaultConnection(false);

This works for me :)

If you don't like this solution or it can't fit in your setup - then please ignore this.

@danieleteti
Copy link
Owner

@marcelojaloto and @Microcom-Bjarne
Guys, it seems that there is a problem (I don't know if real or not) regarding the ActiveRecordMiddleware, but I still don't understand what it is. The proposed solutions should not be necessary at all.
Please, send me a reproducible example which show the problem.

@Microcom-Bjarne
Copy link
Contributor

@danieleteti
I ran into the issue when starting to use my own connections definitions file. And having multiple connections in it.
But skipping the middleware has eliminated my problems - as for now.

@marcelojaloto
Copy link
Contributor Author

@danieleteti

As soon as I have some free time, I'll try to send you an example.

@Microcom-Bjarne

I'd like it to reflect the following: If you need to remove a DEFAULT connection that doesn't exist, then the RemoveDefaultConnection method assumes that it has already been removed. Right?

DMVC already provides this in the RaiseExceptionIfNotAvailable parameter of the RemoveDefaultConnection method.

Otherwise, justify me why an exception must be generated that impairs the proper functioning of the system?

Does your solution work fine, but will everyone who uses DMVC need to implement it or another solution, if you just passing False in the parameter of the RemoveDefaultConnection method in the OnAfterRouting event call?

I really appreciate your contribution, if my PR doesn't get approved I'll try to take it into consideration.

I made this contribution hoping to help the entire community.

@Microcom-Bjarne
Copy link
Contributor

@danieleteti

Here is modified sample application using 2 databases.
If this is not the correct way - when using ActiveRecord - then please advice on how.

Please adjust connections.ini and fddrivers.ini to match your setup.
I've used the supplied activerecorddb.fdb database - just made a copy so I have 2.

With this setup type - I cannot see a way of using the Middleware setup. Since it will only add the "default" connection.
Then I will have to do something similar for second connection.

This will get more visible when using BOTH connections in one endpoint.
middleware_activerecord.zip

@danieleteti
Copy link
Owner

@Microcom-Bjarne
you can just create another custom middleware, register it after the TMVCActiveRecordMiddleware and then register another connection (with a name <> 'default') in the ActiveRecordConnectionsRegistry and it should work.
However, I think I'll change the TMVCActiveRecordMiddleware to allow a more pleasant experience in cases like your.

@Microcom-Bjarne
Copy link
Contributor

@danieleteti yes - but my current setup allows me to edit the connections.ini file to add or remove connections, without having to stop my api. It will simply reload the connections.ini.
I can send you my source for this - if you are interested.

@danieleteti
Copy link
Owner

@Microcom-Bjarne
About the "multiple connectiondef" problem, check this example using this commit "cb6f0f0"

For your particular case, where the connections can be updated without stopping the service, I'd use a timestamp check and reload the "Connections.ini" only if its "last modified" timestamp is changed. Also, deep test the solution (with automated test) in concurrent environment.

middleware_activerecord_issue581.zip

@Microcom-Bjarne
Copy link
Contributor

@danieleteti I'll have a look at it - probably over the weekend.
My current reload scheme is based upon - if the connection does not exist then reload the connections.ini and see if it exists after.
But that is just another way of doing the same thing.

@danieleteti
Copy link
Owner

@Microcom-Bjarne yes, but checking the timestamp allows to reload connections also if something else, apart the connection name, changes (eg. password, hostname, some other configurations). Also what about connection "removing"? I don't know you scenario, maybe this is needed.
Just my 2c

@danieleteti danieleteti added open Some team member is working on this and removed task labels Nov 24, 2022
@danieleteti
Copy link
Owner

any news on this?

danieleteti pushed a commit that referenced this issue Oct 18, 2024
* Issue #546 was missing from the merge.

* #581 - Fix error on removing default connection;

* #583 - Fixes bugs 'Invalid class typecast' when using Active Record and Enumerated Type;

* issue #789;

* issue #789;
danieleteti pushed a commit that referenced this issue Nov 29, 2024
* Issue #546 was missing from the merge.

* #581 - Fix error on removing default connection;

* #583 - Fixes bugs 'Invalid class typecast' when using Active Record and Enumerated Type;
danieleteti pushed a commit that referenced this issue Jan 6, 2025
* Issue #546 was missing from the merge.

* #581 - Fix error on removing default connection;

* #583 - Fixes bugs 'Invalid class typecast' when using Active Record and Enumerated Type;

* Fixes rendering in Swagger documentation for TArray type fields.

* It also allows overriding the MapTValueToParam method of the TMVCActiveRecord class.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open Some team member is working on this
Projects
None yet
Development

No branches or pull requests

3 participants