-
Notifications
You must be signed in to change notification settings - Fork 825
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
Initial DDEX support (Entity Model) #67
Conversation
That's great, @wwindcloud ! I'll test it and will let you know how it goes. Regarding the byte[], it is a result of recent @glenebob Glen Parker's excellent work on performance improvements. We changed our internal data handling to be all byte[] to easy writing to networkstreams and this specific case went unnoticed. But as you noticed, a fix for this is really easy. The only change you would need to do is to use the UTF8 encoding, instead of ASCII. You could use the following method which is used when we need to get a string from byte[]:
Just another questions, does this vs package work only on vs.net 2012 or does it work on 2010 or even on the 2008? |
No wonder, should be easy to fix similar cases around. As I am using only vs 2012 right now, haven't bother checking with vs2010 or vs2008, but since I am using DDEX platform version 2.0, I assume they work as 2.0 is already supported from vs2008. In the meantime I am trying to fix the copy/paste/drag/drop issue, it seems that the connectionstringbuilder is not conformant enough, somehow connection string were different from place to place as such VS cannot get back the original connection when an IDataObject get passed around containers, so that's why there's always an error pop out when pasting/dragging data tables/views from server explorer. Do you mind if I make changes to NpgsqlConnectionStringBuilder to add PropertyDescriptorAttributes, TypeConverterAttribute, etc to align with how Microsoft did theirs with SqlConnectionStringBuilder such that people can get/set Npgsql connection property with .net type conversion support on-the-fly especially on forms. Right now NpgsqlConnectionStringBuilder simply convert things internally and I think there might be some inconsistencies when .net/3rd component want to get/set some of the connection properties from Npgsql. |
If you find any other place where the byte[] approach is giving you problems, please let us know. :) It's great to know you are using DDEX version which works since vs.net 2008. This will help us get more developers on board :) Please, go ahead and make the changes. Your fixes will improve Npgsql compatibility and conformance. Those changes are always welcome! :) |
…d customization DDEX: Make copy/paste drag/drop work Npgsql: Rework NpgsqlConnectionStringBuilder to be more consistent Npgsql: Fix bug in NpgsqlMetaData.xml, wrong CompositeIdentifierSeparatorPattern Npgsql: Fix GetSchema for tables to return TABLE only
Ok, finally, most basic feature should be implemented by now. Please let me know if you find something wrong or bugs, I would like to see these integrated into the official version of Npgsql. Entity/Dataset/Query in VS should be working as expected. The most drastic change is NpgsqlConnectionStringBuilder. Actually, the one implemented in Npgsql right now did not taken care the fact DbConnectionStringBuilder implement itself an ICustomTypeDescriptor, all public property type were getting exposed. 3rd party or VS can rely on ICustomTypeDescriptor and Reflection to get/set value from the builder. However, because DbConnectionStringBuilder will utimately convert every value into a string inside its' base container, getting the value back from base[keyword] will return you back a string instead of the original type. Therefore, those using reflection on SslMode, ProtocolVersion and System.Version will eventually lead to an exception because it doesn't know how to convert string back forth to from their original type. Base on the fact builders with the same connection string should have the same state, I have removed all custom property like _username, _database, _* etc and simply rely on the underneath DbConnectionStringBuilder collections. |
Excellent, @wwindcloud ! I noticed that on this line: Another thing I noticed is that NpgsqlConnectionStringBuilder.resx is now referencing system.forms version 4.0. This can give us problem compiling it to .net 3.5 and 2.0 for backward compatibility. Is it possible to keep referencing previous assembly versions? If not, I think we would need to check how to get different resx files compiled for different target framework versions. Maybe visual studio project files wizard @roji may help us! :) I'm still checking your changes to NpgsqlConnectionStringBuilder and they seem to be very nice! This PR conflicts with current master. Would you mind to rebase your code on master and resolve any conflicts you may find? This would help us a lot when merging your pull request. Thanks in advance. |
… change GetSchema() Npgsql: Revert NpgsqlConnectionStringBuilder.resx to using 2.0 assembly Npgsql: Fix typo Keywords.Host to Keywords.UserName
Yup, nice catch, that was a typo suppose to be username, not host. And revert to use 2.0 assembly as request and rebase my code to merge with your latest commit, hope things would be easier for you. Thanks. |
Thanks for the changes, now the merge applies cleanly. But I noticed that NpgsqlConnectionStringBuilder doesn't compile on .net2.0 anymore. In fact, thanks to this line:
It only compiles on .net4.5 because of the GetValue(this) call. Also, while testing on Xamarin Studio, I'm getting compilation errors in the lines:
Because there is no method GetCustomAttributes which takes a single Type parameter. I had to add a second parameter with value false to make it compile past this point. I still need to install vs.net 2012 to do more tests, and I don't know yet if it will allow me to create the extension. I remember express editions have some limitations about that. But we would need to keep compatibility with .net 2.0. At least compilability with .net 3.5 (as we already don't have compilability with .net 2.0 although Npgsql can be run on .net 2.0) I think your modification will benefit both DDEX development as non-DDEX development as well. And for non-DDEX we would need compatibility with .net 2.0. But if the changes only concern about DDEX, I think we can work on some conditional compilation or something else. We would need to think about it. Thank you very much in advance. P.S.: I'll be out for some days and will be back on 24th. So I may not be able to comment until then. |
Npgsql - Make it so that custom property (Protocol, SslMode, Compatible, etc) to throw an error if invalid string is passed.
Yup, I haven't thought about .NET 2.0, made some changes, Npgsql should now able to be compiled under .NET 2.0. Also, I think DDEX can be done the same, only the resx need to be modified I think, can someone take a look as I don't know how to save a resx in vs2012 to an older version. Manually text editing perhaps??? |
Hi @wwindcloud ! When I merge your code, Npgsql testsuite is throwing errors. It says:
I think there may be something wrong with the keyword_mappings initialization. I'm still checking what is going wrong, but if you have any idea about what the problem can be, it would be very helpful. Thanks in advance. |
…TargetInvokeException when parsing invalid value Fix MinPoolSize/MaxPoolSize check Make NpgsqlConnectionBuilder keyword case insensitive Fix GetSchema SQL on Contraints and ConstraintColumns when using restriction
Ah! I did not run NpgsqlTest to test. Run it now and fix some of the bugs. I didn't realize the old version was case insensitive on Keywords, so just make all keyword uppercase then. I noticed there are at least 50 test case failed on my postgresql 9.2, I guess they need some work but at least NpgsqlConnectionStringBuilder is working and pass those tests now. Thanks. |
Excellent! Thanks for having a look at this. It is strange that you are getting errors in the testsuite... You weren't When I get home I'll check it out and I'll check those test errors too. Thank you again for your help.
|
Hi guys, I have some observations on NpgsqlConnectionStringBuilder changes...
I think SslMode and Ssl should be connected. I assume Ssl exists for compatibly? Suggestion: Setting Ssl = true should set SslMode = Prefer, setting Ssl = false should set SslMode = Disable, setting SslMode = Prefer/Require should set Ssl = true and setting SslMode = Disable should set Ssl = false. Then we can simply remove the internal use of Ssl, and use SslMode only. Protocal should default to Unknown, as it did in the past. PR #83 allows automatic version selection and fallback to work again. -Glen |
Hello Glen, The reason why I set Protocol and SslMode to internal because they were Enum type, and the internal IDictionary storage in DbConnectionString use string for everything, These can be solved by using a converter. But for some reason, IVsDataConnectionProperties doesn't use a converter when getting a property from NpgsqlConnectionStringBuilder and setting it to a DataColumn of a DataTable, the odd thing is the type of DataColumn is set according to the property types of DbConnectionStringBuilder, thus Enum for Protocol and SslMode. So by setting Protocol and SslMode to internal, it will make IVsDataConnectionProperties ignore these two properties and use protocolasstring and sslmodeasstirng instead, therefore eliminating the conversion problem. People can use ProtocolAsString and SslModeAsString as replacement, type checking is still enforced internally as you can see in the code I explictly convert the string to Enum first before setting the internal property. However, if we really need Protocol and SslMode as an Enum type being public, I can customize IVsDataConnectionProperties to bypass the conversion problem. 3,4 and 5 can be solved by changing the DefaultValue attribute. I am not sure about 6. If you apply DefaultValue to every property, you should be able to get everything unless you were passing a wrong keyword string, but let me check again on that. Thanks for your feedback. |
Hi. I'm interested in Npgsql's DDEX support. I'm curious to know. Does this work relate to Npgsql Design time support preview? It is introduced at http://fxjr.blogspot.jp/2011/05/npgsql-design-time-support-preview.html I'm privately extending its DDEX provider to fit to recent VS2012/2013. |
Hi, Kenji. This isn't directly related to this post. The code from this post was a previous contribution. This pull request is a new implementation. |
Hi. I want to test this DDEX support. @wwindcloud Could you please teach me how to test this solution. I could build NpgsqlDdexProvider on VS2013 Pro (trial). And then I could install NpgsqlDdexProvider.vsix successfully. I get a message when I'm making new data connection: Failed to find or load the registered .Net Framework Data Provider. https://drive.google.com/file/d/0BzIsP2o582nbUXFlZ3oyN1QweHM/edit?usp=sharing Could you advise me what I need to do? |
Hi, Kenji! I also tried to test the project but it says the project can't be open. I'm using vs2013 trial too. Do I need to install anything else? Thanks for any info. |
It seems it needs external sdk to open Visual Studio extensions project, please try this first: Microsoft Visual Studio 2013 SDK |
Finally I could reach connection dialog. My build steps, and installation steps:
Trying
But I get error, when I click OK, after filling connection info. 「データ接続を追加できません。値の型が列の型と合いません。列Compatibleに<2.0.12.91>を格納できませんでした。必要な型はVersionです。」 It will be translated like: "Unable to add data connection. Value type doesn't match column type. Column Compatible cannot store <2.0.12.91>. Required type is Version" Anyone knows workaround for this issue? |
Great! I could open the project now. I thought I had installed that before. :) How are you debugging the extension? After checking this post: http://stackoverflow.com/questions/9281662/how-to-debug-visual-studio-extensions I could debug it by installing Npgsql.dll and Mono.Security.dll to GAC and setting visual studio to start another experimental instance of visual studio itself where the extension would be installed. This way I didn't need to install the extension in the primary visual studio configuration. I'm getting a lot of null reference exceptions which I'm checking. But I think we are very close to get it working. :) @wwindcloud , do you have any tips for us? |
Hi.
Thanks. I could connect using connection dialog, and I could see some tables/views listed.
It is problem on connection dialog? I got the same problem, when I'm working on debugger. Workaround: tweak ConvertTo to allow null input, at NpgsqlEnumConverter<T> in NpgsqlConnectionStringBuilder.cs,
In another case, I have trouble on data viewer of table/view. It seems that VS failed to load an icon image. ArgumentException "Path format is invalid..." is thrown wrapped by TargetInvocationException. I need a lot of inspections.
Yes I think so too. :) |
System.IO.Path.GetExtension ate:
I don't understand why such a path is requested... Perhaps quotation marks cause trouble? How to add breakpoint:
If you hit there, display Debug→Window→Local. |
Hi, Kenji! I could get past the Connection String dialog, but I could only get the Schemata folder. I couldn't get the subitems like you have: pg_toast, public etc :( I think it may be related to database permissions. I set up a simple user. Which permissions your database user have? |
Hello, sorry to have left for some time as I am busy with other things, I Hi Kenji, good to hear somebody is interested in my work. Well, my DDEX is Ok, as for your errors, it's the problem of the VS builtin dialog box On Mon, Dec 2, 2013 at 7:53 PM, kenjiuno notifications@github.com wrote:
|
Hi, @wwindcloud ! Welcome back.
That's excellent! Please let us know if you have any questions about those changes. |
Hi!
Ah, sorry. It is by my fault. I used different version of Npgsql. I have no problem now. :) |
Hi @kenjiuno,
Can you be a bit more explicit ? I have exacly the same problem you described. |
I'm afraid I don't remember so much... Perhaps we have brought another Npgsql.dll into GAC, from another DDEX support like Npgsql Design time support preview available for download!. Try removing current Npgsql.dll from GAC. |
I remember when I was testing this patch that I left this field empty. Did you try that? |
At Microsoft Connect, Table data viewer hangs up on Npgsql DDEX provider report got reply.
I'll test VS2013 Update 2 RC! |
Hi.
I knew why this happens...
This ArgumentException is thrown by DataTable/DataRow. You can avoid this by implementing this inside NpgsqlConnectionStringBuilder. public override bool TryGetValue(string keyword, out object value) {
try {
value = GetValue(GetKey(keyword));
return true;
}
catch (ArgumentException) {
value = null;
return false;
}
} |
Here is my branch. It is branched from official v2.1.3. I couldn't rebase correctly. 2.1 was so much changed from 2.0. |
That's excellent, @kenjiuno !! Do you think it would be easy to create a pull request for master branch? This would be very helpful! |
Thank you very much @kenjiuno for these detailed informations. |
Hi. Thanks for helpful comment. Yes, 2 branches are for recent 2.1 npgsql versions.
|
Initial DDEX support Initial DDEX support code is inherited from #67 started by wwindcloud.
0b35ef0
to
c43b6af
Compare
e1d8c57
to
1f140e5
Compare
1236d1f
to
e8ae2e0
Compare
2fc8944
to
3eb6fa4
Compare
Closing as I guess this is no longer relevant. Please reopen if otherwise. |
Hello there, as mentioned in pgfoundry this is my initial attempt to create a DDEX provider.
Since Npgsql can work without visual studio, it would be better to implement a thin layer supporting DDEX over Npgsql in visual studio when needed, and no dependencies attached when not need to. Thus, this merge is simply a separate VS package that can be installed or uninstalled using VS extension manager. You don't even need Npgsql to compile, but I add it to npgsql vs2012 sln for complete sake.
Right now, there's no data connection dialog support, only built-in VS dialog is used, so some properties might have problem showing itself upon changing the value, but it's just cosmetic, the underlying properties still works as expected.
Some part of Npgsql has been enhanced to support DDEX. Prime example is GetSchema API, it's useless if you cannot obtain primary keys and etc, so I add some more support.
StoreSchemaDefinitionVersion3 should be supported under NET45 and vs2012.
As said last time, there's bug somewhere preventing entity model from generating and I have pinpointed it now. It's a problem when the PrimitiveTypeKind is string or alike, your backend type conversion is smart and useful but it return a Byte[] as a result, but the visitedexpression engine only accept a String input to WriteSql, so things went haywire there. Right now, I fix it by convert utf8 encoded bytes to it's underlying ascii char, not sure if it's correct, but please check. I am a little curious here how this little error get unchecked.
There's still some known problem like DataSet cannot be added succussfully if you are using the data source wizard, i suspect it might be a bug in VS Designer. The Copy/Drag also not working as I am still figuring out why it said parameters are invalid.
Well, all in all, this should get a head start to let people and me to use EF using Npgsql with ease. Thanks.