This is an automated email from the ASF dual-hosted git repository. stariy95 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/cayenne.git
commit 923c5feb55bdd975c24514c082118006346bcf15 Author: ntimofeev <[email protected]> AuthorDate: Thu Aug 20 13:59:36 2026 +0300 CAY-2994 cgen: improve path handling --- RELEASE-NOTES.txt | 1 + .../apache/cayenne/tools/CayenneGeneratorTask.java | 2 +- .../org/apache/cayenne/gen/CgenConfiguration.java | 137 ++++---- .../apache/cayenne/gen/ClassGenerationAction.java | 9 +- .../org/apache/cayenne/gen/internal/Utils.java | 18 +- .../apache/cayenne/gen/xml/CgenConfigHandler.java | 18 +- .../apache/cayenne/gen/xml/CgenSaverDelegate.java | 26 +- .../cayenne/gen/BaseTemplatesGenerationTest.java | 2 +- .../apache/cayenne/gen/CgenConfigurationTest.java | 368 ++++++++++++++++----- .../cayenne/gen/ClassGenerationActionTest.java | 6 +- .../cayenne/gen/xml/CgenSaverDelegateTest.java | 70 ++-- .../java/org/apache/cayenne/tools/CgenTask.java | 4 +- .../apache/cayenne/tools/CayenneGeneratorMojo.java | 4 +- .../apache/cayenne/mcp/tools/cgen/CgenRunTool.java | 8 +- .../mcp/tools/cgen/CgenRunValidationTest.java | 34 +- .../apache/cayenne/modeler/project/CgenOps.java | 13 +- .../editor/datadomain/cgen/DataDomainCgenTab.java | 3 +- .../editor/datamap/cgen/CgenConfigPanel.java | 13 +- .../ui/project/editor/datamap/cgen/CgenPanel.java | 2 +- 19 files changed, 494 insertions(+), 244 deletions(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index fc75d68b7..88cc14a06 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -38,6 +38,7 @@ CAY-2986 Cgen to run unconditionally CAY-2987 DataNode to own PkGenerator CAY-2989 Upgrade MCP SDK to 2.0 CAY-2992 dbimport: support posgres drivers newer than 42.7.4 +CAY-2994 cgen: improve path handling CAY-2995 Migrate Modeler to FlatLaf library Bug Fixes: diff --git a/cayenne-ant/src/main/java/org/apache/cayenne/tools/CayenneGeneratorTask.java b/cayenne-ant/src/main/java/org/apache/cayenne/tools/CayenneGeneratorTask.java index dc19c0539..5317d94c8 100644 --- a/cayenne-ant/src/main/java/org/apache/cayenne/tools/CayenneGeneratorTask.java +++ b/cayenne-ant/src/main/java/org/apache/cayenne/tools/CayenneGeneratorTask.java @@ -174,7 +174,7 @@ public class CayenneGeneratorTask extends CayenneTask { CgenConfiguration cgenConfiguration = new CgenConfiguration(); cgenConfiguration.setDataMap(dataMap); if(destDir != null) { - cgenConfiguration.updateOutputPath(destDir.toPath()); + cgenConfiguration.setOutputDir(destDir.toPath()); } cgenConfiguration.setEncoding(encoding != null ? encoding : cgenConfiguration.getEncoding()); cgenConfiguration.setMakePairs(makepairs != null ? makepairs : cgenConfiguration.isMakePairs()); diff --git a/cayenne-cgen/src/main/java/org/apache/cayenne/gen/CgenConfiguration.java b/cayenne-cgen/src/main/java/org/apache/cayenne/gen/CgenConfiguration.java index b19121edd..ac4576276 100644 --- a/cayenne-cgen/src/main/java/org/apache/cayenne/gen/CgenConfiguration.java +++ b/cayenne-cgen/src/main/java/org/apache/cayenne/gen/CgenConfiguration.java @@ -36,6 +36,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.HashSet; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -48,16 +49,17 @@ import java.util.stream.Collectors; public class CgenConfiguration implements Serializable, XMLSerializable { /** - * Should point to the directory that holds Cayenne project XML. - * Could be null in some cases now. + * Absolute directory this configuration is anchored to, normally the one holding the Cayenne + * project XML. Null when cgen is driven directly by a tool (Ant, Maven or Gradle). */ private Path rootProjectPath; /** - * Target directory for generated classes, relative to the {@code rootProjectPath} - * (if root path is set, and it's possible to relativize). + * Target directory for the generated classes, stored exactly as it was supplied - either absolute, + * or relative to the {@code rootProjectPath}. Never null: an empty path means "the root directory" + * when a root is known, and "not configured" when it isn't. */ - private Path cgenOutputPath; + private Path cgenOutputPath = Path.of(""); private final Collection<Artifact> artifacts; private Set<String> entityArtifacts; @@ -129,7 +131,7 @@ public class CgenConfiguration implements Serializable, XMLSerializable { * Builds a default configuration for a DataMap that has no stored cgen config, generating every * non-generic entity and embeddable into the given output directory. * - * @param outputDir directory to generate classes into; if {@code null}, the output path is left unset + * @param outputDir directory to generate classes into, or {@code null} to derive a default * @since 5.0 */ public static CgenConfiguration createDefault(DataMap dataMap, Path outputDir) { @@ -137,27 +139,19 @@ public class CgenConfiguration implements Serializable, XMLSerializable { config.setDataMap(dataMap); dataMap.getObjEntities().forEach(config::loadEntity); dataMap.getEmbeddables().forEach(config::loadEmbeddable); - if (dataMap.getConfigurationSource() != null) { - config.setRootPath(Utils.getRootPathForDataMap(dataMap)); + + Path root = Utils.rootPathForDataMap(dataMap).orElse(null); + if (root != null) { + config.setRootPath(root); } if (outputDir != null) { - config.updateOutputPath(outputDir); + config.setOutputDir(outputDir); + } else if (root != null) { + Utils.getMavenSrcPathForPath(root).map(Path::of).ifPresent(config::setOutputDir); } return config; } - /** - * Returns the default output directory for a saved DataMap, derived from the standard Maven - * source layout ({@code src/main/resources} → {@code src/main/java}, likewise for - * {@code test}), falling back to the DataMap's own directory for non-Maven layouts. - * - * @since 5.0 - */ - public static Path defaultOutputDir(DataMap dataMap) { - Path mapDir = Utils.getRootPathForDataMap(dataMap); - return Utils.getMavenSrcPathForPath(mapDir).map(Path::of).orElse(mapDir); - } - public void resetCollections() { embeddableArtifacts.clear(); entityArtifacts.clear(); @@ -214,7 +208,7 @@ public class CgenConfiguration implements Serializable, XMLSerializable { /** * @param rootProjectPath root path for the Cayenne project this config relates to - * @see #updateOutputPath(Path) + * @see #setOutputDir(Path) */ public void setRootPath(Path rootProjectPath) { if (!Objects.requireNonNull(rootProjectPath).isAbsolute()) { @@ -224,70 +218,83 @@ public class CgenConfiguration implements Serializable, XMLSerializable { } /** - * Method returns output path as is, without any processing. + * Stores the output directory exactly as supplied. Setting the output directory and setting + * the root path are independent operations that may happen in any order; they are combined + * lazily by {@link #outputDirectory()} and at serialization time. * - * @return cgen output relative path - * @see #buildOutputPath() - * @since 5.0 renamed from {@code getRelPath()} + * @param dir absolute directory, or a directory relative to the {@code rootProjectPath} + * @see #setRootPath(Path) + * @see #outputDirectory() + * @since 5.0 replaces {@code updateOutputPath()} */ - public Path getRawOutputPath() { - return cgenOutputPath; + public void setOutputDir(Path dir) { + this.cgenOutputPath = Objects.requireNonNull(dir, "Null output directory"); } /** - * Method that updates output path based on provided {@code Path} and {@code rootProjectPath} + * Calculates the effective output directory by combining the stored output directory with the {@code rootProjectPath}. + * + * @return the effective output directory, or an empty {@code Optional} when the stored path is + * relative and there is no root path to resolve it against * - * @param path to update output path with, could be an absolute path or a path - * relative to the {@code rootProjectPath} or cgen tool environment * @see #setRootPath(Path) - * @since 5.0 + * @see #setOutputDir(Path) + * @since 5.0 replaces {@code buildOutputPath()} */ - public void updateOutputPath(Path path) { + public Optional<Path> outputDirectory() { + if (cgenOutputPath.isAbsolute()) { + return Optional.of(cgenOutputPath.normalize()); + } if (rootProjectPath != null) { - if (path.isAbsolute() && rootProjectPath.getRoot().equals(path.getRoot())) { - this.cgenOutputPath = rootProjectPath.relativize(path); - return; - } + return Optional.of(rootProjectPath.resolve(cgenOutputPath).normalize()); } - this.cgenOutputPath = path; + return Optional.empty(); } /** - * @return normalized relative path - * @since 5.0 renamed from {@code buildRelPath()} and made package private + * @return the effective output directory + * @throws ValidationException if no output directory has been configured + * @see #outputDirectory() + * @since 5.0 */ - String getNormalizedOutputPath() { - if (cgenOutputPath == null || cgenOutputPath.toString().isEmpty()) { - return "."; - } - return cgenOutputPath.toString(); + public Path requireOutputDirectory() { + return outputDirectory().orElseThrow(() -> new ValidationException("Output directory is not set.")); } /** - * This method calculates effective output directory for the class generator. - * It uses {@code cgenOutputPath} and {@code rootProjectPath} (if set). + * Moves this configuration to a new root path while keeping the effective output directory pointing + * at the same physical location. Used when a project is saved to a new location. A configuration + * that never had an output directory keeps following the root. * - * @return calculated output directory + * @param newRoot new absolute root path * @see #setRootPath(Path) - * @see #updateOutputPath(Path) - * @since 5.0 renamed from {@code buildPath()} + * @since 5.0 */ - public Path buildOutputPath() { - if (rootProjectPath == null) { - // this could be an unsaved project or direct usage in tools (Ant, Maven or Gradle) - return cgenOutputPath; - } - - if (cgenOutputPath == null) { - // this case should be invalid, but let the caller deal with it - return null; + public void rebase(Path newRoot) { + Path effective = outputDirectory().orElse(null); + setRootPath(newRoot); + if (effective != null) { + this.cgenOutputPath = effective; } + } - if (cgenOutputPath.isAbsolute()) { - return cgenOutputPath.normalize(); - } else { - return rootProjectPath.resolve(cgenOutputPath).toAbsolutePath().normalize(); + /** + * Renders the output directory for the {@code destDir} XML tag: relative to the root path whenever + * the two can be relativized, absolute otherwise, always with Unix separators. + * + * @return the {@code destDir} value, never empty + * @since 5.0 + */ + String encodedDestDir() { + Path out = cgenOutputPath; + if (rootProjectPath != null + && rootProjectPath.isAbsolute() + && out.isAbsolute() + && Objects.equals(rootProjectPath.getRoot(), out.getRoot())) { + out = rootProjectPath.relativize(out); } + String encoded = out.toString(); + return encoded.isEmpty() ? "." : separatorsToUnix(encoded); } public boolean isOverwrite() { @@ -508,13 +515,13 @@ public class CgenConfiguration implements Serializable, XMLSerializable { } @Override - public void encodeAsXML(XMLEncoder encoder, ConfigurationNodeVisitor delegate) { + public void encodeAsXML(XMLEncoder encoder, ConfigurationNodeVisitor<?> delegate) { encoder.start("cgen") .attribute("xmlns", CgenExtension.NAMESPACE) .simpleTag("name", this.name) .simpleTag("excludeEntities", getExcludedEntities()) .simpleTag("excludeEmbeddables", getExcludedEmbeddables()) - .simpleTag("destDir", separatorsToUnix(getNormalizedOutputPath())) + .simpleTag("destDir", encodedDestDir()) .simpleTag("mode", this.artifactsGenerationMode.getLabel()) .start("template").cdata(this.template.getData(), !this.template.isFile()).end() .start("superTemplate").cdata(this.superTemplate.getData(), !this.superTemplate.isFile()).end() diff --git a/cayenne-cgen/src/main/java/org/apache/cayenne/gen/ClassGenerationAction.java b/cayenne-cgen/src/main/java/org/apache/cayenne/gen/ClassGenerationAction.java index 82a322b54..76d97c984 100644 --- a/cayenne-cgen/src/main/java/org/apache/cayenne/gen/ClassGenerationAction.java +++ b/cayenne-cgen/src/main/java/org/apache/cayenne/gen/ClassGenerationAction.java @@ -280,10 +280,7 @@ public class ClassGenerationAction { * Called internally from "execute". */ protected void validateAttributes() { - Path dir = configuration.buildOutputPath(); - if (dir == null) { - throw new CayenneRuntimeException("Output directory is not set."); - } + Path dir = configuration.requireOutputDirectory(); if (Files.notExists(dir)) { try { Files.createDirectories(dir); @@ -336,7 +333,7 @@ public class ClassGenerationAction { String packageName = (String) context.get(Artifact.SUPER_PACKAGE_KEY); String className = (String) context.get(Artifact.SUPER_CLASS_KEY); - File dir = mkpath(configuration.buildOutputPath().toFile(), packageName); + File dir = mkpath(configuration.requireOutputDirectory().toFile(), packageName); String fileName = StringUtils .getInstance() .replaceWildcardInStringWithString(WILDCARD, configuration.getOutputPattern(), className); @@ -354,7 +351,7 @@ public class ClassGenerationAction { String className = (String) context.get(Artifact.SUB_CLASS_KEY); String filename = StringUtils.getInstance().replaceWildcardInStringWithString(WILDCARD, configuration.getOutputPattern(), className); - File dest = new File(mkpath(configuration.buildOutputPath().toFile(), packageName), filename); + File dest = new File(mkpath(configuration.requireOutputDirectory().toFile(), packageName), filename); if (dest.exists()) { // no overwrite of subclasses diff --git a/cayenne-cgen/src/main/java/org/apache/cayenne/gen/internal/Utils.java b/cayenne-cgen/src/main/java/org/apache/cayenne/gen/internal/Utils.java index c664fcfc0..8ac0a3df1 100644 --- a/cayenne-cgen/src/main/java/org/apache/cayenne/gen/internal/Utils.java +++ b/cayenne-cgen/src/main/java/org/apache/cayenne/gen/internal/Utils.java @@ -19,11 +19,11 @@ package org.apache.cayenne.gen.internal; -import org.apache.cayenne.CayenneRuntimeException; import org.apache.cayenne.map.DataMap; import java.io.File; import java.net.URISyntaxException; +import java.nio.file.FileSystemNotFoundException; import java.nio.file.Files; import java.nio.file.Path; import java.util.Optional; @@ -59,20 +59,24 @@ public class Utils { return Optional.empty(); } - public static Path getRootPathForDataMap(DataMap dataMap) { - if(dataMap.getConfigurationSource() == null) { - throw new CayenneRuntimeException("Unable to create path from the unsaved DataMap"); + /** + * @return directory holding the DataMap, or {@code Optional.empty()} for a DataMap that has no resolvable location + */ + public static Optional<Path> rootPathForDataMap(DataMap dataMap) { + if (dataMap.getConfigurationSource() == null) { + return Optional.empty(); } Path resourcePath; try { resourcePath = Path.of(dataMap.getConfigurationSource().getURL().toURI()); - } catch (URISyntaxException e) { - throw new CayenneRuntimeException("Unable to create path from the DataMap source location", e); + } catch (URISyntaxException | IllegalArgumentException | FileSystemNotFoundException e) { + // not a "file:" URL - e.g. a DataMap loaded from a jar + return Optional.empty(); } if (Files.isRegularFile(resourcePath)) { resourcePath = resourcePath.getParent(); } - return resourcePath; + return Optional.ofNullable(resourcePath); } private static String checkDefaultMavenResourceDir(String path, String dirType) { diff --git a/cayenne-cgen/src/main/java/org/apache/cayenne/gen/xml/CgenConfigHandler.java b/cayenne-cgen/src/main/java/org/apache/cayenne/gen/xml/CgenConfigHandler.java index a552ff52c..002cc42d9 100644 --- a/cayenne-cgen/src/main/java/org/apache/cayenne/gen/xml/CgenConfigHandler.java +++ b/cayenne-cgen/src/main/java/org/apache/cayenne/gen/xml/CgenConfigHandler.java @@ -18,13 +18,8 @@ ****************************************************************/ package org.apache.cayenne.gen.xml; -import java.net.URISyntaxException; -import java.net.URL; -import java.nio.file.Files; -import java.nio.file.Path; import java.nio.file.Paths; -import org.apache.cayenne.CayenneRuntimeException; import org.apache.cayenne.configuration.xml.DataChannelMetaData; import org.apache.cayenne.configuration.xml.NamespaceAwareNestedTagHandler; import org.apache.cayenne.gen.CgenConfiguration; @@ -32,7 +27,6 @@ import org.apache.cayenne.gen.CgenConfigList; import org.apache.cayenne.gen.CgenTemplate; import org.apache.cayenne.gen.TemplateType; import org.apache.cayenne.gen.internal.Utils; -import org.apache.cayenne.map.DataMap; import org.xml.sax.Attributes; /** @@ -60,6 +54,7 @@ public class CgenConfigHandler extends NamespaceAwareNestedTagHandler { private static final String EXCLUDE_EMBEDDABLES_TAG = "excludeEmbeddables"; private static final String CREATE_PK_PROPERTIES = "createPKProperties"; private static final String SUPER_PKG_TAG = "superPkg"; + private static final String EXTERNAL_TOOL_CONFIG_TAG = "externalToolConfig"; public static final String TRUE = "true"; @@ -153,7 +148,7 @@ public class CgenConfigHandler extends NamespaceAwareNestedTagHandler { if (path.trim().length() == 0) { return; } - configuration.updateOutputPath(Paths.get(path)); + configuration.setOutputDir(Paths.get(path)); } private void createGenerationMode(String mode) { @@ -293,10 +288,17 @@ public class CgenConfigHandler extends NamespaceAwareNestedTagHandler { configuration.setSuperPkg(data); } + private void createExternalToolConfig(String data) { + if (data.trim().length() == 0) { + return; + } + configuration.setExternalToolConfig(data); + } + private void createConfig() { loaderContext.addDataMapListener(dataMap -> { configuration.setDataMap(dataMap); - configuration.setRootPath(Utils.getRootPathForDataMap(dataMap)); + Utils.rootPathForDataMap(dataMap).ifPresent(configuration::setRootPath); configuration.resolveExcludedEntities(); configuration.resolveExcludedEmbeddables(); diff --git a/cayenne-cgen/src/main/java/org/apache/cayenne/gen/xml/CgenSaverDelegate.java b/cayenne-cgen/src/main/java/org/apache/cayenne/gen/xml/CgenSaverDelegate.java index 4e774502b..286974ad2 100644 --- a/cayenne-cgen/src/main/java/org/apache/cayenne/gen/xml/CgenSaverDelegate.java +++ b/cayenne-cgen/src/main/java/org/apache/cayenne/gen/xml/CgenSaverDelegate.java @@ -63,25 +63,19 @@ public class CgenSaverDelegate extends BaseSaverDelegate { } Path baseDirectory = getBaseDirectoryForURL(baseURL); - Path prevRootPath = cgenConfiguration.getRootPath(); - Path prevOutputPath = cgenConfiguration.buildOutputPath(); - // Update cgen root path. - cgenConfiguration.setRootPath(baseDirectory); + // A config with no root and no output dir has never been configured at all; anything else already + // points somewhere, and rebasing keeps it pointing at the same physical directory. + boolean neverConfigured = cgenConfiguration.getRootPath() == null + && cgenConfiguration.outputDirectory().isEmpty(); - // If no root path was set, try to calculate if we are inside Maven tree structure and use it - if(prevRootPath == null) { + cgenConfiguration.rebase(baseDirectory); + + if(neverConfigured) { + // Inside a Maven tree the sources dir is a better default than the project dir itself. + // Otherwise the empty output path already resolves to the new root. Utils.getMavenSrcPathForPath(baseDirectory) .map(Path::of) - .ifPresent(cgenConfiguration::updateOutputPath); - } - - if(prevOutputPath != null) { - // Update relative path to match with the new root - cgenConfiguration.updateOutputPath(prevOutputPath); - } else if(cgenConfiguration.buildOutputPath() == null) { - // No path was set, and we are not in the Maven tree. - // Set output dir match with the root, nothing else we could do here. - cgenConfiguration.updateOutputPath(baseDirectory); + .ifPresent(cgenConfiguration::setOutputDir); } } diff --git a/cayenne-cgen/src/test/java/org/apache/cayenne/gen/BaseTemplatesGenerationTest.java b/cayenne-cgen/src/test/java/org/apache/cayenne/gen/BaseTemplatesGenerationTest.java index 8b7359774..f72a4b019 100644 --- a/cayenne-cgen/src/test/java/org/apache/cayenne/gen/BaseTemplatesGenerationTest.java +++ b/cayenne-cgen/src/test/java/org/apache/cayenne/gen/BaseTemplatesGenerationTest.java @@ -149,7 +149,7 @@ public class BaseTemplatesGenerationTest extends CgenCase { cgenConfiguration.addArtifact(artifact); cgenConfiguration.setRootPath(folder.toPath()); - cgenConfiguration.updateOutputPath(Paths.get(".")); + cgenConfiguration.setOutputDir(Paths.get(".")); cgenConfiguration.loadEntity(objEntity); cgenConfiguration.setDataMap(dataMap); diff --git a/cayenne-cgen/src/test/java/org/apache/cayenne/gen/CgenConfigurationTest.java b/cayenne-cgen/src/test/java/org/apache/cayenne/gen/CgenConfigurationTest.java index 18d63718b..0717a4be6 100644 --- a/cayenne-cgen/src/test/java/org/apache/cayenne/gen/CgenConfigurationTest.java +++ b/cayenne-cgen/src/test/java/org/apache/cayenne/gen/CgenConfigurationTest.java @@ -40,12 +40,24 @@ import java.util.Locale; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; public class CgenConfigurationTest { + /** + * No root and a relative output path: nothing to resolve the output against, so no output directory + * has been configured at all. + */ + @Test + public void relativeOutputDirWithoutRootIsNotConfigured() { + CgenConfiguration configuration = new CgenConfiguration(); + configuration.setOutputDir(Paths.get("out")); + + assertTrue(configuration.outputDirectory().isEmpty()); + assertThrows(ValidationException.class, configuration::requireOutputDirectory); + } + @Nested public class CreateDefaultTest { @@ -83,15 +95,12 @@ public class CgenConfigurationTest { public void derivesMavenOutputDir(@TempDir Path tmp) throws IOException { Path resources = Files.createDirectories(tmp.resolve("src/main/resources")); Path mapFile = Files.createFile(resources.resolve("test.map.xml")); - DataMap map = savedMap(mapFile); - // src/main/resources -> src/main/java - Path outputDir = CgenConfiguration.defaultOutputDir(map); - assertEquals(tmp.resolve("src/main/java"), outputDir); + // src/main/resources -> src/main/java, derived from the map's own directory + CgenConfiguration config = CgenConfiguration.createDefault(savedMap(mapFile), null); - CgenConfiguration config = CgenConfiguration.createDefault(map, outputDir); assertEquals(resources, config.getRootPath()); - assertEquals(tmp.resolve("src/main/java"), config.buildOutputPath()); + assertEquals(tmp.resolve("src/main/java"), config.requireOutputDirectory()); } @Test @@ -101,21 +110,199 @@ public class CgenConfigurationTest { CgenConfiguration config = CgenConfiguration.createDefault(savedMap(mapFile), outputDir); - assertEquals(outputDir, config.buildOutputPath()); + assertEquals(outputDir, config.requireOutputDirectory()); + } + + /** + * The Import DataMap state: the map has a configuration source but no output dir was supplied, + * and the layout is not a Maven one. Used to leave the config with a root and no output path at + * all, which made the effective output directory null. + */ + @Test + public void savedMapWithoutOutputDirFallsBackToMapDir(@TempDir Path tmp) throws IOException { + Path mapFile = Files.createFile(tmp.resolve("test.map.xml")); + + CgenConfiguration config = CgenConfiguration.createDefault(savedMap(mapFile), null); + + assertEquals(tmp, config.getRootPath()); + assertEquals(tmp, config.requireOutputDirectory()); + assertEquals(".", config.encodedDestDir()); } @Test public void unsavedMapSkipsRootPath() { CgenConfiguration config = CgenConfiguration.createDefault(unsavedMap(), null); - assertNull(config.getRootPath()); - assertNull(config.buildOutputPath()); + assertTrue(config.outputDirectory().isEmpty()); + assertThrows(ValidationException.class, config::requireOutputDirectory); // artifacts still populated even without a saved location assertTrue(config.getEntities().contains("Person")); assertTrue(config.getEmbeddables().contains("com.example.Address")); } } + /** + * Setting the root path and the output directory are independent writes; neither reads the other, so + * the order in which they happen must not change the outcome. The XML loader sets them in one order + * (destDir during SAX parsing, root from a later DataMap callback) and everything else in the other. + */ + @Nested + public class OrderIndependenceTest { + + private CgenConfiguration rootFirst(Path root, Path out) { + CgenConfiguration config = new CgenConfiguration(); + config.setRootPath(root); + config.setOutputDir(out); + return config; + } + + private CgenConfiguration outputFirst(Path root, Path out) { + CgenConfiguration config = new CgenConfiguration(); + config.setOutputDir(out); + config.setRootPath(root); + return config; + } + + private void assertCommutes(Path root, Path out) { + CgenConfiguration a = rootFirst(root, out); + CgenConfiguration b = outputFirst(root, out); + + assertEquals(a.outputDirectory(), b.outputDirectory()); + assertEquals(a.encodedDestDir(), b.encodedDestDir()); + } + + @Test + public void absoluteUnderRoot(@TempDir Path tmp) { + assertCommutes(tmp, tmp.resolve("out")); + } + + @Test + public void absoluteOutsideRoot(@TempDir Path tmp) { + assertCommutes(tmp.resolve("project"), tmp.resolve("elsewhere/out")); + } + + @Test + public void relative(@TempDir Path tmp) { + assertCommutes(tmp, Paths.get("../java")); + } + + @Test + public void empty(@TempDir Path tmp) { + assertCommutes(tmp, Paths.get("")); + } + } + + /** + * {@code destDir} is what actually lands in the DataMap XML, so it has to survive an arbitrary number + * of save/load cycles unchanged. + */ + @Nested + public class DestDirRoundTripTest { + + private void assertRoundTrips(Path root, String destDir) { + CgenConfiguration loaded = new CgenConfiguration(); + loaded.setOutputDir(Paths.get(destDir)); + loaded.setRootPath(root); + + String encoded = loaded.encodedDestDir(); + + CgenConfiguration reloaded = new CgenConfiguration(); + reloaded.setOutputDir(Paths.get(encoded)); + reloaded.setRootPath(root); + + assertEquals(encoded, reloaded.encodedDestDir()); + assertEquals(loaded.outputDirectory(), reloaded.outputDirectory()); + } + + @Test + public void currentDir(@TempDir Path tmp) { + assertRoundTrips(tmp, "."); + } + + @Test + public void parentDir(@TempDir Path tmp) { + assertRoundTrips(tmp, "../java"); + } + + @Test + public void grandParentDir(@TempDir Path tmp) { + assertRoundTrips(tmp, "../../java"); + } + + @Test + public void plainChildDir(@TempDir Path tmp) { + assertRoundTrips(tmp, "cgenConfigTest"); + } + + @Test + public void emptyOutputPathEncodesAsCurrentDir(@TempDir Path tmp) { + CgenConfiguration config = new CgenConfiguration(); + config.setRootPath(tmp); + + assertEquals(".", config.encodedDestDir()); + assertEquals(tmp, config.requireOutputDirectory()); + } + } + + /** + * Rebasing follows the project to a new location while the generated classes keep going to the same + * physical directory. A configuration that never had an output directory keeps following the root. + */ + @Nested + public class RebaseTest { + + @Test + public void relativeOutputKeepsPointingAtTheSameDir(@TempDir Path tmp) { + Path oldRoot = tmp.resolve("old"); + Path newRoot = tmp.resolve("new"); + + CgenConfiguration config = new CgenConfiguration(); + config.setRootPath(oldRoot); + config.setOutputDir(Paths.get("../java")); + + config.rebase(newRoot); + + assertEquals(newRoot, config.getRootPath()); + assertEquals(tmp.resolve("java"), config.requireOutputDirectory()); + assertEquals("../java", config.encodedDestDir()); + } + + @Test + public void absoluteOutputSurvives(@TempDir Path tmp) { + Path target = tmp.resolve("generated"); + + CgenConfiguration config = new CgenConfiguration(); + config.setRootPath(tmp.resolve("old")); + config.setOutputDir(target); + + config.rebase(tmp.resolve("new")); + + assertEquals(target, config.requireOutputDirectory()); + } + + @Test + public void neverConfiguredFollowsTheRoot(@TempDir Path tmp) { + Path newRoot = tmp.resolve("new"); + + CgenConfiguration config = new CgenConfiguration(); + config.rebase(newRoot); + + assertEquals(newRoot, config.requireOutputDirectory()); + assertEquals(".", config.encodedDestDir()); + } + + @Test + public void rejectsRelativeRoot() { + CgenConfiguration config = new CgenConfiguration(); + assertThrows(ValidationException.class, () -> config.rebase(Paths.get("relative"))); + } + } + + /** + * Windows has more than one filesystem root - drive letters and UNC shares - and {@code relativize()} + * throws across them. Output directories that cannot be relativized against the project root must be + * stored and persisted absolute, unchanged. See CAY-2615 and CAY-2710. + */ @Nested public class CgenWindowsConfigurationTest { @@ -134,81 +321,102 @@ public class CgenConfigurationTest { @Test public void equalRootsEqualDirectories() { configuration.setRootPath(Paths.get("C:\\test1\\test2\\test3")); - Path relPath = Paths.get("C:\\test1\\test2\\test3"); - configuration.updateOutputPath(relPath); + Path outputDir = Paths.get("C:\\test1\\test2\\test3"); + configuration.setOutputDir(outputDir); - assertEquals(Paths.get(""), configuration.getRawOutputPath()); - assertEquals(relPath, configuration.buildOutputPath()); + assertEquals(".", configuration.encodedDestDir()); + assertEquals(outputDir, configuration.requireOutputDirectory()); } @Test public void equalRootsNotEqualDirectories() { configuration.setRootPath(Paths.get("C:\\test1\\test2\\test3")); - Path relPath = Paths.get("C:\\test1\\test2\\testAnother"); - configuration.updateOutputPath(relPath); + Path outputDir = Paths.get("C:\\test1\\test2\\testAnother"); + configuration.setOutputDir(outputDir); - assertEquals(Paths.get("..\\testAnother"), configuration.getRawOutputPath()); - assertEquals(relPath, configuration.buildOutputPath()); + assertEquals("../testAnother", configuration.encodedDestDir()); + assertEquals(outputDir, configuration.requireOutputDirectory()); } @Test public void equalRootsEmptyDirectories() { configuration.setRootPath(Paths.get("C:\\")); - Path relPath = Paths.get("C:\\"); - configuration.updateOutputPath(relPath); + Path outputDir = Paths.get("C:\\"); + configuration.setOutputDir(outputDir); - assertEquals(Paths.get(""), configuration.getRawOutputPath()); - assertEquals(relPath, configuration.buildOutputPath()); + assertEquals(".", configuration.encodedDestDir()); + assertEquals(outputDir, configuration.requireOutputDirectory()); } @Test public void notEqualRootsEqualDirectories() { configuration.setRootPath(Paths.get("C:\\test1\\test2\\test3")); - Path relPath = Paths.get("E:\\test1\\test2\\test3"); - configuration.updateOutputPath(relPath); + Path outputDir = Paths.get("E:\\test1\\test2\\test3"); + configuration.setOutputDir(outputDir); - assertEquals(Paths.get("E:\\test1\\test2\\test3"), configuration.getRawOutputPath()); - assertEquals(relPath, configuration.buildOutputPath()); + assertEquals("E:/test1/test2/test3", configuration.encodedDestDir()); + assertEquals(outputDir, configuration.requireOutputDirectory()); } @Test public void notEqualRootsNotEqualDirectories() { configuration.setRootPath(Paths.get("C:\\test1\\test2\\test3")); - Path relPath = Paths.get("E:\\test1\\test2\\testAnother"); - configuration.updateOutputPath(relPath); + Path outputDir = Paths.get("E:\\test1\\test2\\testAnother"); + configuration.setOutputDir(outputDir); - assertEquals(Paths.get("E:\\test1\\test2\\testAnother"), configuration.getRawOutputPath()); - assertEquals(relPath, configuration.buildOutputPath()); + assertEquals("E:/test1/test2/testAnother", configuration.encodedDestDir()); + assertEquals(outputDir, configuration.requireOutputDirectory()); } @Test public void notEqualRootsEmptyDirectories() { configuration.setRootPath(Paths.get("C:\\")); - Path relPath = Paths.get("E:\\"); - configuration.updateOutputPath(relPath); + Path outputDir = Paths.get("E:\\"); + configuration.setOutputDir(outputDir); + + assertEquals("E:/", configuration.encodedDestDir()); + assertEquals(outputDir, configuration.requireOutputDirectory()); + } - assertEquals(Paths.get("E:\\"), configuration.getRawOutputPath()); - assertEquals(relPath, configuration.buildOutputPath()); + /** + * UNC shares are separate filesystem roots too: same share relativizes, different shares don't. + */ + @Test + public void uncShares() { + configuration.setRootPath(Paths.get("\\\\server\\share\\project")); + configuration.setOutputDir(Paths.get("\\\\server\\share\\java")); + assertEquals("../java", configuration.encodedDestDir()); + + configuration.setOutputDir(Paths.get("\\\\server\\other\\java")); + assertEquals("//server/other/java", configuration.encodedDestDir()); + assertEquals(Paths.get("\\\\server\\other\\java"), configuration.requireOutputDirectory()); + } + + /** + * A drive-relative path such as {@code C:foo} carries a root but is not absolute, so it cannot be + * relativized against an absolute root - {@code relativize()} would throw "different type of Path". + */ + @Test + public void driveRelativeOutputDirIsNotRelativized() { + configuration.setRootPath(Paths.get("C:\\test1")); + configuration.setOutputDir(Paths.get("C:foo")); + + assertEquals("C:foo", configuration.encodedDestDir()); + assertEquals(Paths.get("C:\\test1\\foo"), configuration.requireOutputDirectory()); } @Test public void emptyRootNotEmptyRelPath() { - Path relPath = Paths.get("E:\\"); - assertThrows(ValidationException.class, () -> { - configuration.setRootPath(Paths.get("")); - configuration.updateOutputPath(relPath); - }); + assertThrows(ValidationException.class, () -> configuration.setRootPath(Paths.get(""))); } @Test public void notEmptyRootEmptyRelPath() { configuration.setRootPath(Paths.get("E:\\")); - Path relPath = Paths.get(""); + configuration.setOutputDir(Paths.get("")); - configuration.updateOutputPath(relPath); - - assertEquals(relPath, configuration.getRawOutputPath()); - assertEquals(Paths.get("E:\\"), configuration.buildOutputPath()); + assertEquals(".", configuration.encodedDestDir()); + assertEquals(Paths.get("E:\\"), configuration.requireOutputDirectory()); } @Test @@ -218,9 +426,10 @@ public class CgenConfigurationTest { @Test public void nullRootPath() { - configuration.updateOutputPath(Path.of("C:\\test1\\test2\\test3")); - assertEquals(Paths.get("C:\\test1\\test2\\test3"), configuration.getRawOutputPath()); - assertEquals(Paths.get("C:\\test1\\test2\\test3"), configuration.buildOutputPath()); + configuration.setOutputDir(Path.of("C:\\test1\\test2\\test3")); + + assertEquals("C:/test1/test2/test3", configuration.encodedDestDir()); + assertEquals(Paths.get("C:\\test1\\test2\\test3"), configuration.requireOutputDirectory()); } } @@ -242,84 +451,67 @@ public class CgenConfigurationTest { @Test public void equalRootsEqualDirectories() { configuration.setRootPath(Paths.get("/test1/test2/test3")); - Path relPath = Paths.get("/test1/test2/test3"); - configuration.updateOutputPath(relPath); - + Path outputDir = Paths.get("/test1/test2/test3"); + configuration.setOutputDir(outputDir); - assertEquals(Paths.get(""), configuration.getRawOutputPath()); - assertEquals(relPath, configuration.buildOutputPath()); + assertEquals(".", configuration.encodedDestDir()); + assertEquals(outputDir, configuration.requireOutputDirectory()); } @Test public void equalRootsNotEqualDirectories() { configuration.setRootPath(Paths.get("/test1/test2/test3")); - Path relPath = Paths.get("/test1/test2/testAnother"); - configuration.updateOutputPath(relPath); + Path outputDir = Paths.get("/test1/test2/testAnother"); + configuration.setOutputDir(outputDir); - assertEquals(Paths.get("../testAnother"), configuration.getRawOutputPath()); - assertEquals(relPath, configuration.buildOutputPath()); + assertEquals("../testAnother", configuration.encodedDestDir()); + assertEquals(outputDir, configuration.requireOutputDirectory()); } @Test public void equalRootsEmptyDirectories() { configuration.setRootPath(Paths.get("/")); - Path relPath = Paths.get("/"); - configuration.updateOutputPath(relPath); + Path outputDir = Paths.get("/"); + configuration.setOutputDir(outputDir); - assertEquals(Paths.get(""), configuration.getRawOutputPath()); - assertEquals(relPath, configuration.buildOutputPath()); + assertEquals(".", configuration.encodedDestDir()); + assertEquals(outputDir, configuration.requireOutputDirectory()); } @Test public void concatCorrectRootPathAndRelPath() { configuration.setRootPath(Paths.get("/test1/test2/test3")); - Path relPath = Paths.get("test1/test2/test3"); - configuration.updateOutputPath(relPath); + configuration.setOutputDir(Paths.get("test1/test2/test3")); - assertEquals(Paths.get("test1/test2/test3"), configuration.getRawOutputPath()); - assertEquals(Paths.get("/test1/test2/test3/test1/test2/test3"), configuration.buildOutputPath()); + assertEquals("test1/test2/test3", configuration.encodedDestDir()); + assertEquals(Paths.get("/test1/test2/test3/test1/test2/test3"), configuration.requireOutputDirectory()); } @Test public void emptyRootNotEmptyRelPath() { - Path relPath = Paths.get("/"); - assertThrows(ValidationException.class, () -> { - configuration.setRootPath(Paths.get("")); - configuration.updateOutputPath(relPath); - }); + assertThrows(ValidationException.class, () -> configuration.setRootPath(Paths.get(""))); } @Test public void notEmptyRootEmptyRelPath() { configuration.setRootPath(Paths.get("/")); - configuration.updateOutputPath(Paths.get("")); + configuration.setOutputDir(Paths.get("")); - assertEquals(Paths.get(""), configuration.getRawOutputPath()); - assertEquals(Paths.get("/"), configuration.buildOutputPath()); + assertEquals(".", configuration.encodedDestDir()); + assertEquals(Paths.get("/"), configuration.requireOutputDirectory()); } @Test public void invalidRootPath() { - assertThrows(ValidationException.class, () -> { - configuration.setRootPath(Paths.get("invalidRoot:/test")); - configuration.updateOutputPath(Paths.get("/test1/test2/test3")); - }); - } - - @Test - public void concatInvalidRootPathAndRelPath() { - assertThrows(ValidationException.class, () -> { - configuration.setRootPath(Paths.get("invalidRoot:/test")); - configuration.updateOutputPath(Paths.get("test1/test2/test3")); - }); + assertThrows(ValidationException.class, () -> configuration.setRootPath(Paths.get("invalidRoot:/test"))); } @Test public void nullRootPath() { - configuration.updateOutputPath(Paths.get("/test1/test2/test3")); - assertEquals(Paths.get("/test1/test2/test3"), configuration.getRawOutputPath()); - assertEquals(Paths.get("/test1/test2/test3"), configuration.buildOutputPath()); + configuration.setOutputDir(Paths.get("/test1/test2/test3")); + + assertEquals("/test1/test2/test3", configuration.encodedDestDir()); + assertEquals(Paths.get("/test1/test2/test3"), configuration.requireOutputDirectory()); } } - } diff --git a/cayenne-cgen/src/test/java/org/apache/cayenne/gen/ClassGenerationActionTest.java b/cayenne-cgen/src/test/java/org/apache/cayenne/gen/ClassGenerationActionTest.java index 7888e693d..dfcb01263 100644 --- a/cayenne-cgen/src/test/java/org/apache/cayenne/gen/ClassGenerationActionTest.java +++ b/cayenne-cgen/src/test/java/org/apache/cayenne/gen/ClassGenerationActionTest.java @@ -242,7 +242,7 @@ public class ClassGenerationActionTest extends CgenCase { TemplateType templateType = TemplateType.DATAMAP_SUPERCLASS; cgenConfiguration.setRootPath(tempFolder.toPath()); - cgenConfiguration.updateOutputPath(Paths.get(".")); + cgenConfiguration.setOutputDir(Paths.get(".")); action = newAction(); ObjEntity testEntity1 = new ObjEntity("TEST"); testEntity1.setClassName("TestClass1"); @@ -273,7 +273,7 @@ public class ClassGenerationActionTest extends CgenCase { TemplateType templateType = TemplateType.DATAMAP_SUPERCLASS; cgenConfiguration.setRootPath(tempFolder.toPath()); - cgenConfiguration.updateOutputPath(Paths.get(".")); + cgenConfiguration.setOutputDir(Paths.get(".")); action = newAction(); action.context.put(Artifact.SUPER_PACKAGE_KEY, ""); action.context.put(Artifact.SUPER_CLASS_KEY, "TestClass1"); @@ -301,7 +301,7 @@ public class ClassGenerationActionTest extends CgenCase { TemplateType templateType = TemplateType.DATAMAP_SINGLE_CLASS; cgenConfiguration.setRootPath(tempFolder.toPath()); - cgenConfiguration.updateOutputPath(Paths.get(".")); + cgenConfiguration.setOutputDir(Paths.get(".")); action = newAction(); ObjEntity testEntity1 = new ObjEntity("TEST"); testEntity1.setClassName("TestClass1"); diff --git a/cayenne-cgen/src/test/java/org/apache/cayenne/gen/xml/CgenSaverDelegateTest.java b/cayenne-cgen/src/test/java/org/apache/cayenne/gen/xml/CgenSaverDelegateTest.java index cc7f8a831..51d1dcc6f 100644 --- a/cayenne-cgen/src/test/java/org/apache/cayenne/gen/xml/CgenSaverDelegateTest.java +++ b/cayenne-cgen/src/test/java/org/apache/cayenne/gen/xml/CgenSaverDelegateTest.java @@ -20,64 +20,86 @@ package org.apache.cayenne.gen.xml; import java.net.URL; +import java.nio.file.Path; import java.nio.file.Paths; import org.apache.cayenne.gen.CgenConfiguration; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; public class CgenSaverDelegateTest { + private static URL baseURL(String path) throws Exception { + return Paths.get(path).toAbsolutePath().toUri().toURL(); + } + + private static Path absolute(String path) { + return Paths.get(path).toAbsolutePath(); + } + + /** + * A configuration anchored somewhere else but with no output directory of its own: rebasing keeps it + * generating into the directory it already resolved to, {@code /tmp/src/main/java}. It used to silently + * jump to the new base instead, which is what the "do we care about this case?" note on this test was + * about; it now behaves the same way as {@link #existingRootAndRelPath()}. + */ @Test public void existingRootOverride() throws Exception { CgenConfiguration config = new CgenConfiguration(); + config.setRootPath(absolute("/tmp/src/main/java")); - config.setRootPath(Paths.get("/tmp/src/main/java").toAbsolutePath()); - URL baseURL = Paths.get("/tmp/src/main/resources").toUri().toURL(); + CgenSaverDelegate.resolveOutputDir(baseURL("/tmp/src/main/resources"), config); - CgenSaverDelegate.resolveOutputDir(baseURL, config); - - assertEquals(Paths.get("/tmp/src/main/resources").toAbsolutePath(), config.getRootPath()); - assertEquals(Paths.get(""), config.getRawOutputPath()); // TODO: do we care about this case? + assertEquals(absolute("/tmp/src/main/resources"), config.getRootPath()); + assertEquals(absolute("/tmp/src/main/java"), config.requireOutputDirectory()); } @Test public void existingRootAndRelPath() throws Exception { CgenConfiguration config = new CgenConfiguration(); + config.setRootPath(absolute("/tmp/src/main/java")); + config.setOutputDir(Paths.get("")); - config.setRootPath(Paths.get("/tmp/src/main/java").toAbsolutePath()); - config.updateOutputPath(Paths.get("")); - - URL baseURL = Paths.get("/tmp/src/main/resources").toUri().toURL(); - - CgenSaverDelegate.resolveOutputDir(baseURL, config); + CgenSaverDelegate.resolveOutputDir(baseURL("/tmp/src/main/resources"), config); - assertEquals(Paths.get("/tmp/src/main/resources").toAbsolutePath(), config.getRootPath()); - assertEquals(Paths.get("../java"), config.getRawOutputPath()); + assertEquals(absolute("/tmp/src/main/resources"), config.getRootPath()); + assertEquals(absolute("/tmp/src/main/java"), config.requireOutputDirectory()); } @Test public void emptyRootInMavenTree() throws Exception { CgenConfiguration config = new CgenConfiguration(); - URL baseURL = Paths.get("/tmp/src/main/resources").toUri().toURL(); + CgenSaverDelegate.resolveOutputDir(baseURL("/tmp/src/main/resources"), config); - CgenSaverDelegate.resolveOutputDir(baseURL, config); - - assertEquals(Paths.get("/tmp/src/main/resources").toAbsolutePath(), config.getRootPath()); - assertEquals(Paths.get("../java"), config.getRawOutputPath()); + assertEquals(absolute("/tmp/src/main/resources"), config.getRootPath()); + assertEquals(absolute("/tmp/src/main/java"), config.requireOutputDirectory()); } @Test public void emptyRoot() throws Exception { CgenConfiguration config = new CgenConfiguration(); - URL baseURL = Paths.get("/tmp/somefolder").toUri().toURL(); + CgenSaverDelegate.resolveOutputDir(baseURL("/tmp/somefolder"), config); + + assertEquals(absolute("/tmp/somefolder"), config.getRootPath()); + assertEquals(absolute("/tmp/somefolder"), config.requireOutputDirectory()); + } + + /** + * An output directory the user picked explicitly stays pointing at the same physical directory when the + * project is saved somewhere else. + */ + @Test + public void absoluteOutputDirSurvivesRebase() throws Exception { + CgenConfiguration config = new CgenConfiguration(); + config.setRootPath(absolute("/tmp/project")); + config.setOutputDir(absolute("/tmp/generated")); - CgenSaverDelegate.resolveOutputDir(baseURL, config); + CgenSaverDelegate.resolveOutputDir(baseURL("/tmp/other/project"), config); - assertEquals(Paths.get("/tmp/somefolder").toAbsolutePath(), config.getRootPath()); - assertEquals(Paths.get(""), config.getRawOutputPath()); + assertEquals(absolute("/tmp/other/project"), config.getRootPath()); + assertEquals(absolute("/tmp/generated"), config.requireOutputDirectory()); } } diff --git a/cayenne-gradle-plugin/src/main/java/org/apache/cayenne/tools/CgenTask.java b/cayenne-gradle-plugin/src/main/java/org/apache/cayenne/tools/CgenTask.java index dc2ddb7f9..9a9b02f58 100644 --- a/cayenne-gradle-plugin/src/main/java/org/apache/cayenne/tools/CgenTask.java +++ b/cayenne-gradle-plugin/src/main/java/org/apache/cayenne/tools/CgenTask.java @@ -343,7 +343,7 @@ public class CgenTask extends BaseCayenneTask { getLogger().info("Using default cgen config."); cgenConfiguration = new CgenConfiguration(); if(getDestDirFile() != null) { - cgenConfiguration.updateOutputPath(getDestDirFile().toPath()); + cgenConfiguration.setOutputDir(getDestDirFile().toPath()); } cgenConfiguration.setDataMap(dataMap); return Collections.singletonList(cgenConfiguration); @@ -354,7 +354,7 @@ public class CgenTask extends BaseCayenneTask { CgenConfiguration cgenConfiguration = new CgenConfiguration(); cgenConfiguration.setDataMap(dataMap); if(getDestDirFile() != null) { - cgenConfiguration.updateOutputPath(getDestDirFile().toPath()); + cgenConfiguration.setOutputDir(getDestDirFile().toPath()); } cgenConfiguration.setEncoding(encoding != null ? encoding : cgenConfiguration.getEncoding()); cgenConfiguration.setMakePairs(makePairs != null ? makePairs : cgenConfiguration.isMakePairs()); diff --git a/cayenne-maven-plugin/src/main/java/org/apache/cayenne/tools/CayenneGeneratorMojo.java b/cayenne-maven-plugin/src/main/java/org/apache/cayenne/tools/CayenneGeneratorMojo.java index 25fb8590b..176d3800d 100644 --- a/cayenne-maven-plugin/src/main/java/org/apache/cayenne/tools/CayenneGeneratorMojo.java +++ b/cayenne-maven-plugin/src/main/java/org/apache/cayenne/tools/CayenneGeneratorMojo.java @@ -343,7 +343,7 @@ public class CayenneGeneratorMojo extends AbstractMojo { LOGGER.info("Using default cgen config."); CgenConfiguration cgenConfiguration = new CgenConfiguration(); cgenConfiguration.setDataMap(dataMap); - cgenConfiguration.updateOutputPath(defaultDir.toPath()); + cgenConfiguration.setOutputDir(defaultDir.toPath()); return Collections.singletonList(cgenConfiguration); } } @@ -351,7 +351,7 @@ public class CayenneGeneratorMojo extends AbstractMojo { private CgenConfiguration cgenConfigFromPom(DataMap dataMap) { CgenConfiguration cgenConfiguration = new CgenConfiguration(); cgenConfiguration.setDataMap(dataMap); - cgenConfiguration.updateOutputPath(destDir != null ? destDir.toPath() : defaultDir.toPath()); + cgenConfiguration.setOutputDir(destDir != null ? destDir.toPath() : defaultDir.toPath()); cgenConfiguration.setEncoding(encoding != null ? encoding : cgenConfiguration.getEncoding()); cgenConfiguration.setMakePairs(makePairs != null ? makePairs : cgenConfiguration.isMakePairs()); if (mode != null && mode.equals("datamap")) { diff --git a/cayenne-mcp-server/src/main/java/org/apache/cayenne/mcp/tools/cgen/CgenRunTool.java b/cayenne-mcp-server/src/main/java/org/apache/cayenne/mcp/tools/cgen/CgenRunTool.java index a2a8a9b40..a35e9eeb4 100644 --- a/cayenne-mcp-server/src/main/java/org/apache/cayenne/mcp/tools/cgen/CgenRunTool.java +++ b/cayenne-mcp-server/src/main/java/org/apache/cayenne/mcp/tools/cgen/CgenRunTool.java @@ -50,6 +50,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; /** @@ -164,16 +165,17 @@ public class CgenRunTool { CgenConfigList configList = metaData.get(dataMap, CgenConfigList.class); boolean usedDefaultConfig = configList == null || configList.getAll().isEmpty(); CgenConfiguration cgenConfig = usedDefaultConfig - ? CgenConfiguration.createDefault(dataMap, CgenConfiguration.defaultOutputDir(dataMap)) + ? CgenConfiguration.createDefault(dataMap, null) : configList.getAll().getFirst(); // Step 5 — destDir specified? - Path destDir = cgenConfig.buildOutputPath(); - if (destDir == null) { + Optional<Path> resolvedDestDir = cgenConfig.outputDirectory(); + if (resolvedDestDir.isEmpty()) { return validationFailed(CgenErrorCode.destdir_not_specified, "cgen configuration for '" + dataMapName + "' does not specify a destination directory.", new CgenValidation(true, true, true, false, null)); } + Path destDir = resolvedDestDir.get(); // Step 6 — destDir writable (create if absent)? if (!ensureWritable(destDir)) { diff --git a/cayenne-mcp-server/src/test/java/org/apache/cayenne/mcp/tools/cgen/CgenRunValidationTest.java b/cayenne-mcp-server/src/test/java/org/apache/cayenne/mcp/tools/cgen/CgenRunValidationTest.java index 32318b67a..6063a2dce 100644 --- a/cayenne-mcp-server/src/test/java/org/apache/cayenne/mcp/tools/cgen/CgenRunValidationTest.java +++ b/cayenne-mcp-server/src/test/java/org/apache/cayenne/mcp/tools/cgen/CgenRunValidationTest.java @@ -25,10 +25,12 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import java.io.IOException; +import java.net.URI; import java.net.URISyntaxException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.*; @@ -96,20 +98,38 @@ public class CgenRunValidationTest { assertNull(result.validation().destDirWritable()); } + /** + * An absent {@code <destDir>} means the same thing as the {@code <destDir>.</destDir>} the Cayenne + * encoder writes when no output directory is configured: generate next to the DataMap. It used to be + * a validation failure, which made a saved project behave differently depending on whether the tag + * had ever been written out. + */ @Test - public void destDirNotSpecified() throws URISyntaxException { - String projectPath = fixtureProject("no-destdir"); + public void destDirDefaultsToTheDataMapDirectory(@TempDir Path tempDir) throws IOException { + Path projectFile = copyFixture("no-destdir", tempDir); - CgenRunResult result = tool.run(projectPath, "TestMap"); + CgenRunResult result = tool.run(projectFile.toString(), "TestMap"); - assertEquals("validation_failed", result.status()); - assertEquals(CgenErrorCode.destdir_not_specified, result.error().code()); + assertNull(result.error()); + assertEquals("generated", result.status()); + assertEquals(tempDir.toAbsolutePath().toString(), result.resolved().destDir()); assertTrue(result.validation().projectFound()); assertTrue(result.validation().dataMapFound()); assertTrue(result.validation().cgenConfigPresent()); - assertFalse(result.validation().destDirSpecified()); - assertNull(result.validation().destDirWritable()); + assertTrue(result.validation().destDirSpecified()); + assertTrue(result.validation().destDirWritable()); + } + + private static Path copyFixture(String fixture, Path targetDir) throws IOException { + Path source = Paths.get(URI.create(CgenRunValidationTest.class + .getResource("/cgen-fixtures/" + fixture).toString())); + try (Stream<Path> files = Files.list(source)) { + for (Path file : files.toList()) { + Files.copy(file, targetDir.resolve(file.getFileName().toString())); + } + } + return targetDir.resolve("cayenne-project.xml"); } private static String fixtureProject(String fixture) throws URISyntaxException { diff --git a/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/project/CgenOps.java b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/project/CgenOps.java index 0e85adad2..239e326f3 100644 --- a/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/project/CgenOps.java +++ b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/project/CgenOps.java @@ -31,6 +31,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.prefs.Preferences; +import java.util.Optional; public class CgenOps { @@ -43,13 +44,19 @@ public class CgenOps { return configuration; } - public static Path baseDir(ProjectSession session) { + /** + * Default cgen output directory: the project directory, mapped through the standard Maven layout when it applies. + * Empty for a project that has not been saved yet. + */ + public static Optional<Path> baseDir(ProjectSession session) { Path projectRoot = projectRoot(session); if (projectRoot == null) { - return Paths.get("."); + return Optional.empty(); } - return Utils.getMavenSrcPathForPath(projectRoot).map(Paths::get).orElse(projectRoot); + return Optional.of(Utils.getMavenSrcPathForPath(projectRoot) + .map(Paths::get) + .orElse(projectRoot)); } private static Path projectRoot(ProjectSession session) { diff --git a/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/ui/project/editor/datadomain/cgen/DataDomainCgenTab.java b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/ui/project/editor/datadomain/cgen/DataDomainCgenTab.java index e6f586ebb..369fcca51 100644 --- a/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/ui/project/editor/datadomain/cgen/DataDomainCgenTab.java +++ b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/ui/project/editor/datadomain/cgen/DataDomainCgenTab.java @@ -86,7 +86,8 @@ public class DataDomainCgenTab extends DataDomainGeneratorsTab<CgenConfiguration } public CgenConfiguration createConfiguration(DataMap dataMap) { - CgenConfiguration cgenConfiguration = CgenConfiguration.createDefault(dataMap, CgenOps.baseDir(session)); + CgenConfiguration cgenConfiguration = CgenConfiguration.createDefault(dataMap, + CgenOps.baseDir(session).orElse(null)); cgenConfiguration.setEncoding(new GeneralPrefs(app.getPrefsLocator().appNode(GeneralPrefs.NODE)).getEncoding()); return cgenConfiguration; } diff --git a/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/ui/project/editor/datamap/cgen/CgenConfigPanel.java b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/ui/project/editor/datamap/cgen/CgenConfigPanel.java index def321e3e..21bb53d15 100644 --- a/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/ui/project/editor/datamap/cgen/CgenConfigPanel.java +++ b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/ui/project/editor/datamap/cgen/CgenConfigPanel.java @@ -134,12 +134,13 @@ public class CgenConfigPanel extends ProjectPanel { public void initForm(CgenConfiguration cgenConfiguration) { this.cgenConfiguration = cgenConfiguration; - if (cgenConfiguration.getRootPath() != null) { - outputFolder.setText(cgenConfiguration.buildOutputPath().toString()); + Path outputDir = cgenConfiguration.outputDirectory().orElse(null); + if (outputDir != null) { + outputFolder.setText(outputDir.toString()); applyOutputFolder(outputFolder.getText()); } else { - // unsaved project: no project root to resolve a relative output path against, - // so leave the field empty rather than running validation on a stale value + // unsaved project: no project root to resolve the output path against, so there is nothing + // to show. Leave the field empty rather than running validation on a stale value. outputFolder.setText(""); } if (cgenConfiguration.getArtifactsGenerationMode().equalsIgnoreCase("all")) { @@ -352,7 +353,7 @@ public class CgenConfigPanel extends ProjectPanel { updateGenerateButton(false); throw new ValidationException(NEED_TO_SAVE_PROJECT_MSG); } - cgenConfiguration.updateOutputPath(path); + cgenConfiguration.setOutputDir(path); updateGenerateButton(true); cgen.checkCgenConfigDirty(); } @@ -399,7 +400,7 @@ public class CgenConfigPanel extends ProjectPanel { String currentDir = outputFolder.getText(); File initialDir = !Util.isEmptyString(currentDir) ? new File(currentDir) - : CgenOps.baseDir(session).toFile(); + : CgenOps.baseDir(session).map(Path::toFile).orElse(null); File selected = app.getFileChooser(this, "Select Output Folder").openDir(initialDir); if (selected != null) { diff --git a/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/ui/project/editor/datamap/cgen/CgenPanel.java b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/ui/project/editor/datamap/cgen/CgenPanel.java index acc4a40de..8ea669eaf 100644 --- a/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/ui/project/editor/datamap/cgen/CgenPanel.java +++ b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/ui/project/editor/datamap/cgen/CgenPanel.java @@ -535,7 +535,7 @@ public class CgenPanel extends ProjectPanel implements ObjEntityListener, Embedd private void onProjectSaved(ProjectAfterSaveEvent e) { if (cgenConfigPanel != null && configuration != null) { cgenConfigPanel.getOutputFolder() - .setText(configuration.buildOutputPath().toString()); + .setText(configuration.outputDirectory().map(Path::toString).orElse("")); } }
