This is an automated email from the ASF dual-hosted git repository.

ntimofeev pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cayenne.git


The following commit(s) were added to refs/heads/master by this push:
     new 31a5664a3 CAY-2785 Modeler: improve folder selection for cgen  - 
refactor output-dir related methods in cgen
31a5664a3 is described below

commit 31a5664a3b57bce61c21706f08f53962078a2559
Author: Nikita Timofeev <stari...@gmail.com>
AuthorDate: Wed Dec 7 15:14:24 2022 +0300

    CAY-2785 Modeler: improve folder selection for cgen
     - refactor output-dir related methods in cgen
---
 .../org/apache/cayenne/gen/CgenConfiguration.java  | 81 +++++++++++++++-------
 .../org/apache/cayenne/gen/internal/Utils.java     | 14 ++--
 .../apache/cayenne/gen/xml/CgenConfigHandler.java  |  4 ++
 .../apache/cayenne/gen/xml/CgenSaverDelegate.java  | 45 ++++++------
 .../cayenne/gen/xml/CgenSaverDelegateTest.java     |  2 +-
 .../apache/cayenne/tools/CayenneGeneratorMojo.java |  5 +-
 .../editor/cgen/CodeGeneratorController.java       |  2 +-
 .../editor/cgen/GeneratorControllerPanel.java      |  5 +-
 8 files changed, 96 insertions(+), 62 deletions(-)

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 8bdaeb67f..a09bec15a 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
@@ -47,6 +47,18 @@ import org.apache.cayenne.validation.ValidationException;
  */
 public class CgenConfiguration implements Serializable, XMLSerializable {
 
+    /**
+     * Should point to the directory that holds Cayenne project XML.
+     * Could be null in some cases now.
+     */
+    private Path rootProjectPath;
+
+    /**
+     * Target directory for generated classes, relative to the {@code 
rootProjectPath}
+     * (if root path is set, and it's possible to relativize).
+     */
+    private Path cgenOutputRelativePath;
+
     private Collection<Artifact> artifacts;
     private Set<String> entityArtifacts;
     private Collection<String> excludeEntityArtifacts;
@@ -59,9 +71,6 @@ public class CgenConfiguration implements Serializable, 
XMLSerializable {
 
     private ArtifactsGenerationMode artifactsGenerationMode;
     private boolean makePairs;
-
-    private Path rootPath;
-    private Path relPath;
     private boolean overwrite;
     private boolean usePkgPath;
 
@@ -171,70 +180,92 @@ public class CgenConfiguration implements Serializable, 
XMLSerializable {
     }
 
     public Path getRootPath() {
-        return rootPath;
+        return rootProjectPath;
     }
 
     /**
-     * @param rootPath absolute target path for the cgen generation
+     * TODO: this should be used in loadin and sa
+     * @param rootProjectPath root path for the Cayenne project this config 
relates to
      */
-    public void setRootPath(Path rootPath) {
-        if (!Objects.requireNonNull(rootPath).isAbsolute()) {
-            throw new ValidationException("Root path : " + '"' + rootPath + 
'"' + "should be absolute");
+    public void setRootPath(Path rootProjectPath) {
+        if (!Objects.requireNonNull(rootProjectPath).isAbsolute()) {
+            throw new ValidationException("Project root path '%s' should be 
absolute.", rootProjectPath);
         }
-        this.rootPath = rootPath;
+        this.rootProjectPath = rootProjectPath;
     }
 
     /**
-     * Directly set relative (to {@code rootPath}) output directory
+     * Directly set relative (to {@code rootProjectPath}) output directory
+     *
      * @param relPath to set
-     * @since 5.0 renamed from {@code setRelPath()}
+     * @since 5.0 renamed from {@code setRelPath()}*
      */
     public void setRelativePath(Path relPath) {
-        this.relPath = relPath;
+        this.cgenOutputRelativePath = relPath;
     }
 
+    /**
+     * @return cgen output relative path
+     * TODO: used only it tests, maybe should be hidden completely
+     */
     public Path getRelPath() {
-        return relPath;
+        return cgenOutputRelativePath;
     }
 
     /**
+     * TODO: used only by TextInput in the Cgen UI, review this
+     *
      * @param pathStr to update relative path with
      * @since 5.0 renamed from {@code setRelPath()}
      */
     public void updateRelativeOutputPath(String pathStr) {
-        Path path = Paths.get(pathStr);
-        if (rootPath != null) {
-            if (path.isAbsolute() && 
rootPath.getRoot().equals(path.getRoot())) {
-                this.relPath = rootPath.relativize(path);
+        updateRelativeOutputPath(Paths.get(pathStr));
+    }
+
+    /**
+     * @param path  to update relative path with
+     * @since 5.0
+     */
+    public void updateRelativeOutputPath(Path path) {
+        if (rootProjectPath != null) {
+            if (path.isAbsolute() && 
rootProjectPath.getRoot().equals(path.getRoot())) {
+                this.cgenOutputRelativePath = rootProjectPath.relativize(path);
                 return;
             }
         }
-        this.relPath = path;
+        this.cgenOutputRelativePath = path;
     }
 
     /**
+     * TODO: used for the XML serialization, could be changed
      * @return normalized relative path
      * @since 5.0 renamed from {@code buildRelPath()} and made package private
      */
     String getNormalizedRelativePath() {
-        if (relPath == null || relPath.toString().isEmpty()) {
+        if (cgenOutputRelativePath == null || 
cgenOutputRelativePath.toString().isEmpty()) {
             return ".";
         }
-        return relPath.toString();
+        return cgenOutputRelativePath.toString();
     }
 
     /**
+     * TODO: change return type to Optional&lt;Path&gt;
      * @return calculated output directory
      * @since 5.0 renamed from {@code buildPath()}
      */
     public Path buildOutputPath() {
-        if (rootPath == null) {
-            return relPath;
+        if (rootProjectPath == null) {
+            return cgenOutputRelativePath;
+        }
+        if(cgenOutputRelativePath == null) {
+            return null;
         }
-        if (relPath == null) {
-            return rootPath;
+
+        if(cgenOutputRelativePath.isAbsolute()) {
+            return cgenOutputRelativePath.normalize();
+        } else {
+            return 
rootProjectPath.resolve(cgenOutputRelativePath).toAbsolutePath().normalize();
         }
-        return rootPath.resolve(relPath).toAbsolutePath().normalize();
     }
 
     public boolean isOverwrite() {
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 8901a1198..ef4164902 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
@@ -20,6 +20,7 @@
 package org.apache.cayenne.gen.internal;
 
 import java.io.File;
+import java.nio.file.Path;
 import java.util.Optional;
 
 /**
@@ -29,6 +30,10 @@ import java.util.Optional;
  */
 public class Utils {
 
+    public static Optional<String> getMavenSrcPathForPath(Path path) {
+        return getMavenSrcPathForPath(path.toAbsolutePath().toString());
+    }
+
     /**
      * @param path to check
      * @return path in the maven src layout or {@code Optional.empty()} if we 
are not inside maven project structure
@@ -59,13 +64,6 @@ public class Utils {
     }
 
     private static String buildFilePath(String... pathElements) {
-        if (pathElements.length == 0) {
-            return "";
-        }
-        StringBuilder path = new StringBuilder(pathElements[0]);
-        for (int i = 1; i < pathElements.length; i++) {
-            path.append(File.separator).append(pathElements[i]);
-        }
-        return path.toString();
+        return String.join(File.separator, pathElements);
     }
 }
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 038bb6f4f..50f8c7907 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
@@ -307,6 +307,10 @@ public class CgenConfigHandler extends 
NamespaceAwareNestedTagHandler {
         });
     }
 
+    /**
+     * @param dataMap loaded cgen config related to
+     * @return base path to the Cayenne project
+     */
     private Path buildRootPath(DataMap dataMap) {
         URL url = dataMap.getConfigurationSource().getURL();
         Path resourcePath;
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 3f40e0658..df532360a 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
@@ -62,6 +62,30 @@ public class CgenSaverDelegate extends BaseSaverDelegate {
             return;
         }
 
+        Path baseDirectory = getBaseDirectoryForURL(baseURL);
+        Path prevRootPath = cgenConfiguration.getRootPath();
+        Path prevOutputPath = cgenConfiguration.buildOutputPath();
+        // Update cgen root path.
+        cgenConfiguration.setRootPath(baseDirectory);
+
+        // If no root path was set, try to calculate if we are inside Maven 
tree structure and use it
+        if(prevRootPath == null) {
+            Utils.getMavenSrcPathForPath(baseDirectory)
+                    .map(Path::of)
+                    .ifPresent(cgenConfiguration::updateRelativeOutputPath);
+        }
+
+        if(prevOutputPath != null) {
+            // Update relative path to match with the new root
+            cgenConfiguration.updateRelativeOutputPath(prevOutputPath);
+        } else if(cgenConfiguration.getRelPath() == 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.updateRelativeOutputPath(baseDirectory);
+        }
+    }
+
+    private static Path getBaseDirectoryForURL(URL baseURL) {
         Path resourcePath;
         try {
             resourcePath = Paths.get(baseURL.toURI());
@@ -71,25 +95,6 @@ public class CgenSaverDelegate extends BaseSaverDelegate {
         if(Files.isRegularFile(resourcePath)) {
             resourcePath = resourcePath.getParent();
         }
-
-        Path oldRoot = cgenConfiguration.getRootPath();
-        if(oldRoot == null) {
-            Path cgenPath = 
Utils.getMavenSrcPathForPath(resourcePath.toString())
-                    .map(Path::of)
-                    .orElse(resourcePath);
-            cgenConfiguration.setRootPath(cgenPath);
-        }
-
-        Path prevPath = cgenConfiguration.buildOutputPath();
-        if(prevPath != null) {
-            if(prevPath.isAbsolute()) {
-                Path relPath = prevPath;
-                if (resourcePath.getRoot().equals(prevPath.getRoot())) {
-                    relPath = resourcePath.relativize(prevPath).normalize();
-                }
-                cgenConfiguration.setRelativePath(relPath);
-            }
-        }
-        cgenConfiguration.setRootPath(resourcePath);
+        return resourcePath;
     }
 }
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 12780ebf3..eb2ba18ac 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
@@ -42,7 +42,7 @@ public class CgenSaverDelegateTest {
         CgenSaverDelegate.resolveOutputDir(baseURL, config);
 
         assertEquals(Paths.get("/tmp/src/main/resources").toAbsolutePath(), 
config.getRootPath());
-        assertEquals(Paths.get("../java"), config.getRelPath());
+        assertEquals(Paths.get(""), config.getRelPath()); // TODO: do we care 
about this case?
     }
 
     @Test
diff --git 
a/maven-plugins/cayenne-maven-plugin/src/main/java/org/apache/cayenne/tools/CayenneGeneratorMojo.java
 
b/maven-plugins/cayenne-maven-plugin/src/main/java/org/apache/cayenne/tools/CayenneGeneratorMojo.java
index 305b1bc47..201876b29 100644
--- 
a/maven-plugins/cayenne-maven-plugin/src/main/java/org/apache/cayenne/tools/CayenneGeneratorMojo.java
+++ 
b/maven-plugins/cayenne-maven-plugin/src/main/java/org/apache/cayenne/tools/CayenneGeneratorMojo.java
@@ -348,7 +348,7 @@ public class CayenneGeneratorMojo extends AbstractMojo {
             logger.info("Using default cgen config.");
             CgenConfiguration cgenConfiguration = new CgenConfiguration();
             cgenConfiguration.setDataMap(dataMap);
-            cgenConfiguration.updateRelativeOutputPath(defaultDir.getPath());
+            cgenConfiguration.setRelativePath(defaultDir.toPath());
             return Collections.singletonList(cgenConfiguration);
         }
     }
@@ -356,8 +356,7 @@ public class CayenneGeneratorMojo extends AbstractMojo {
     private CgenConfiguration cgenConfigFromPom(DataMap dataMap) {
         CgenConfiguration cgenConfiguration = new CgenConfiguration();
         cgenConfiguration.setDataMap(dataMap);
-        // TODO: check this call, CayenneGeneratorTask and CgenTask use 
setRelPath()
-        cgenConfiguration.updateRelativeOutputPath(destDir != null ? 
destDir.getPath() : defaultDir.getPath());
+        cgenConfiguration.setRelativePath(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/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/editor/cgen/CodeGeneratorController.java
 
b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/editor/cgen/CodeGeneratorController.java
index b4f14d2d1..ae4e32e10 100644
--- 
a/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/editor/cgen/CodeGeneratorController.java
+++ 
b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/editor/cgen/CodeGeneratorController.java
@@ -534,7 +534,7 @@ public class CodeGeneratorController extends 
CayenneController implements ObjEnt
         if(getStandardModeController() != null
                 && getStandardModeController().getView() != null
                 && cgenConfiguration != null) {
-            
getStandardModeController().getView().getOutputFolder().setText(cgenConfiguration.getRootPath().toString());
+            
getStandardModeController().getView().getOutputFolder().setText(cgenConfiguration.buildOutputPath().toString());
         }
     }
 
diff --git 
a/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/editor/cgen/GeneratorControllerPanel.java
 
b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/editor/cgen/GeneratorControllerPanel.java
index ccf8d1ee8..d10b95995 100644
--- 
a/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/editor/cgen/GeneratorControllerPanel.java
+++ 
b/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/editor/cgen/GeneratorControllerPanel.java
@@ -46,13 +46,10 @@ public class GeneratorControllerPanel extends JPanel {
         this.outputFolder = new TextAdapter(new JTextField()) {
             @Override
             protected void updateModel(String text) throws ValidationException 
{
-
                 CgenConfiguration cgenByDataMap = getCgenConfig();
-
                 if (cgenByDataMap != null) {
-
                     if (cgenByDataMap.getRootPath() == null && 
!Paths.get(text).isAbsolute()) {
-                        throw new ValidationException("You should save project 
to use rel path as output directory ");
+                        throw new ValidationException("You should save project 
to use relative path as an output directory.");
                     }
                     cgenByDataMap.updateRelativeOutputPath(text);
                     checkConfigDirty();

Reply via email to