Update language server to support microprofile-config.properties

[jclingan]

Quarkus supports both src/main/resources/application.properties and src/main/resources/META-INF/microprofile-config.properties, the latter being the MicroProfile standard configuration file.

The language server supports application.properties but not microprofile-config.properties. This request is to add support for microprofile-config.properties to get syntax highlighting, code completion, etc.

The primary justification is that developers often like to develop portable code, and this request would help to achieve that goal while remaining productive. In addition, developers often want to try existing projects on Quarkus, and having code completion of existing microprofile-config.properties would greatly facilitate this use case without having to RTFM. Ex: "Re-hosting my project on Quarkus nearly works, but how the heck do I change the default http port"? Code-completion really helps with scenarios like this!

[angelozerr]

@jclingan thank's for having reported this issue.

If I understand correctly microprofile-config.properties should have only microprofile properties (ex : MyClass/mp-rest/url) and application.properties should have microprofile properties and Quarkus properties both.

Is that?

[jclingan]

Let me add @kenfinnigan. We would want any kind of property (quarkus, application, microprofile) supported by both property files. However, I want to check with Ken (who can pull in others if required) on having both property files at the same time for the same app. I wouldn't think we would want that.

[maxandersen]

Is this anything more than adding an additional filename the editor and validation extension will react to ?

[angelozerr]

Is this anything more than adding an additional filename the editor and validation extension will react to ?

If the idea is to have in microprofile-config.properties the same behavior than application.properties I think it can be easy to do. As @maxandersen it's just adding a file name is the package.json of the vscode-quarkus.

The only thing that we must manage too is on JDT LS extension side which load application.properties to get the server port (to manage CodeLens URL (base url)). We will need to load the both files.

[maxandersen]

codelens url ? what is that for ?

What from the properties file does JDT LS need to get a base url ? (I assume here JDT LS is just for java content assist/validation but sounds like there is something else at play here ? )

[angelozerr]

codelens url ? what is that for ?

It's for JAX-RS:

See PR at redhat-developer/quarkus-ls#140 (comment)

You can configure the port in application.properties with

quarkus.http.port=9090. In this case, the Codelens URL will display this port. The CodeLens is managed by the JDT LS extension because we need the Java AST to get the proper JAX RS method.

[kenfinnigan]

We definitely don't want both files present in the same app.

For Quarkus we push for MP and Quarkus config to be in application.properties.

The use case would really only be for migrations from other frameworks that use MicroProfile.

So the file only needs to support MicroProfile properties I think

[angelozerr]

@kenfinnigan did you mean the same thing than my comments #181 (comment) ?

[kenfinnigan]

Yes, sorry for confusion

[jclingan]

@angelozerr @kenfinnigan No, wait, not the same thing if i parse that comment correctly. Only one file should exist in a project, either microprofile-config.properties or application.properties. The chosen configuration file should contain all properties necessary to configure quarkus, microprofile, and the application.

BTW, if both exist, things get ugly with "which one takes priority", etc. It's just easier to say one or the other.

[kenfinnigan]

I would disagree that it makes sense for microprofile-config.properties to include Quarkus properties

[angelozerr]

I would disagree that it makes sense for microprofile-config.properties to include Quarkus properties

@jclingan @kenfinnigan once you will decide together what you want to have we will start implementing something. Please describe what you wish to have in detail (takes a sample and please tell me what you want to have (which properties for completion, etc).

The thing that I understand is that it's not possible to have the two files microprofile-config.properties and application.properties in the same project.

Perhaps we could have an error on the top of application.properties (when it is opened) and in microprofile-config.properties (when it is opened) to say "you cannot have microprofile-config.properties and application.properties file in the same project". We could have too a code action (quick fix) which provide "delete application.properties and copy properties in the microprofile-config.properties " (we should just check that LSP protocol can manage a delete file (@fbricon are you aware with that?)

[maxandersen]

See PR at redhat-developer/quarkus-ls#140 (comment)

You can configure the port in application.properties with

quarkus.http.port=9090. In this case, the Codelens URL will display this port. The CodeLens is managed by the JDT LS extension because we need the Java AST to get the proper JAX RS method.

okey, but this feature seem to be useful for a bunch of other tech (like any jax-rs app) so I reckon this feature would need to be able to look in multiple locations anyway :)

[maxandersen]

lets just keep this simple. the editor should work on any of these property files imo; no need to treat them differently at this stage.

wether we are interested in adding hard or even soft validation of what kind of properties are in them I'm +0 on as with custom config sources I don't see how we can even validate based on filename what is expected or not.

[jclingan]

I got a +1 from @dmlloyd as well. Waiting to get feedback from @kenfinnigan.

[kenfinnigan]

If consensus is to align them, I’m ok with that

…

Sent from my iPhone

On Jan 8, 2020, at 17:00, John Clingan ***@***.***> wrote:  I got a +1 from @dmlloyd as well. Waiting to get feedback from @kenfinnigan. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[jclingan]

Taking a look at other MicroProfile runtimes out there, it looks like they allow any property in any supported config source, but there is an order of precedence/priority. For quarkus, my tests (dev & prod modes) show that properties in application.properties overrides anything in microprofile-config.properties. I think that order is appropriate and supporting both simultaneously is proper behavior. @maxandersen, @kenfinnigan, @dmlloyd, agreed?

[jclingan]

OK, @angelozerr, via various methods we have come to an agreement. Here are the details;

• As a baseline, microprofile-config.properties editing should have the same behavior as application.properties. The only difference is the filename.
• Both application.properties and microprofile-config.properties can exist in the same project at the same time (FYI: application.properties takes precedence).
• Not immediately required but an additional nice feature would be yellow underlining of properties that are being overridden (or are overriding) the same property from another config source. For example, if quarkus.http.port is in both the microprofile-config.properties and application.properties, then the yellow underline message in microprofile-config.properties should be that it is being overridden by application.properties, and in application.properties, there should be a message that it is overriding the same property in microprofile-config.properties. Ditto for other config sources like: system properties ("-D"), env variables, application.properties, and microprofile-config.properties in that order of precedence (system property the highest priority) . I guess throw application.yaml in there too when tooling is ready. We don't have to get into custom or dynamic config sources.

[angelozerr]

@jclingan thank a lot for your detailed and very clear comment. We could start to manage microprofile-config.properties like application.properties which should not be hard.

[jclingan]

Thanks Angelo. When this is done, I can open a separate issue for the third bullet (underlining) when this issue is complete. I just wanted to give you a bit of direction of where we could go.

[angelozerr]

Thanks Angelo.

You are welcome!

When this is done, I can open a separate issue for the third bullet (underlining) when this issue is complete. I just wanted to give you a bit of direction of where we could go.

I will do that soon. You can now create an issue about the third bullet

[fbricon]

Fixed with redhat-developer/quarkus-ls#199