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

Edit Starters doesn't properly handle removal of all dependencies #52

Closed
kdvolder opened this issue Apr 26, 2018 · 5 comments
Closed

Edit Starters doesn't properly handle removal of all dependencies #52

kdvolder opened this issue Apr 26, 2018 · 5 comments
Milestone

Comments

@kdvolder
Copy link
Member

When all dependencies are removed/unchecked from a boot project. The edit starters tends to create a broken project because it does not ensure that at leats a dependency like:

<dependency>
    <groupId>org.springframework.boot</groupId>
    <artifactId>spring-boot-starter</artifactId>
</dependency>

Steps to reproduce:

  1. create a project with "new Spring starter". Select a single dependency (e.g. web).
  2. Use "Edit starters" and remove the dependency.

=> The dependency is removed from the pom leaving no dependencies in the pom at all.

The project is now broken and has compile errors.

Expected behavior is that upon removal of the last dependency we ensure that at least a dependency on
the 'base' spring-boot-starter dependency exists. If it doesn't it should be added.

@Eskibear
Copy link
Contributor

Eskibear commented May 2, 2018

The expected behavior is not enough, a corner case is found.

The starter devtools only introduces following dependency in runtime scope:

    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-devtools</artifactId>
      <scope>runtime</scope>
    </dependency>

So we still need to add the base starter back if it's the only starter of the project.

Ref:
microsoft/vscode-spring-initializr#64

@kdvolder
Copy link
Member Author

kdvolder commented May 2, 2018

@Eskibear

I just got a report from another user in slack chat, complaining about other discrepancies between edit starters and what intializer app generates.

Apparantly since this was implemented in STS quite a while ago, the initializer app has gotten a bit more sophisticated and it is not just a 1-1 mapping between a selected checkbox and a dependency. There are also some dependencies that get added as a kind of 'cross selection'. I.e. sometimes extra dependencies will get added as 'extra' when two boxes are selected together.

The example he gave was:

If I select RabbitMQ and Cloud Stream on a new project, I get the amqp starter, spring-cloud-stream, spring-cloud-stream-binder-rabbit and spring-cloud-stream-test-support.

If you instead only selects RabbitMQ intially; and subsequently add 'Cloud Stream' via edit starters. Then spring-cloud-stream-binder-rabbit and spring-cloud-stream-test-support are not being added.

I'm not totally sure yet how we can handle this. But admittedly I think it should be thought of as a bug.

I have some idea. Perhaps the 'Edit Starters' should be re-written from scratch so that it doesn't try to decide by itself how to create the edits. Instead, we could use initializr app to create two poms. One for the 'initial' state of the edit starters dialog selection, and then a second one for the edited state. Then we can compute some kind of a diff between the two poms, and then somehow use that diff to update the user's pom. I think this might be a better way, as it wouldn't need to know exactly how the initializer logic works internally. And perhaps it would solve all of these discrepancies at once (though I guess implementing the diffing system might be a little tricky, as I doubdt that simply doing a textual diff will be good enough).

@Eskibear
Copy link
Contributor

Eskibear commented May 3, 2018

Yes we also noticed the similar scenarios. I think the best way is to have related APIs telling the exact mapping of dependencies. Diffing XML content is also acceptable. But plain textual diff also has some pitalls, e.g. compare the following two BOMs :

  <dependencyManagement>
    <dependencies>
      <dependency>
        <groupId>com.microsoft.azure</groupId>
        <artifactId>azure-spring-boot-bom</artifactId>
        <version>2.0.1</version>      <!-- HERE -->
        <type>pom</type>
        <scope>import</scope>
      </dependency>
    </dependencies>
  </dependencyManagement>
    <properties>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
        <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
        <azure.version>2.0.1</azure.version>       <!-- HERE -->
        <java.version>1.8</java.version>
    </properties>

    <dependencyManagement>
        <dependencies>
            <dependency>
                <groupId>com.microsoft.azure</groupId>
                <artifactId>azure-spring-boot-bom</artifactId>
                <version>${azure.version}</version>      <!-- HERE -->
                <type>pom</type>
                <scope>import</scope>
            </dependency>
        </dependencies>
    </dependencyManagement>

@kdvolder
Copy link
Member Author

Above commit only fixes the 'empty selection' case. It doesn't deal with more complex scenarios such as cross-selection of starters (i.e. where adding two dependencies together, causes other 'cross-election' dependencies to be added as well)

@spring-projects-issues
Copy link

(comment in Pivotal Tracker added by Martin Lippert:)

I accepted this story, seems to work fine, but uncovered a little bug in the edit starters wizard, please also take a look at bug item #158427358. Thanks!!!

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

No branches or pull requests

4 participants