Skip to content

Fix Gradle add extension and add tests #4614

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 17, 2019
Merged

Fix Gradle add extension and add tests #4614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a393df7
Fix Gradle add extension and add tests
ia3andy Oct 16, 2019
38d237f
Fix compilation error in test
ia3andy Oct 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 90 additions & 43 deletions devtools/common/src/main/java/io/quarkus/cli/commands/file/GradleBuildFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,59 +13,43 @@
import java.util.function.Consumer;

import org.apache.maven.model.Dependency;
import org.apache.maven.model.Model;

import io.quarkus.cli.commands.writer.ProjectWriter;
import io.quarkus.dependencies.Extension;
import io.quarkus.generators.BuildTool;

public class GradleBuildFile extends BuildFile {

protected static final String BUILD_GRADLE_PATH = "build.gradle";
private static final String BUILD_GRADLE_PATH = "build.gradle";
private static final String SETTINGS_GRADLE_PATH = "settings.gradle";
private static final String GRADLE_PROPERTIES_PATH = "gradle.properties";

private String settingsContent = "";
private String buildContent = "";
private Properties propertiesContent = new Properties();
private Model model;

public GradleBuildFile(ProjectWriter writer) throws IOException {
public GradleBuildFile(ProjectWriter writer) {
super(writer, BuildTool.GRADLE);
}

@Override
public void close() throws IOException {
write(SETTINGS_GRADLE_PATH, settingsContent);
write(BUILD_GRADLE_PATH, buildContent);
write(SETTINGS_GRADLE_PATH, getModel().getSettingsContent());
write(BUILD_GRADLE_PATH, getModel().getBuildContent());
ByteArrayOutputStream out = new ByteArrayOutputStream();
propertiesContent.store(out, "Gradle properties");
getModel().getPropertiesContent().store(out, "Gradle properties");
write(GRADLE_PROPERTIES_PATH, out.toString(StandardCharsets.UTF_8.toString()));
}

@Override
public void completeFile(String groupId, String artifactId, String version)
throws IOException {
init();
completeSettingsContent(artifactId);
completeBuildContent(groupId, version);
completeProperties();
}

protected void init() throws IOException {
if (getWriter().exists(SETTINGS_GRADLE_PATH)) {
final byte[] settings = getWriter().getContent(SETTINGS_GRADLE_PATH);
settingsContent = new String(settings, StandardCharsets.UTF_8);
}
if (getWriter().exists(BUILD_GRADLE_PATH)) {
final byte[] build = getWriter().getContent(BUILD_GRADLE_PATH);
buildContent = new String(build, StandardCharsets.UTF_8);
}
if (getWriter().exists(GRADLE_PROPERTIES_PATH)) {
final byte[] properties = getWriter().getContent(GRADLE_PROPERTIES_PATH);
propertiesContent.load(new ByteArrayInputStream(properties));
}
}

private void completeBuildContent(String groupId, String version) {
private void completeBuildContent(String groupId, String version) throws IOException {
final String buildContent = getModel().getBuildContent();
StringBuilder res = new StringBuilder(buildContent);
if (!buildContent.contains("io.quarkus:quarkus-gradle-plugin")) {
res.append(System.lineSeparator());
Expand Down Expand Up @@ -103,11 +87,12 @@ private void completeBuildContent(String groupId, String version) {
res.append(System.lineSeparator()).append(versionLine)
.append(System.lineSeparator());
}
buildContent = res.toString();
getModel().setBuildContent(res.toString());
}

private void completeSettingsContent(String artifactId) {
StringBuilder res = new StringBuilder(settingsContent);
private void completeSettingsContent(String artifactId) throws IOException {
final String settingsContent = getModel().getSettingsContent();
final StringBuilder res = new StringBuilder(settingsContent);
if (!settingsContent.contains("io.quarkus:quarkus-gradle-plugin")) {
res.append(System.lineSeparator());
res.append("pluginManagement {").append(System.lineSeparator());
Expand All @@ -130,20 +115,20 @@ private void completeSettingsContent(String artifactId) {
res.append(System.lineSeparator()).append("rootProject.name='").append(artifactId).append("'")
.append(System.lineSeparator());
}
settingsContent = res.toString();
getModel().setSettingsContent(res.toString());
}

private void completeProperties() {
if (propertiesContent.getProperty("quarkusVersion") == null) {
propertiesContent.setProperty("quarkusVersion", getPluginVersion());
private void completeProperties() throws IOException {
if (getModel().getPropertiesContent().getProperty("quarkusVersion") == null) {
getModel().getPropertiesContent().setProperty("quarkusVersion", getPluginVersion());
}
}

@Override
protected void addDependencyInBuildFile(Dependency dependency) {
protected void addDependencyInBuildFile(Dependency dependency) throws IOException {
StringBuilder newBuildContent = new StringBuilder();
readLineByLine(buildContent, new AppendDependency(newBuildContent, dependency));
buildContent = newBuildContent.toString();
readLineByLine(getModel().getBuildContent(), new AppendDependency(newBuildContent, dependency));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while it's not an issue with this PR specifically, I was looking into the code for AppendDependency and I noticed 2 corner cases that don't currently work:

  1. It looks for line.startsWith("dependencies {" which doesn't account for additional spacing between dependencies and { and also doesn't account for leading whitespace (which is normal if this is being applied in a subprojects{} block for example).
  2. If we have the quarkus plugin installed already, but no dependencies{} at all, nothing gets added. For this case I think we should probably add the quarkus-bom dependency with a version corresponding to the quarkus-gradle-plugin, followed by the rest of the requested extension dependencies as normal

Copy link
Contributor Author

@ia3andy ia3andy Oct 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I didn't really look at the business involved.. I just fixed the bug using the same system used in MavenBuildFile and added the tests that should have been there in the first place..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point we are going to rewrite most of the codegen part with a real template engine for Quarkus that is beeing developed https://github.com/quarkusio/qute.

But for now, any corner case you see, please feel free to provide a PR with some tests :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this part of the code is messy currently... So trying to improve little things probably doesn't make a ton sense, tests on the other hand definitely do :).
@ia3andy did you sign up for rewriting the part this qute? :P

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@geoand why not, that might be fun, though for now I have plenty on my plate 😅

getModel().setBuildContent(newBuildContent.toString());
}

private void readLineByLine(String content, Consumer<String> lineConsumer) {
Expand Down Expand Up @@ -185,25 +170,25 @@ public void accept(String currentLine) {
}

@Override
protected boolean hasDependency(Extension extension) {
protected boolean hasDependency(Extension extension) throws IOException {
return getDependencies().stream()
.anyMatch(d -> extension.getGroupId().equals(d.getGroupId())
&& extension.getArtifactId().equals(d.getArtifactId()));
}

@Override
protected boolean containsBOM() {
return buildContent.contains("enforcedPlatform(\"io.quarkus:quarkus-bom:");
protected boolean containsBOM() throws IOException {
return getModel().getBuildContent().contains("enforcedPlatform(\"io.quarkus:quarkus-bom:");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

although G:A:V syntax is more common, people can also declare gradle dependencies with expanded syntax like this:

group: 'io.quarkus', name: 'quarkus-bom', version: '0.25.0'

with any amount of spacing or newlines in between

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, not mentioning the code formatting could vary. This functionality needs to be re-written properly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qute to the rescue :)

}

@Override
public List<Dependency> getDependencies() {
public List<Dependency> getDependencies() throws IOException {
return Collections.emptyList();
}

@Override
public String getProperty(String propertyName) {
return propertiesContent.getProperty(propertyName);
public String getProperty(String propertyName) throws IOException {
return getModel().getPropertiesContent().getProperty(propertyName);
}

@Override
Expand All @@ -212,7 +197,69 @@ protected List<Dependency> getManagedDependencies() {
return Collections.emptyList();
}

protected String getBuildContent() {
return buildContent;
private Model getModel() throws IOException {
if (model == null) {
initModel();
}
return model;
}

protected void initModel() throws IOException {
String settingsContent = "";
String buildContent = "";
Properties propertiesContent = new Properties();
if (getWriter().exists(SETTINGS_GRADLE_PATH)) {
final byte[] settings = getWriter().getContent(SETTINGS_GRADLE_PATH);
settingsContent = new String(settings, StandardCharsets.UTF_8);
}
if (getWriter().exists(BUILD_GRADLE_PATH)) {
final byte[] build = getWriter().getContent(BUILD_GRADLE_PATH);
buildContent = new String(build, StandardCharsets.UTF_8);
}
if (getWriter().exists(GRADLE_PROPERTIES_PATH)) {
final byte[] properties = getWriter().getContent(GRADLE_PROPERTIES_PATH);
propertiesContent.load(new ByteArrayInputStream(properties));
}
this.model = new Model(settingsContent, buildContent, propertiesContent);
}

protected String getBuildContent() throws IOException {
return getModel().getBuildContent();
}

private class Model {
private String settingsContent;
private String buildContent;
private Properties propertiesContent;

public Model(String settingsContent, String buildContent, Properties propertiesContent) {
this.settingsContent = settingsContent;
this.buildContent = buildContent;
this.propertiesContent = propertiesContent;
}

public String getSettingsContent() {
return settingsContent;
}

public String getBuildContent() {
return buildContent;
}

public Properties getPropertiesContent() {
return propertiesContent;
}

public void setSettingsContent(String settingsContent) {
this.settingsContent = settingsContent;
}

public void setBuildContent(String buildContent) {
this.buildContent = buildContent;
}

public void setPropertiesContent(Properties propertiesContent) {
this.propertiesContent = propertiesContent;
}
}
}
Loading