IntelliTect / IntelliTect.AspNetCore.SignalR.SqlServer

A Microsoft SQL Server backplane for ASP.NET Core SignalR.
Apache License 2.0
39 stars 5 forks source link

Add option to disable schema install #13

Closed awaescher closed 1 year ago

awaescher commented 1 year ago

I was just about to try this backplane, thanks a lot for building it.

However, I cannot make use of the backplane's EnsureSqlServerConnection() as I don't have the permissions to alter the database in production systems from my app - this is done with migrations upfront.

So I came across the install.sql script which I could add to my migrations. Perfect. I also added the permissions like this:

GRANT SELECT ON [SignalR].[Schema] TO «User»;
GRANT SELECT, DELETE, UPDATE, INSERT ON [SignalR].[Messages_0] TO «User»;
GRANT SELECT, DELETE, UPDATE, INSERT ON [SignalR].[Messages_0_Id] TO «User»;

But the next issue is this:

image

So it says the current version is 1 and it's about to install version 1 and I guess the IFs are not meant the way they are implemented:

-- Install tables, etc.
IF @CURRENT_SCHEMA_VERSION IS NULL OR @CURRENT_SCHEMA_VERSION <= @TARGET_SCHEMA_VERSION
    BEGIN
        IF @CURRENT_SCHEMA_VERSION IS NULL OR @CURRENT_SCHEMA_VERSION = @TARGET_SCHEMA_VERSION
            BEGIN
                -- Install version 1
                PRINT 'Installing schema version 1';

Right now, it's checking if 1 <= 1 in the first IF which is true for my case. The next IF checks if 1=1 which is also true and leads to schema installation. Shouldn't it be one single IF that checks if the current schema is older than the target schema only?

This leads to two questions:

ascott18 commented 1 year ago

The install script is correct. It is identical to https://github.com/aspnet/SignalR-SqlServer/blob/dev/src/Microsoft.AspNetCore.SignalR.SqlServer/install.sql except for a comment and an error message string.

There is only one schema version, and there are no current plans or need to have a version 2 of the schema, just as there was no v2 schema in the legacy aspnet backplane upon which this project is based.

The main part of the schema install script is creating additional tables up to @MESSAGE_TABLE_COUNT, which is needed any time the table count increases, not only when the schema version were to hypothetically rev.

A setting to disable auto install could be added, yes.

awaescher commented 1 year ago

A setting to disable auto install could be added, yes.

Would be awesome. I patched SqlInstaller.Install() to just return a completed task and scripted the tables upfront and everything works as expected.

awaescher commented 1 year ago

Awesome, thanks a thousand times for this. I'll be reporting back to the team 👏