Skip to content

Conversation

@Croway
Copy link
Contributor

@Croway Croway commented May 9, 2024

No description provided.

@Croway Croway requested review from avano and mcarlett May 9, 2024 13:44
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;

public class ZipUtils {
Copy link

Choose a reason for hiding this comment

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

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.design.HideUtilityClassConstructorCheck> reported by reviewdog 🐶
Utility classes should not have a public or default constructor.


import com.google.auto.service.AutoService;

import software.tnb.product.LocalProduct;
Copy link

Choose a reason for hiding this comment

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

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.imports.ImportOrderCheck> reported by reviewdog 🐶
Wrong order for 'software.tnb.product.LocalProduct' import.

package software.tnb.product.csb.application;

import software.tnb.common.config.TestConfiguration;
import software.tnb.common.utils.WaitUtils;
Copy link

Choose a reason for hiding this comment

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

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.imports.UnusedImportsCheck> reported by reviewdog 🐶
Unused import - software.tnb.common.utils.WaitUtils.

import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
Copy link

Choose a reason for hiding this comment

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

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.imports.UnusedImportsCheck> reported by reviewdog 🐶
Unused import - java.nio.file.Paths.

try {
tomcatProcess = processBuilder.start();
Runtime.getRuntime().addShutdownHook(new Thread(() -> {
if(tomcatProcess != null) {
Copy link

Choose a reason for hiding this comment

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

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.whitespace.WhitespaceAfterCheck> reported by reviewdog 🐶
'if' is not followed by whitespace.

class MySpringBootApplication extends org.springframework.boot.web.servlet.support.SpringBootServletInitializer {
@Override
protected org.springframework.boot.builder.SpringApplicationBuilder configure(org.springframework.boot.builder.SpringApplicationBuilder application) {
Copy link

Choose a reason for hiding this comment

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

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck> reported by reviewdog 🐶
Line is longer than 150 characters (found 174).

import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;

import software.tnb.common.utils.WaitUtils;
Copy link

Choose a reason for hiding this comment

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

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.imports.ImportOrderCheck> reported by reviewdog 🐶
Wrong order for 'software.tnb.common.utils.WaitUtils' import.

@Croway Croway force-pushed the springboot-tomcat branch 2 times, most recently from d8b6f09 to 227bcbd Compare May 9, 2024 13:50
@Croway Croway force-pushed the springboot-tomcat branch from 227bcbd to ab49566 Compare May 9, 2024 14:01
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;

public class ZipUtils {
Copy link

Choose a reason for hiding this comment

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

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.design.FinalClassCheck> reported by reviewdog 🐶
Class ZipUtils should be declared as final.


import com.google.auto.service.AutoService;

import software.tnb.product.util.maven.Maven;
Copy link

Choose a reason for hiding this comment

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

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.imports.ImportOrderCheck> reported by reviewdog 🐶
Wrong order for 'software.tnb.product.util.maven.Maven' import.

package software.tnb.product.csb.application;

import software.tnb.common.config.TestConfiguration;
import software.tnb.product.application.App;
Copy link

Choose a reason for hiding this comment

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

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.imports.UnusedImportsCheck> reported by reviewdog 🐶
Unused import - software.tnb.product.application.App.

import software.tnb.product.application.App;
import software.tnb.product.application.Phase;
import software.tnb.product.csb.configuration.SpringBootConfiguration;
import software.tnb.product.customizer.Customizer;
Copy link

Choose a reason for hiding this comment

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

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.imports.UnusedImportsCheck> reported by reviewdog 🐶
Unused import - software.tnb.product.customizer.Customizer.

import software.tnb.product.application.Phase;
import software.tnb.product.csb.configuration.SpringBootConfiguration;
import software.tnb.product.customizer.Customizer;
import software.tnb.product.customizer.component.rest.RestCustomizer;
Copy link

Choose a reason for hiding this comment

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

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.imports.UnusedImportsCheck> reported by reviewdog 🐶
Unused import - software.tnb.product.customizer.component.rest.RestCustomizer.

import software.tnb.product.util.maven.Maven;

import org.apache.commons.io.FileUtils;
import org.apache.maven.model.Dependency;
Copy link

Choose a reason for hiding this comment

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

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.imports.UnusedImportsCheck> reported by reviewdog 🐶
Unused import - org.apache.maven.model.Dependency.

import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
Copy link

Choose a reason for hiding this comment

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

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.imports.UnusedImportsCheck> reported by reviewdog 🐶
Unused import - java.util.List.

import java.nio.file.Path;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
Copy link

Choose a reason for hiding this comment

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

🚫 [reviewdog] <com.puppycrawl.tools.checkstyle.checks.imports.UnusedImportsCheck> reported by reviewdog 🐶
Unused import - java.util.stream.Collectors.

@Override
public void setupProduct() {
super.setupProduct();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

not necessary here since it only calls super

@Override
public void teardownProduct() {
app.stop();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

apps are stopped via Product#removeIntegrations


@Override
public App createIntegrationApp(AbstractIntegrationBuilder<?> integrationBuilder) {
// Let's remove restcustomizer, since it exclude tomcat
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say restcustomizer (and all customizers) should be changed to handle the tomcat as well, rather than changing it here

public class TomcatSpringBootApp extends SpringBootApp {
private static final Logger LOG = LoggerFactory.getLogger(TomcatSpringBootApp.class);

private AbstractIntegrationBuilder<?> integrationBuilder;
Copy link
Contributor

Choose a reason for hiding this comment

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

not used for anything

private static final String TOMCAT_ARCHIVE_NAME = "tomcat-archive.zip";

@Override
public Log getLog() {
Copy link
Contributor

Choose a reason for hiding this comment

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

set the log variable after the app is started, instead of overriding this method

Comment on lines +109 to +133
Path pomPath = TestConfiguration.appLocation().resolve(name).resolve("pom.xml");
String pom = Files.readString(pomPath);
pom = pom.replace("<artifactId>" + name + "</artifactId>",
"<artifactId>" + name + "</artifactId><packaging>war</packaging>");
Files.write(pomPath, pom.getBytes(StandardCharsets.UTF_8));

Path mainPath = TestConfiguration.appLocation().resolve(name)
.resolve("src")
.resolve("main")
.resolve("java")
.resolve("com")
.resolve("test")
.resolve("MySpringBootApplication.java");
String main = Files.readString(mainPath);

main = main.replace("class MySpringBootApplication {",
"""
class MySpringBootApplication extends org.springframework.boot.web.servlet.support.SpringBootServletInitializer {
@Override
protected org.springframework.boot.builder.SpringApplicationBuilder
configure(org.springframework.boot.builder.SpringApplicationBuilder application) {
return application.sources(MySpringBootApplication.class);
}
""");
Copy link
Contributor

Choose a reason for hiding this comment

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

if this is something that should be done for all tomcat apps, it should be done by a TomcatCustomizer

""");
Files.write(mainPath, main.getBytes(StandardCharsets.UTF_8));
} catch (IOException e) {
throw new RuntimeException(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

throw new RuntimeException("Unable to start application", e);


ZipUtils.unzip(tomcatTmpDirectory.resolve(TOMCAT_ARCHIVE_NAME), tomcatTmpDirectory.resolve(TOMCAT_PARENT_DIRECTORY));
} catch (IOException e) {
throw new RuntimeException(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

throw new RuntimeException("Unable to download tomcat", e);

Files.copy(TestConfiguration.appLocation().resolve(name).resolve("target").resolve(warName),
tomcatHome.resolve("webapps").resolve(warName));
} catch (IOException e) {
throw new RuntimeException(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

throw new RuntimeException("Unable to copy file", e);

}

public static String tomcatZipUrl() {
return getProperty(TOMCAT_ZIP_DOWNLOAD_URL, "https://dlcdn.apache.org/tomcat/tomcat-10/v10.1.23/bin/apache-tomcat-10.1.23.zip");
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't the version be configurable?

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

Successfully merging this pull request may close these issues.

3 participants