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

Comments

Projects
None yet
4 participants
@kdvolder
Member

kdvolder commented Apr 26, 2018

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

This comment has been minimized.

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

This comment has been minimized.

Member

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

This comment has been minimized.

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

This comment has been minimized.

Member

kdvolder commented Jun 15, 2018

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-issuemaster

This comment has been minimized.

spring-issuemaster commented Jun 18, 2018

(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