mattcasters commented on code in PR #8583:
URL: https://github.com/apache/hop/pull/8583#discussion_r4106413822
##########
plugins/misc/projects/src/main/java/org/apache/hop/projects/project/ProjectDialog.java:
##########
@@ -894,15 +904,15 @@ private void ok() {
int anwser = box.open();
if ((anwser & SWT.NO) != 0) {
wName.setText(oriProjectName);
+ projectName = oriProjectName;
}
}
- if (!oriProjectName.equals(projectName)) {
- List<String> refs =
ProjectsUtil.getParentProjectReferences(oriProjectName);
-
- if (!refs.isEmpty()) {
- ProjectsUtil.changeParentProjectReferences(oriProjectName,
projectName);
- }
+ if (this.editMode
+ && StringUtils.isNotEmpty(oriProjectName)
+ && !oriProjectName.equals(projectName)) {
+ ProjectsUtil.changeParentProjectReferences(oriProjectName,
projectName);
Review Comment:
**[bug]** Child project files and the default/standard parent pointers are
updated before the rename is validated or saved.
`changeParentProjectReferences` writes every child's `project-config.json`
immediately, and `renameProjectReferences` retargets `defaultProject` /
`standardParentProject` on the shared config. Both run before `getInfo` (line
918). `getInfo` still throws: `verifyProjectsChain` rejects a parent loop
longer than the one-level check above, and `setConfigFilename` rethrows
anything that is not a missing home folder. `ok()` then shows the error and
leaves `returnValue` null, so `editProject()` never reaches
`HopConfig.saveToFile()`. A later `saveToFile()` failure inside the child loop
does the same for the children already written.
After cancel, or a crash before hop-config is saved, those children name a
project that is not registered. The in-memory default project is also already
moved, so the next unrelated `saveConfig()` persists that pointer anyway.
`SelectProjectsDialog.editSelected` (line 355) has the same order: it rewrites
children, then `updateProjectConfig`, then `saveConfig()`.
**Suggestion:** Keep the captured old/new names, run `getInfo` and the
rename confirmation first, and only then update references. In
`changeParentProjectReferences`, decide which children can be saved before
writing any file. Save the renamed hop-config registration before rewriting
children (or roll those files back if that save throws).
##########
plugins/misc/projects/src/main/java/org/apache/hop/projects/config/ProjectsConfig.java:
##########
@@ -168,15 +169,50 @@ public int indexOfProjectConfig(String projectName) {
new ProjectConfig(projectName, null, null)); // Only considers the name
}
+ /**
+ * Remove a project registration. The default project and standard parent
project settings are
+ * cleared when they point to the removed project, so they never name a
project that doesn't
+ * exist.
+ *
+ * @param projectName the name of the project to remove
+ * @return the removed project registration or null if it wasn't found
+ */
public ProjectConfig removeProjectConfig(String projectName) {
int index = indexOfProjectConfig(projectName);
if (index >= 0) {
+ renameProjectReferences(projectName, null);
return projectConfigurations.remove(index);
} else {
return null;
}
}
+ /**
+ * Point the default project and standard parent project settings to a
renamed project.
+ *
+ * @param oldName the previous name of the project
+ * @param newName the new name of the project, null to clear the settings
+ */
+ public void renameProjectReferences(String oldName, String newName) {
+ if (StringUtils.isEmpty(oldName)) {
+ return;
+ }
+ if (oldName.equalsIgnoreCase(defaultProject)) {
Review Comment:
**[suggestion]** This only retargets `defaultProject` and
`standardParentProject`. `LifecycleEnvironment.projectName` is the other
persisted project reference on this object, and both rename paths
(`ProjectDialog.ok` and `updateProjectConfig`) now go through this method.
The environment menu keeps an entry only when
`currentProjectName.equals(environment.getProjectName())`
(`ProjectsGuiPlugin.createEnvironmentContextMenu`), and
`findEnvironmentsOfProject` uses that same case-sensitive `equals`. After a
rename, including a case-only rename, those environments no longer match and
disappear from the toolbar even though they are still in hop-config.
**Suggestion:** On rename, set `environment.setProjectName(newName)` when
the old name matches, ignoring case. Do not set the name to null on delete: an
empty project name is treated as global and the environment would show up for
every project. Cover it next to
`renamingRegistrationInPlaceRenamesDefaultAndStandardParentProject`.
##########
plugins/misc/projects/src/main/java/org/apache/hop/projects/util/ProjectsUtil.java:
##########
@@ -338,79 +337,108 @@ public static void validateFileInProject(
* @return
*/
public static boolean projectExists(String projectName) {
-
- boolean prjFound = false;
-
- ProjectsConfig config = ProjectsConfigSingleton.getConfig();
- List<String> prjs = config.listProjectConfigNames();
- Iterator<String> iPrj = prjs.iterator();
-
- while (!prjFound && iPrj.hasNext()) {
- String p = iPrj.next();
- prjFound = p.equals(projectName);
- }
-
- return prjFound;
+ return ProjectsConfigSingleton.getConfig().findProjectConfig(projectName)
!= null;
}
+ /**
+ * Find the registered projects which have the given project as their parent
project.
+ *
+ * @param projectName the name of the parent project
+ * @return the names of the child projects
+ */
public static List<String> getParentProjectReferences(String projectName)
throws HopException {
+ HopGui hopGui = HopGui.getInstance();
+ return getParentProjectReferences(projectName, hopGui.getVariables(),
hopGui.getLog());
+ }
+ /**
+ * Find the registered projects which have the given project as their parent
project. Projects
+ * which can't be loaded are logged and skipped.
+ *
+ * @param projectName the name of the parent project
+ * @param variables the variables to resolve the project locations with
+ * @param log the log channel to report projects which can't be loaded
+ * @return the names of the child projects
+ */
+ public static List<String> getParentProjectReferences(
+ String projectName, IVariables variables, ILogChannel log) {
+ List<String> references = new ArrayList<>();
+ if (StringUtils.isEmpty(projectName)) {
+ return references;
+ }
ProjectsConfig config = ProjectsConfigSingleton.getConfig();
- List<String> prjs = config.listProjectConfigNames();
-
- HopGui hopGui = HopGui.getInstance();
- List<String> parentProjectReferences = new ArrayList<>();
- ProjectConfig currentProjectConfig = config.findProjectConfig(projectName);
-
- if (currentProjectConfig == null) {
- parentProjectReferences = List.of();
- } else {
- for (String prj : prjs) {
- if (!prj.equals(projectName)) {
- ProjectConfig prjCfg = config.findProjectConfig(prj);
- Project thePrj = prjCfg.loadProject(hopGui.getVariables());
- if (thePrj != null) {
- if (thePrj.getParentProjectName() != null
- && thePrj.getParentProjectName().equals(projectName)) {
- parentProjectReferences.add(prj);
- }
- } else {
- hopGui.getLog().logError("Unable to load project '" + prj + "'
from its configuration");
- }
- }
+ for (String name : config.listProjectConfigNames()) {
+ if (name.equalsIgnoreCase(projectName)) {
+ continue;
+ }
+ Project project = loadProject(config.findProjectConfig(name), variables,
log);
+ if (project != null &&
projectName.equalsIgnoreCase(project.getParentProjectName())) {
+ references.add(name);
}
}
- return parentProjectReferences;
+ return references;
}
+ /**
+ * Point the child projects of a renamed project to its new name and save
their configuration.
+ * Read-only projects are left alone.
+ *
+ * @param currentName the previous name of the parent project
+ * @param newName the new name of the parent project
+ * @return the names of the child projects which were updated
+ */
public static List<String> changeParentProjectReferences(String currentName,
String newName)
throws HopException {
+ HopGui hopGui = HopGui.getInstance();
+ return changeParentProjectReferences(
+ currentName, newName, hopGui.getVariables(), hopGui.getLog());
+ }
+ /**
+ * Point the child projects of a renamed project to its new name and save
their configuration.
+ * Read-only projects are left alone.
+ *
+ * @param currentName the previous name of the parent project
+ * @param newName the new name of the parent project
+ * @param variables the variables to resolve the project locations with
+ * @param log the log channel to report projects which can't be loaded or
changed
+ * @return the names of the child projects which were updated
+ */
+ public static List<String> changeParentProjectReferences(
+ String currentName, String newName, IVariables variables, ILogChannel
log)
+ throws HopException {
+ List<String> changed = new ArrayList<>();
ProjectsConfig config = ProjectsConfigSingleton.getConfig();
- List<String> prjs = config.listProjectConfigNames();
-
- HopGui hopGui = HopGui.getInstance();
- List<String> parentProjectReferences = new ArrayList<>();
- ProjectConfig currentProjectConfig = config.findProjectConfig(currentName);
-
- if (currentProjectConfig == null) {
- parentProjectReferences = List.of();
- } else {
- for (String prj : prjs) {
- if (!prj.equals(currentName)) {
- ProjectConfig prjCfg = config.findProjectConfig(prj);
- Project thePrj = prjCfg.loadProject(hopGui.getVariables());
- if (thePrj != null) {
- if (thePrj.getParentProjectName() != null
- && thePrj.getParentProjectName().equals(currentName)) {
- thePrj.setParentProjectName(newName);
- }
- } else {
- hopGui.getLog().logError("Unable to load project '" + prj + "'
from its configuration");
- }
- }
+ for (String name : getParentProjectReferences(currentName, variables,
log)) {
+ ProjectConfig projectConfig = config.findProjectConfig(name);
+ if (projectConfig.isReadOnly()) {
+ log.logError(
+ "Project '"
+ + name
+ + "' is read-only, its parent project '"
+ + currentName
+ + "' can't be changed to '"
+ + newName
+ + "'");
+ continue;
Review Comment:
**[suggestion]** A read-only child is skipped with a log line, then the
rename still commits. Both callers ignore the returned list (`ProjectDialog.ok`
and `SelectProjectsDialog.editSelected`), so the parent project is renamed
while that child keeps the old name. Delete refuses this case; rename does not.
Writable siblings listed earlier may already have been saved before this
`continue`.
Archive homes are also not treated as read-only here unless the flag is set.
`ProjectDialog` uses `ProjectConfig.isArchiveUri` as well, and a save into an
archive throws after those earlier writes.
**Suggestion:** Preflight every referenced project (read-only flag, archive
URI, and a successful load) and throw `HopException` naming the ones that
cannot be updated before writing any file. Let the dialog show that error
instead of closing. The read-only test can expect the exception.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
