Skip to content
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

ConfigurationMetadataProperty name not stripped with same source types #16549

Closed
jvalkeal opened this issue Apr 12, 2019 · 5 comments
Closed
Assignees
Labels
type: bug A general bug
Milestone

Comments

@jvalkeal
Copy link
Contributor

Sample for this issue is in https://github.com/jvalkeal/randomstuff/tree/master/metadatanaming which uses 2.1.4.RELEASE.

Running a sample app prints (ConfigurationMetadataProperty.getId() / ConfigurationMetadataProperty.getName()):

spring.rabbitmq.listener.direct.acknowledge-mode / acknowledge-mode
spring.rabbitmq.listener.direct.auto-startup / auto-startup
spring.rabbitmq.listener.direct.consumers-per-queue / consumers-per-queue
spring.rabbitmq.listener.direct.default-requeue-rejected / default-requeue-rejected
spring.rabbitmq.listener.direct.idle-event-interval / idle-event-interval
spring.rabbitmq.listener.direct.missing-queues-fatal / missing-queues-fatal
spring.rabbitmq.listener.direct.prefetch / prefetch
spring.rabbitmq.listener.direct.retry.enabled / enabled
spring.rabbitmq.listener.direct.retry.initial-interval / initial-interval
spring.rabbitmq.listener.direct.retry.max-attempts / max-attempts
spring.rabbitmq.listener.direct.retry.max-interval / max-interval
spring.rabbitmq.listener.direct.retry.multiplier / multiplier
spring.rabbitmq.listener.direct.retry.stateless / stateless
spring.rabbitmq.listener.simple.acknowledge-mode / acknowledge-mode
spring.rabbitmq.listener.simple.auto-startup / auto-startup
spring.rabbitmq.listener.simple.concurrency / concurrency
spring.rabbitmq.listener.simple.default-requeue-rejected / default-requeue-rejected
spring.rabbitmq.listener.simple.idle-event-interval / idle-event-interval
spring.rabbitmq.listener.simple.max-concurrency / max-concurrency
spring.rabbitmq.listener.simple.missing-queues-fatal / missing-queues-fatal
spring.rabbitmq.listener.simple.prefetch / prefetch
spring.rabbitmq.listener.simple.retry.enabled / spring.rabbitmq.listener.simple.retry.enabled
spring.rabbitmq.listener.simple.retry.initial-interval / spring.rabbitmq.listener.simple.retry.initial-interval
spring.rabbitmq.listener.simple.retry.max-attempts / spring.rabbitmq.listener.simple.retry.max-attempts
spring.rabbitmq.listener.simple.retry.max-interval / spring.rabbitmq.listener.simple.retry.max-interval
spring.rabbitmq.listener.simple.retry.multiplier / spring.rabbitmq.listener.simple.retry.multiplier
spring.rabbitmq.listener.simple.retry.stateless / spring.rabbitmq.listener.simple.retry.stateless
spring.rabbitmq.listener.simple.transaction-size / transaction-size

As you can see every property under spring.rabbitmq.listener.simple.retry group will not strip groupId from its name.

When you i.e. use ConfigurationMetadataRepositoryJsonBuilder manually to build repository to get access to ConfigurationMetadataProperty's, RawConfigurationMetadata is getting slightly confused when it's trying to strip group id from a fully qualified property name to set a simple name.

Below is metadata for spring.rabbitmq.listener.direct.retry.enabled and spring.rabbitmq.listener.simple.retry.enabled which have a same sourceType.

{
  "properties": [
    {
      "name": "spring.rabbitmq.listener.direct.retry.enabled",
      "type": "java.lang.Boolean",
      "description": "Whether publishing retries are enabled.",
      "sourceType": "org.springframework.boot.autoconfigure.amqp.RabbitProperties$ListenerRetry",
      "defaultValue": false
    },
    {
      "name": "spring.rabbitmq.listener.simple.retry.enabled",
      "type": "java.lang.Boolean",
      "description": "Whether publishing retries are enabled.",
      "sourceType": "org.springframework.boot.autoconfigure.amqp.RabbitProperties$ListenerRetry",
      "defaultValue": false
    }
  ]
}

Issue seem to be RawConfigurationMetadata.getSource() which returns first ConfigurationMetadataSource matching a source type. As seen above, these two properties have different group but same sourceType, thus first match is returned. However as seen in below, when actual ConfigurationMetadataItem name is tried to re-set to a simple name(stripping group id), group prefix doesn't match, thus stripping doesn't happen.

if (hasLength(groupId) && id.startsWith(dottedPrefix)) {
String name = id.substring(dottedPrefix.length());
item.setName(name);
}

I created this issue from spring-cloud/spring-cloud-dataflow-ui#769 and as @snicoll asked to create a sample to provide more details. MetadataResolver class in a sample is pretty much how we're using boot metadata in a dataflow space.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 12, 2019
@jvalkeal jvalkeal changed the title ConfigurationMetadataProperty name contains group id with same types ConfigurationMetadataProperty name not stripped with same source types Apr 12, 2019
@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 12, 2019
@philwebb philwebb added this to the 2.1.x milestone Apr 12, 2019
@philwebb
Copy link
Member

ListenerRetry is mapped to two different sources because it's in DirectContainer and SimpleContainer which both extend AmqpContainer. This setup is fairly rare and I guess we just didn't account for it.

@jvalkeal
Copy link
Contributor Author

@philwebb AmqpContainer was just given as an example to re-prod this issue. This same thing happens other places, especially in dataflow apps, deployers, etc where some metadata properties extends same classes.

@philwebb
Copy link
Member

@jvalkeal Understood. It just took me a while to work out why the same sourceType was registered with two different names and I wanted to add some notes to the issue to jog my memory later.

@jvalkeal
Copy link
Contributor Author

@philwebb I had a chat with Stephane when he was just to board his flight and I think he was planning to poke around things while being on a cloud.

@philwebb
Copy link
Member

OK, I'll assign it to him so nobody else picks it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants