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

Entity Framework 6 #19

Closed
wants to merge 3 commits into from
Closed

Entity Framework 6 #19

wants to merge 3 commits into from

Conversation

pdonald
Copy link
Contributor

@pdonald pdonald commented May 4, 2013

In EF6 everything from System.Data.Entity.dll was moved to EntityFramework.dll and namespaces were also changed.

I inserted the new namespaces surrounded with #if ENTITIES6 so that nothing changes if you are using a previous version. Since I also had to remove a reference to System.Data.Entity I created a new project NpgsqlEF6 which targets v4.5 and defines ENTITIES6 and has this reference removed.

If you are like me and want to use the new Entity Framework 6 now, you can use the NpgsqlEF6 project and for everyone else nothing changes.

@franciscojunior
Copy link
Member

Thank you very much, Peteris.

Did you get any problems about Npgsql Factory like the one reported here?:
http://stackoverflow.com/questions/14690567/entity-framework-5-0-postgresql-npgsql-default-connection-factory

If not, I think this is very good news as they could get the EF working without changes being needed in the current providers.

@pdonald
Copy link
Contributor Author

pdonald commented May 8, 2013

@franciscojunior I had no problems. I'm only querying the database though, not updating or inserting anything.

I did it for the Framework Benchmarks project. You can see how I use Npgsql here (Web.config, Models\EntityFramework.cs, Controllers\EntityFrameworkController.cs and also Controllers\AdoController.cs). Works perfectly on Windows and Mono.

@franciscojunior
Copy link
Member

Excellent! This is very nice to know, @pdonald !

Thank you very much for your feedback and patch. I'll make some tests and then will merge it.

@pdonald
Copy link
Contributor Author

pdonald commented May 8, 2013

Another possibility is to create a new configuration for EF6 in the existing project files like the MySQL Connector/Net does it instead of creating a new project file like I did.

@pdonald
Copy link
Contributor Author

pdonald commented May 8, 2013

@franciscojunior Oh, I forgot to mention I made another small change but I didn't commit it. To get it to work on mono, I commented out these lines in NpgsqlConnectionStringBuilder.cs:

if ((_integrated_security) && (String.IsNullOrEmpty(_username)))
{
    // System.Security.Principal.WindowsIdentity identity = System.Security.Principal.WindowsIdentity.GetCurrent();
    // _username = identity.Name.Split('\\')[1];
}

Mono doesn't implement System.Security.Principal.WindowsIdentity and this piece of code was called twice.

@maxbundchen
Copy link

@pdonald and @franciscojunior I'm using Npgsql with EF6 with similar changes as the ones posted here. However I found a problem with NpgsqlConnection: the EF6 depends on open/close events in some cases (like update or insert) and Npgsql don't trigger this ones. I changed the Open() and Close() methods from NpgsqlConnection to accomplish this:

public override void Open()
    {
        CheckConnectionClosed();

        NpgsqlEventLog.LogMethodEnter(LogLevel.Debug, CLASSNAME, "Open");

        // Check if there is any missing argument.
        if (!settings.ContainsKey(Keywords.Host))
        {
            throw new ArgumentException(resman.GetString("Exception_MissingConnStrArg"),
                                        NpgsqlConnectionStringBuilder.GetKeyName(Keywords.Host));
        }
        if (!settings.ContainsKey(Keywords.UserName) && !settings.ContainsKey(Keywords.IntegratedSecurity))
        {
            throw new ArgumentException(resman.GetString("Exception_MissingConnStrArg"),
                                        NpgsqlConnectionStringBuilder.GetKeyName(Keywords.UserName));
        }

        // Get a Connector.  The connector returned is guaranteed to be connected and ready to go.
        connector = NpgsqlConnectorPool.ConnectorPoolMgr.RequestConnector(this);

        connector.Notice += NoticeDelegate;
        connector.Notification += NotificationDelegate;

        if (SyncNotification)
        {
            connector.AddNotificationThread();
        }

        if (Enlist)
        {
            Promotable.Enlist(Transaction.Current);
        }

        this.OnStateChange(new StateChangeEventArgs(ConnectionState.Closed, ConnectionState.Open)); // EF6
    }

