Do not display passwords of GeoResources and Services in UI by nprigour · Pull Request #262

Do not display passwords of GeoResources and Services in UI by nprigour · Pull Request #262 · locationtech/udig-platform

Conversation

Usually we do not want passwords to be displayed in the UI (i.e. for catalog servives or layer resources corresponding to jdbc resources).
This pull request provides some extra methods in IGeoResource, IService and LayerImpl that can be used for displaying the ID value without displaying the password.

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

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

Example screenshots provided:

Layer Resource
Catalog View

		* @param layer
		* @return
		*/
		public static String getDisplayID(final Layer layer) {

Choose a reason for hiding this comment

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

I'm wondering why the method signiture has not been added to Interface org.locationtech.udig.project.internal.Layer
Since the interface is intenal, nobody else should have implemented another layer .. but could be because package org.locationtech.udig.project.internal is exported .. hmm

Any opinions?

Choose a reason for hiding this comment

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

Probably it could be also have been implemented the way you mention.
I think I choose to do it this way (static method) due to the fact that Layer inherits from EObject and I did not want to give the impression that this is a member returning function.
If you think it is more clear the way you propose we can change it.

Choose a reason for hiding this comment

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

@nprigour I had a look at the changset today. I'd like to understand two methods in LayerImpl and IGeoResource which looks like a bit duplicated code to me. Is there any reason why LayerSummary cannot add layer.getGeoResource().getDisplayID()?

The second note I have : Have you used the codeformatter.xml configuration to format new code? If not, please have a look at https://github.com/locationtech/udig-platform#preferences and format new code blocks you contributed. It helps us to read the code regarding tabs and spaces ;)

Many thanks

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

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

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

Hi @fgdrf ,

I have modified the code to LayerSummary in order to use IGeoResource DisplayID() method and remove it from LayerImpl. Also fix formatting using the udig formatter (sorry for the formatting issue but since I am also cpontributing to geotools and working to my own project on the same eclipse IDE, sometimes I forgot to change the used formatter!!)

I'm going to test rebasing to master and if no test failing I'm going to rebase and merge it. Thanks again for providing this improvement!

Signed-off-by: Frank Gasdorf <fgdrf@users.sourceforge.net>

Hi @nprigour,
Looks like everything works fine (at least on my machine). Please see pull request #6 your branch that addes a test case and rebased commits to latest improvements on master (where almost all tests are successful).

fgdrf added this to the uDig-2.0.0 milestone

Feb 11, 2018

Hi @fgdrf`,
I had a look at it and think it is pretty OK.

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

I also provide a small correction in LayerSummary that I introduce by inadvertence in my previous commit

Ηι @fgdrf

Is the 1 failure that jenkins CI produces related to this pull request? It does not seem very relevant to me

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Labels

2 participants