mattcasters commented on code in PR #8554:
URL: https://github.com/apache/hop/pull/8554#discussion_r4083672154
##########
ui/src/main/java/org/apache/hop/ui/hopgui/perspective/metadata/MetadataPerspective.java:
##########
@@ -2585,14 +2588,31 @@ private static List<JsonMetadataProvider>
getJsonProviders(IHopMetadataProvider
* provider which has it. Returns null if no file was found (or the metadata
isn't file based).
*/
private String findMetadataFilename(String typeKey, String name) {
+ // An object which wasn't saved since its type was renamed still lives in
a legacy key folder.
+ //
+ List<String> typeKeys = List.of(typeKey);
+ try {
+ HopMetadata annotation =
+ hopGui
+ .getMetadataProvider()
+ .getMetadataClassForKey(typeKey)
+ .getAnnotation(HopMetadata.class);
+ if (annotation != null) {
+ typeKeys = HopMetadataUtil.getAllKeys(annotation);
+ }
+ } catch (Exception e) {
+ // An unknown type: only look in the folder named after the key.
+ }
for (JsonMetadataProvider jsonProvider :
getJsonProviders(hopGui.getMetadataProvider())) {
- String filename = jsonProvider.getBaseFolder() + "/" + typeKey + "/" +
name + ".json";
- try {
- if (HopVfs.fileExists(filename)) {
- return filename;
+ for (String key : typeKeys) {
+ String filename = jsonProvider.getBaseFolder() + "/" + key + "/" +
name + ".json";
+ try {
+ if (HopVfs.fileExists(filename)) {
+ return filename;
Review Comment:
**[bug]** This returns the first existing file while walking
`getJsonProviders` front to back, and that list is parent-then-child
(`HOP_METADATA_FOLDER` is built that way in `Project`, and
`getStandardHopMetadataProvider` keeps the order).
`MultiMetadataSerializer.load` walks the same list in reverse, so the child
wins.
Searching legacy keys inside that forward loop changes which file is opened.
A parent that still has `metadata/restconnection/api.json` and a child that has
`metadata/rest-connection/api.json` (or both still under the legacy folder)
will open the parent file, while the tree and `load()` show the child. The same
helper records the filename when `readVirtualPath` fails, so the "unknown
element" details can point at the other project too.
**Suggestion:** Walk the providers from last to first, and within each
provider keep the current key ahead of the legacy keys, matching
`MultiMetadataSerializer.load` and `JsonMetadataSerializer.findFilename`.
##########
core/src/main/java/org/apache/hop/metadata/serializer/json/JsonMetadataSerializer.java:
##########
@@ -255,53 +344,68 @@ public T delete(String name) throws HopException {
throw new HopException("Error: Object '" + name + "' doesn't exist");
}
T t = load(name);
- String filename = calculateFilename(name);
- try {
- boolean deleted = HopVfs.getFileObject(filename).delete();
- if (!deleted) {
- throw new HopException(
- "Error: Object '" + name + "' could not be deleted, filename : " +
filename);
+
+ // The object can have been saved in the base folder while an older copy
is still sitting in a
+ // legacy folder: remove them all or the old copy would come back.
+ //
+ List<String> folders = new ArrayList<>();
+ folders.add(baseFolder);
Review Comment:
**[bug]** Delete removes the current-folder file before the legacy copies.
`save` does the safe thing (write the new file, then delete the old one). Here
a failure on a later folder — the `FileSystemException` path just below
rethrows and stops the loop — leaves only the legacy file. The next
`load`/`findFilename` then treats that stale copy as the object, so a failed
delete silently rolls the element back to the pre-migration content. The
comment on this loop is exactly that the old copy must not come back.
**Suggestion:** Delete the legacy-folder copies first and the base-folder
file last. If a legacy delete fails, the current file is still the one `load`
returns.
##########
ui/src/main/java/org/apache/hop/ui/hopgui/search/HopGuiMetadataSearchable.java:
##########
@@ -62,6 +62,15 @@ public String getType() {
@Override
public String getFilename() {
if (serializer instanceof JsonMetadataSerializer jsonMetadataSerializer) {
+ // Objects of a renamed metadata type can still live in a legacy folder.
+ try {
+ String filename = jsonMetadataSerializer.findFilename(getName());
Review Comment:
**[suggestion]** This `findFilename` call never runs for project search.
`HopGuiSearchLocationIterator` and `ProjectSearchablesIterator` pass
`metadataProvider.getSerializer(...)`, and
`HopMetadataUtil.getStandardHopMetadataProvider` always wraps the folders in a
`MultiMetadataProvider`. That serializer is a `MultiMetadataSerializer`, so
`getFilename()` still returns null and the search location stays the provider
description instead of the file (legacy or current).
**Suggestion:** Resolve the `JsonMetadataProvider` that owns the object
(`getMetadataProviderName()`, child provider first) and call `findFilename` on
its serializer.
--
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]