public override void Close()
    {
        NpgsqlEventLog.LogMethodEnter(LogLevel.Debug, CLASSNAME, "Close");
        if (connector != null)
        {
            ConnectionState oldState = connector.State; // EF6
            Promotable.Prepare();
            // clear the way for another promotable transaction
            promotable = null;

            connector.Notification -= NotificationDelegate;
            connector.Notice -= NoticeDelegate;

            if (SyncNotification)
            {
                connector.RemoveNotificationThread();
            }

            NpgsqlConnectorPool.ConnectorPoolMgr.ReleaseConnector(this, connector);


            connector = null;

            this.OnStateChange(new StateChangeEventArgs(oldState, ConnectionState.Closed)); // EF6
        }
    }

@franciscojunior
Copy link
Member

Hi, @pdonald. Your idea of using a different configuration is very good. This way we don't need to maintain 2 different projects. I'll take a look at that.

About the Mono specific changes, I think we should wrap that part of the code with the #ifdef WINDOWS we already use for integrated windows security. I think this is worth a bug report. I'll handle that. Thanks for the heads up!

@maxbundchen, thanks for the heads up! I think Npgsql should have implemented those events a long time ago... I'll take care of that too.

@pdonald
Copy link
Contributor Author

pdonald commented May 9, 2013

@franciscojunior I already reported it here: https://bugzilla.xamarin.com/show_bug.cgi?id=12174

But I got it to work without commenting out those lines too: I compiled Npgsql with mono and it now works on both Windows and Mono. But if it's compiled with VS2012 and then run on mono, then I that unhandled exception.

@franciscojunior
Copy link
Member

@pdonald , this different behavior is very strange. Please, let us know if you get any update about that.

@pdonald
Copy link
Contributor Author

pdonald commented May 18, 2013

The bug was rejected. But the guy proposed a solution to fix it.
https://bugzilla.xamarin.com/show_bug.cgi?id=12174

@roji
Copy link
Member

roji commented May 18, 2013

@pdonald and @franciscojunior, maybe I can help explain Rodrigo's answer you the bug report...

The problem with your testcase is that the check for mono (the if) is in the same method as the reference to the missing type/method. Technically, I think this is happening because mono has a JIT - before running any code in your method, the JIT attempts to compile it, runs into the reference to a missing type and bombs - so your check for mono does nothing. To fix this, try to split your testcase into two methods: One that contains the "if mono" and calls the other, and the other accessing the WindowsIdentity and the missing type.

This has nothing to do with the fact the name property may be missing from System.Security.Claims.ClaimsIdentity, which is your bug report - Rodrigo just responded to the problematic testcase.

In short, just split all the code that looks at the missing type into a separate method that won't ever be JITted, since it will never be called in mono.

Let me know if you need any more help/clarifications...

@franciscojunior
Copy link
Member

Hi, @pdonald . I'm working to merge your patch. I notice you added EntityFramework.dll in your patch. Don't you think it would be better if developer installed EntityFramework.dll from Nuget? This way she would get it from the official channel and we wouldn't need to redistribute it. I tested getting EntityFramework from Nuget and it worked ok.

@pdonald
Copy link
Contributor Author

pdonald commented Jun 15, 2013

Yes, you're right.

@franciscojunior
Copy link
Member

Great! I'll finish merging your work and will push it soon! Thank you very much for your excellent work, Peteris! It will help a lot of developers using npgsql and newest ef version.

@franciscojunior
Copy link
Member

Hi, @pdonald ! I just merged and pushed your pull request to master branch! Thank you very much for your help! Please, give it a try and let me know if everything is working ok for you.

@franciscojunior
Copy link
Member

I'm closing this pr as I already merged it to master branch.

@jjchiw
Copy link

jjchiw commented Dec 20, 2013

Does the compilation directive of System.Security.Principal.WindowsIdentity identity was applied to master? I added the directive here jjchiw/Npgsql@af0548a and created other Configurations

Mono-Debug-net20
......
Mono-Release-net45

gencer pushed a commit to gencer/Npgsql2 that referenced this pull request Apr 11, 2014
https://github.com/pdonald and https://github.com/maxbundchen for the
patch.

Npgsql didn't have connection state change events. According to
maxbundchen: "the EF6 depends on open/close events in some cases (like
update or insert) and Npgsql don't trigger this ones."
npgsql#19 (comment)

Added tests about those events.
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.

None yet

5 participants