Validate datasource name duplication before the save

[acumarav]

Validate datasource name duplication before the save.
Show user friendly validation error message.

[OdysseusBot]

Can one of the admins verify this patch?

[YaroslavTir]

can you extract artifact version as property?
Do we really need this library? I do not see that we are using it other projects.

[acumarav]

done

[acumarav]

this lib is already part of spring-boot-starter-test. I want to use newer version. But lib itself is very cool.

[YaroslavTir]

I would prefer reverse this condition, I don't really like negation in if condition:

if (dataSourceService.isDatasourceNameUnique(dataSourceName, dataSourceId)){
 return Optional.empty();
}

[acumarav]

it is possible to introduce variable like:
bool isDatasourceWithTheSameNameAlreadyExists = ataSourceService.isDatasourceNameUnique(dataSourceName, dataSourceId);
if(isDatasourceWithTheSameNameAlreadyExists){
return Error....
}

but I do not like in offered code snipped that one validation criteria become a reason to resolve entire validation chain. It will be working only if this that check is the last one. IMHO code will not become cleaner if I apply this change.

[YaroslavTir]

well done, writing tests are cool. :)

[wivern]

Could you parameterize method to avoid warnings about unchecked assignments?

[wivern]

@acumarav could you make a parameterized method?

[acumarav]

done

[wivern]

But why not to use JsonResult.setValidatorErrors with a Map containing single element as an argument?

[wivern]

@acumarav what you think about comment above?

[acumarav]

I have changed it but j in the current codebase there are 46 usages of: .getValidatorErrors().put(
and only 2 of: .setValidatorErrors(
so I would say both ways are correct

[wivern]

There are lot of usage of this. Why this changes required?

[acumarav]

On one side - Security is checked/handled by the spring so it is overkill to force user handle it too..
and it is inconvenient to use methods with throws non-runtime exceptions in the lamdas.
So I think it will be good to mark them as Runtime to avoid necessary try {
}catch
in business logic.