improve check for jdbc catalog IDs by nprigour · Pull Request #231

improve check for jdbc catalog IDs by nprigour · Pull Request #231 · locationtech/udig-platform

Conversation

Minor improvement so that ID.isJDBC() method also checks the URL protocol field to determine if ID refers to a database

Signed-off-by: Nikolaos Pringouris nprigour@gmail.com

Signed-off-by: Nikolaos Pringouris <nprigour@gmail.com>

		*/
		public boolean isJDBC() {
		return id.startsWith("jdbc"); //$NON-NLS-1$
		return id.startsWith("jdbc") \|\| (url != null && url.getProtocol().contains("jdbc")) ; //$NON-NLS-1$ //$NON-NLS-2$

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nprigour Can you describe a scenario if the id wouldnt start with jdbc while protocol could be jdbc? I'm not sure but if the first is true the second statement isn't required. If the first is false the second should either, shouldn't it?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ID provides various constructors which do not necessary guarantee that id value is always derived from the URL. Moreover the GeoResource implementation provided for different databases do not directly use the jdbc URL passed during creation of java JDBC connection. To give an example with such IDs produced fot Postgres and Mysql when using the default implementations provided by UDIG:

Postgres --> example id value jdbc.postgis://postgres@localhost:5432/db_name#testbed
Mysql --> example id value mysql.jdbc://user:psswd@localhost:3306/db_name#test_1 (see also attached screenshot)
In the mysql case it is evident that id does not start with jdbc

@naprigour Thanks for explanation and the fix!

fgdrf added this to the uDig-2.0.0 milestone

May 29, 2017

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Labels

2 participants