Incorrect/incomplete metadata for 'cloud-function' dependency

[kdvolder]

See: spring-projects/spring-tools#313

Summary:

When we create a project with initializer and select both 'cloud-function' and 'webflux' dependencies, then a pom with these dependencies is created:

	<dependencies>
		<dependency>
			<groupId>org.springframework.boot</groupId>
			<artifactId>spring-boot-starter-webflux</artifactId>
		</dependency>
		<dependency>
			<groupId>org.springframework.cloud</groupId>
			<artifactId>spring-cloud-function-web</artifactId>
		</dependency>

		<dependency>
			<groupId>org.springframework.boot</groupId>
			<artifactId>spring-boot-starter-test</artifactId>
			<scope>test</scope>
		</dependency>
		<dependency>
			<groupId>io.projectreactor</groupId>
			<artifactId>reactor-test</artifactId>
			<scope>test</scope>
		</dependency>
	</dependencies>

According to the metadata returned from this end-point:

https://start-development.cfapps.io/dependencies?bootVersion=2.1.6.RELEASE

The starter with id 'cloud-function' corresponds to a maven dependency with gid:aid of: org.springframework.cloud:spring-cloud-function-context. That dependency is not present in the pom unfortunately.

THe upshot of this is that STS Edit Starters functionality gets confused about which starters are selected in this pom. (There is no way for STS to know that org.springframework.cloud:spring-cloud-function-web represents the 'cloud-function' starter dependency). This makes the Edit Starters wizard behave incorrectly whenever a pom contains that element.

[wilkinsona]

It is intentional that the spring-cloud-function-context dependency is being replaced with another dependency. The logic that does this is here:

start.spring.io/start-site/src/main/java/io/spring/start/site/extension/springcloud/SpringCloudFunctionBuildCustomizer.java

Lines 50 to 71 in 0c96d47

	@Override
	public void customize(Build build) {
	DependencyContainer dependencies = build.dependencies();
	if (dependencies.has("cloud-function")) {
	if ((dependencies.has("cloud-stream") || dependencies.has("reactive-cloud-stream"))
	&& isSpringBootVersionBefore()) {
	dependencies.add("cloud-function-stream", "org.springframework.cloud", "spring-cloud-function-stream",
	DependencyScope.COMPILE);
	removeCloudFunction(build);
	}
	if (dependencies.has("web")) {
	dependencies.add("cloud-function-web", "org.springframework.cloud", "spring-cloud-function-web",
	DependencyScope.COMPILE);
	removeCloudFunction(build);
	}
	if (dependencies.has("webflux") && isSpringBootVersionAtLeastAfter()) {
	dependencies.add("cloud-function-web", "org.springframework.cloud", "spring-cloud-function-web",
	DependencyScope.COMPILE);
	removeCloudFunction(build);
	}
	}
	}

The intended use of the metadata is as an input into project generation. Additional customization is then performed to generate the output. As such, I disagree that the metadata is incomplete or incorrect. The problem is the assumptions that STS is making about how the metadata is used and how something in the metadata will be represented in the resulting output. Those assumptions do not hold true and have not done so for a long time. For example, Spring Session support has swapped out the dependency in the metadata for another one based on other dependencies that are being used since at least July 2017.

Ideally, STS's Edit Starters functionality needs to be implemented without making assumptions about the output based on the metadata input. If that's not possible, I'm not sure what we can do. The swapping of dependencies is intentionally done programmatically rather than declaratively. A declarative approach would be too cumbersome and may not even be able to provide the required flexibility. The existing YAML is large and unwieldy enough as it is. We don't want to make that worse by trying to program in YAML.

[martinlippert]

Understand the issue and the reasons for doing it the way it is done in the initializr codebase. Programming in YAML is definitely not an option for sure... :-)

Ideally, STS's Edit Starters functionality needs to be implemented without making assumptions about the output based on the metadata input. If that's not possible, I'm not sure what we can do.

The question for me here is how STS could implement this. The underlying task is basically: here is a pom file, give me the modules in initializr that resulted in this pom. Since initializr has a hand-coded step in between that can basically do anything, this inverted function doesn't seem possible to me (without producing a combination matrix with exponential complexity... ;-).

Looking at this from a tooling PM perspective, I would still love to keep the "Edit Starters" wizard around and working. Our users seem to love it. Any idea or suggestion how to implement this "inverted function"?

One (maybe crazy idea): what about enhancing initializr itself to do this "inverted function" (maybe with a future edit-starters-feature in mind, don't know). In that case, the customize method would get a counterpart that implements the reverse function. It would add some extra burden on the initializr project, but might open the door for more tooling integrations to create something like the "edit starters" feature, not only STS.

Thoughts?

[wilkinsona]

It's regrettable that the use of the metadata by the Edit Starters functionality wasn't discussed with the Initializr team before it was rolled out. While I can understand that users seem to love it, I don't think there's anything that we can do in the short or even medium term to improve the situation on the Initializr side. However, given that the functionality's been broken in some scenarios for a couple of years (Spring Session), perhaps that's not such a big problem.

We have discussed having support for making additions existing projects but it's not any easy problem to solve. We haven't really considered removing things. It adds significantly to the complexity of the problem and I'm not sure the benefit is sufficient to warrant tackling that complexity. We've considered supporting making additions to a project via the Web UI and also via developer tooling. FWIW, I think that developer tooling is the better place for it as all the information about the project is readily available without the user having to upload all of the relevant pieces. This may lead to an API that could be used by Boot's Maven and Gradle plugins as well as the IDEs but it's a long way off assuming it's even possible at all.

[kdvolder]

It's regrettable that the use of the metadata by the Edit Starters functionality wasn't discussed with the Initializr team before it was rolled out.

Uhm... how do you think this metadata got added? I asked @snicoll explained why we need it, and he implemented it.

[kdvolder]

Can we just focus on the immediate problem rather than get bogged down in deep discussions like this:

We have discussed having support for making additions existing projects but it's not any easy problem to solve.

I agree its hard, but the problem we are having at the moment is fairly simple. STS needs to be able to lookup a maven dependency like this:

<dependency>
			<groupId>org.springframework.cloud</groupId>
			<artifactId>spring-cloud-function-web</artifactId>
		</dependency>

... and conclude that it represents the 'cloud-function' starter.

All that would really be needed to address that concern is a relatively minor tweak to the metadata returned from this endpoint: https://start-development.cfapps.io/dependencies?bootVersion=2.1.6.RELEASE.

[kdvolder]

Here's a proposal to get the ball rolling... Instead of returning this bit of json:

cloud-function: {
  groupId: "org.springframework.cloud",
  artifactId: "spring-cloud-function-context",
  scope: "compile",
  bom: "spring-cloud"
}

Instead return something like this:

cloud-function: {
  groupId: "org.springframework.cloud",
  artifactId: "spring-cloud-function-context",
  scope: "compile",
  bom: "spring-cloud"
  alternatives: [{
     groupId: "org.springframework.cloud",
     artifactId: "spring-cloud-function-web"
  }, ...
  ]
}

That way STS will have all the information it needs to implement Edit Starters correctly.

[wilkinsona]

Uhm... how do you think this metadata got added? I asked @snicoll explained why we need it, and he implemented it.

Sorry, I evidently wasn't involved in that discussion. Can you please link to the relevant issue and commit?

All that would really be needed to address that concern is a relatively minor tweak to the metadata returned from this endpoint

That may well be a minor tweak from the perspective of the metadata, but it has a significant impact on the implementation. It adds complexity and also requires some duplication. Let's wait until Stephane is back in the office next week so that we have the full picture. As things stand, I am opposed to trying to paper over the cracks in this manner.

[kdvolder]

Can you please link to the relevant issue and commit?

I beleave it was discussed mostly over Skype chat. I also recall that Stephane seemed to have forgotten about the discussion, as last time he talked to me about it a few months ago he seemed surprised we had this working. But I assure you, the endpoint was created because I asked for it and it was clear at the time, to me at least, that the purpose of the end-point being put in was to help STS implement 'Edit Starters'.

As things stand, I am opposed to trying to paper over the cracks in this manner.

I think that is a bit sad and counter productive. The only alternative for STS is that we add some special case logic into the STS side, something like if (artficatId.equals("cloud-function-web")) { ... do special things ...}.

But isn't that precisely what you are wanting to avoid moving forward when you say things like this:

It's regrettable that the use of the metadata by the Edit Starters functionality wasn't discussed with the Initializr team.

I agree, there seems to be a communication problem, but unless you actually do something on the initializer side to provide STS with the information it needs, then you really don't leave us much choice but to come up with some hacky / creative solutions on our own.

[wilkinsona]

I haven't said that we should do nothing, just that I am not in favour of papering over the cracks.

If we believe the functionality is important, I would rather that we design and implement official support for making modifications to an existing project. This functionality could then be used by IDEs, build plugins, the spring CLI, etc. If the functionality is not deemed sufficiently important to warrant that development time then my preference would be that the existing feature is removed from STS.

[kdvolder]

I haven't said that we should do nothing

Okay it sounded like that to me. I stand corrected.

If we believe the functionality is important,

I think I speak for all of STS team when I say that indeed we do.

I would rather that we design and implement official support for making modifications to an existing project.

Great! I would love to engage in a deeper discussion. However, I would also not hold my breath on finding a 'perfect' solution here any time soon. And as the saying goes, we shouldn't "make the perfect be the enemy of the good". And I think we would still want a fix for the current bug in the shorter term.

my preference would be that the existing feature is removed from STS.

I don't think that is going to happen. If we can't get traction on getting support for it from the boot/initializer team, it is more likely we will come up with some 'creative' solutions on our end to fix the bugs known to us as best as we can with or without explicit support from initializer.

[kdvolder]

Not really sure where we stand on this after the meeting we just had in zoom. Yes, it was the right thing to focus the zoom chat on the bigger issues. But that leaves the question now... how is this ticket getting resolved? Is there any chance to add some extra metadata for the temporary fix we need today, or shall we decide to handle it purely on the STS side?

[martinlippert]

I discussed the temporary workaround for the extra metadata with the team again and we came to the conclusion that is doesn't make a lot of sense (technically) to add that extra metadata to the service. It is a pity, but I understand the reasons for that.

Since we are working towards a much better solution in the future, we should move this back to the STS issuetracker and see what we can or should do in the meantime as a temporary workaround, probably document this as a limitation of the existing wizard - together with other limitations that exist in the wizard in addition to this one.