From 664f37a026967c90a9a8a4ef3c5336ee426f67aa Mon Sep 17 00:00:00 2001 From: Miro Richard Eklund Date: Wed, 25 Jul 2018 15:56:46 +0300 Subject: [PATCH] Updated file importer interface and fixed Spreadsheet import Spreadsheet import through the generic file importer always failed, since the books cannot be imported to Development Project. Now the generic file interface takes this into account and provides the selection as a resource. Currently only ExcelFileImport uses Amend 1: Changed the way the selected resource is found Amend 2: Take into account Tuukka's review comments (1,2,3 and 5) Amend 3 and 4: Changes by Jani and Miro to fix some issues with the interface gitlab #53 gitlab #56 Change-Id: Ibfb9d54d8c36a3dc2aa7da8f1043613159ae8383 --- .../META-INF/MANIFEST.MF | 3 +- .../fileimport/ui/ImportFileHandler.java | 25 +++++++++++--- .../fileimport/FileImportService.java | 34 +++++++++++++++---- .../fileimport/IGenericFileImport.java | 14 ++++++-- .../SimanticsResourceFileImport.java | 10 +++--- .../fileimport/dropins/FileImportDropins.java | 4 +-- .../fileimport/ExcelFileImport.java | 11 ++++-- .../spreadsheet/graph/SheetNode.java | 3 +- .../spreadsheet/graph/SpreadsheetBook.java | 4 ++- 9 files changed, 82 insertions(+), 26 deletions(-) diff --git a/bundles/org.simantics.fileimport.ui/META-INF/MANIFEST.MF b/bundles/org.simantics.fileimport.ui/META-INF/MANIFEST.MF index db1100bc3..55e40bf54 100644 --- a/bundles/org.simantics.fileimport.ui/META-INF/MANIFEST.MF +++ b/bundles/org.simantics.fileimport.ui/META-INF/MANIFEST.MF @@ -10,7 +10,8 @@ Require-Bundle: org.eclipse.ui, org.eclipse.e4.core.di, org.eclipse.e4.ui.services, org.simantics.fileimport;bundle-version="1.0.0", - org.slf4j.api + org.slf4j.api, + org.simantics.db Bundle-RequiredExecutionEnvironment: JavaSE-1.8 Import-Package: javax.inject;version="1.0.0" Bundle-ActivationPolicy: lazy diff --git a/bundles/org.simantics.fileimport.ui/src/org/simantics/fileimport/ui/ImportFileHandler.java b/bundles/org.simantics.fileimport.ui/src/org/simantics/fileimport/ui/ImportFileHandler.java index 21f8d3c4d..fbe3ce724 100644 --- a/bundles/org.simantics.fileimport.ui/src/org/simantics/fileimport/ui/ImportFileHandler.java +++ b/bundles/org.simantics.fileimport.ui/src/org/simantics/fileimport/ui/ImportFileHandler.java @@ -1,4 +1,3 @@ - package org.simantics.fileimport.ui; import java.nio.file.Paths; @@ -8,12 +7,16 @@ import java.util.function.Consumer; import javax.inject.Named; +import org.eclipse.core.runtime.IAdaptable; import org.eclipse.e4.core.di.annotations.CanExecute; import org.eclipse.e4.core.di.annotations.Execute; import org.eclipse.e4.ui.services.IServiceConstants; +import org.eclipse.jface.viewers.ISelection; +import org.eclipse.jface.viewers.StructuredSelection; import org.eclipse.swt.SWT; import org.eclipse.swt.widgets.FileDialog; import org.eclipse.swt.widgets.Shell; +import org.simantics.db.Resource; import org.simantics.fileimport.FileImportService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -28,8 +31,8 @@ public class ImportFileHandler { } @Execute - public void execute(@Named(IServiceConstants.ACTIVE_SHELL) Shell shell) { - + public void execute(@Named(IServiceConstants.ACTIVE_SELECTION) ISelection selection, + @Named(IServiceConstants.ACTIVE_SHELL) Shell shell) { Map extensions = FileImportService.supportedExtensionsWithFilters(); String[] filterExtensions = (String[]) extensions.keySet().toArray(new String[extensions.keySet().size()]); String[] filterNames = (String[]) extensions.values().toArray(new String[extensions.values().size()]); @@ -49,8 +52,20 @@ public class ImportFileHandler { final String fileName = dialog.open(); if (fileName == null) return; - - FileImportService.performFileImport(Paths.get(fileName), Optional.of((Consumer) t -> { + + Resource selectedResource = null; + try { + if(selection instanceof StructuredSelection) { + StructuredSelection structuredSelection = (StructuredSelection)selection; + Object elem = structuredSelection.getFirstElement(); + IAdaptable a = (IAdaptable)elem; + selectedResource = a.getAdapter(Resource.class); + } + } catch(NullPointerException | ClassCastException npe) { + LOGGER.warn("Failed to find selection, passing null to file importer", npe); + } + + FileImportService.performFileImport(Paths.get(fileName), Optional.of(selectedResource), Optional.of((Consumer) t -> { LOGGER.error("Could not import file " + fileName, t); })); } diff --git a/bundles/org.simantics.fileimport/src/org/simantics/fileimport/FileImportService.java b/bundles/org.simantics.fileimport/src/org/simantics/fileimport/FileImportService.java index 962761542..5f9c7257b 100644 --- a/bundles/org.simantics.fileimport/src/org/simantics/fileimport/FileImportService.java +++ b/bundles/org.simantics.fileimport/src/org/simantics/fileimport/FileImportService.java @@ -93,27 +93,35 @@ public class FileImportService { Files.write(file, bytes); ConsumerHolder holder = new ConsumerHolder(); - String result = performFileImport(file, Optional.of(holder)); + String result = performFileImport(file, Optional.empty(), Optional.of(holder)); if (holder.getThrowable() != null) throw holder.getThrowable(); return result; } - + /** * Method that performs the import of the given file. This method is called when e.g. {@link FileImportDropins} watcher detects {@link java.nio.file.StandardWatchEventKinds.ENTRY_CREATE} operation * * @param file Path file to be imported + * @param possibleSelection - the selected resource (if exists) * @param callback Optional callback which can be used to catch Throwables thrown in the import process */ - public static String performFileImport(Path file, Optional> callback) { + public static String performFileImport(Path file, Optional possibleSelection, Optional> callback) { if (file.getFileName().toString().equals(DB_FILE)) { return null; } String result = "Import failed"; IGenericFileImport service = findServiceForFileExtension(file); + if (service != null) { try { - Optional resource = service.perform(file); + Optional resource; + if (possibleSelection.isPresent() && service.defaultParentResource() == null) { + resource = Optional.of(Long.toString(service.perform(possibleSelection.get(), file).get().getResourceId())); + } + else { + resource = service.performWithDefaultParent(file); + } saveResourceForPath(file, resource); result = resource.get(); } catch (Throwable t) { @@ -130,7 +138,6 @@ public class FileImportService { } return result; } - /** * Remove the entity that matches the file. This method is called when e.g. the {@link FileImportDropins} watcher detects {@link java.nio.file.StandardWatchEventKinds.ENTRY_DELETE} operation @@ -353,12 +360,27 @@ public class FileImportService { return Optional.of(value); } + /** + * Calls the proper imported without a selection (null possibleSelection) + * @param path + * @param extension + * @return + * @throws Exception + */ public static String importGenericFileWithExtension(String path, String extension) throws Exception { IGenericFileImport service = findServiceForExtension(extension); - Optional result = service.perform(Paths.get(path)); + Optional result = service.performWithDefaultParent(Paths.get(path)); return result.get(); } + /** + * Calls the proper imported without a selection (null possibleSelection) + * @param parent + * @param path + * @param extension + * @return + * @throws Exception + */ public static Resource importGenericFileWithExtensionAndParent(Resource parent, String path, String extension) throws Exception { IGenericFileImport service = findServiceForExtension(extension); Optional result = service.perform(parent, Paths.get(path)); diff --git a/bundles/org.simantics.fileimport/src/org/simantics/fileimport/IGenericFileImport.java b/bundles/org.simantics.fileimport/src/org/simantics/fileimport/IGenericFileImport.java index a8bb132c9..16df3ab45 100644 --- a/bundles/org.simantics.fileimport/src/org/simantics/fileimport/IGenericFileImport.java +++ b/bundles/org.simantics.fileimport/src/org/simantics/fileimport/IGenericFileImport.java @@ -18,14 +18,15 @@ import org.simantics.db.Resource; public interface IGenericFileImport { /** - * Performs the import for the given file + * Performs the import for the given file, using the default parent defined in the file-specific import implementation + * Calls perform(parent, file) with the defaultParent it finds * * @param file Path to file to import * @return Optional string which will be the identifier of the imported file which can later be used for removing the imported entity * @throws Exception */ - Optional perform(Path file) throws Exception; - + Optional performWithDefaultParent(Path file) throws Exception; + Optional perform(Resource parent, Path file) throws Exception; /** @@ -42,4 +43,11 @@ public interface IGenericFileImport { */ Map allowedExtensionsWithFilters(); + /** + * Choose the default parent resource in the specific implementations of the importer + * Can be null, in which case the default parent must always be provided from a selection UI or explicitly defined + * @return + */ + public abstract Resource defaultParentResource(); + } diff --git a/bundles/org.simantics.fileimport/src/org/simantics/fileimport/SimanticsResourceFileImport.java b/bundles/org.simantics.fileimport/src/org/simantics/fileimport/SimanticsResourceFileImport.java index 4d2bcbe69..67ff51dab 100644 --- a/bundles/org.simantics.fileimport/src/org/simantics/fileimport/SimanticsResourceFileImport.java +++ b/bundles/org.simantics.fileimport/src/org/simantics/fileimport/SimanticsResourceFileImport.java @@ -29,7 +29,7 @@ import org.simantics.layer0.Layer0; public abstract class SimanticsResourceFileImport implements IGenericFileImport { @Override - final public Optional perform(Path file) throws Exception { + final public Optional performWithDefaultParent(Path file) throws Exception { Path dropins = Activator.getDropinsFolder(); @@ -40,9 +40,13 @@ public abstract class SimanticsResourceFileImport implements IGenericFileImport parts = file.getFileName(); } - Resource parent = resolveParent(null, parts); + Resource parent = defaultParentResource(); + if(parent == null) + parent = resolveParent(null, parts); + if (parent == null) return Optional.empty(); + Optional imported = perform(parent, file); if (imported.isPresent()) { return Optional.of(serialize(imported.get())); @@ -128,7 +132,5 @@ public abstract class SimanticsResourceFileImport implements IGenericFileImport return null; } } - - public abstract Resource defaultParentResource(); } diff --git a/bundles/org.simantics.fileimport/src/org/simantics/fileimport/dropins/FileImportDropins.java b/bundles/org.simantics.fileimport/src/org/simantics/fileimport/dropins/FileImportDropins.java index a2632362b..fb01e42e5 100644 --- a/bundles/org.simantics.fileimport/src/org/simantics/fileimport/dropins/FileImportDropins.java +++ b/bundles/org.simantics.fileimport/src/org/simantics/fileimport/dropins/FileImportDropins.java @@ -141,14 +141,14 @@ public class FileImportDropins { current++; } - FileImportService.performFileImport(newPath, Optional.of(t -> { + FileImportService.performFileImport(newPath, Optional.empty(), Optional.of(t -> { if ((t instanceof FileSystemException) || (t instanceof FileNotFoundException)) { try { syncPath(newPath); } catch (IOException e) { e.printStackTrace(); } - FileImportService.performFileImport(newPath, Optional.empty()); + FileImportService.performFileImport(newPath, Optional.empty(), Optional.empty()); } else { t.printStackTrace(); } diff --git a/bundles/org.simantics.spreadsheet.fileimport/src/org/simantics/spreadsheet/fileimport/ExcelFileImport.java b/bundles/org.simantics.spreadsheet.fileimport/src/org/simantics/spreadsheet/fileimport/ExcelFileImport.java index c3a84f7a4..d1f8924a5 100644 --- a/bundles/org.simantics.spreadsheet.fileimport/src/org/simantics/spreadsheet/fileimport/ExcelFileImport.java +++ b/bundles/org.simantics.spreadsheet.fileimport/src/org/simantics/spreadsheet/fileimport/ExcelFileImport.java @@ -20,8 +20,13 @@ public class ExcelFileImport extends SimanticsResourceFileImport { } @Override - public Optional perform(Resource parent, Path file) throws Exception { - return Optional.ofNullable(ExcelImport.importBookR(parent, file.toFile())); + public Optional perform(Resource possibleSelection, Path file) throws Exception { + if(possibleSelection != null) { + //Make sure the selection is of valid type here + return Optional.ofNullable(ExcelImport.importBookR(possibleSelection, file.toFile())); + } else { + throw new NullPointerException("No selection provided - Cannot import book"); + } } @Override @@ -31,7 +36,7 @@ public class ExcelFileImport extends SimanticsResourceFileImport { @Override public Resource defaultParentResource() { - return Simantics.getSession().getRootLibrary(); + return null; } } diff --git a/bundles/org.simantics.spreadsheet.graph/src/org/simantics/spreadsheet/graph/SheetNode.java b/bundles/org.simantics.spreadsheet.graph/src/org/simantics/spreadsheet/graph/SheetNode.java index c92807a45..d2ec679b7 100644 --- a/bundles/org.simantics.spreadsheet.graph/src/org/simantics/spreadsheet/graph/SheetNode.java +++ b/bundles/org.simantics.spreadsheet.graph/src/org/simantics/spreadsheet/graph/SheetNode.java @@ -1,8 +1,9 @@ package org.simantics.spreadsheet.graph; +import java.io.Serializable; import java.util.Map; -public interface SheetNode, Property extends SheetNode> { +public interface SheetNode, Property extends SheetNode> extends Serializable { String getName(); Map getChildren(); diff --git a/bundles/org.simantics.spreadsheet.graph/src/org/simantics/spreadsheet/graph/SpreadsheetBook.java b/bundles/org.simantics.spreadsheet.graph/src/org/simantics/spreadsheet/graph/SpreadsheetBook.java index d64d008ed..5a7dcd5ee 100644 --- a/bundles/org.simantics.spreadsheet.graph/src/org/simantics/spreadsheet/graph/SpreadsheetBook.java +++ b/bundles/org.simantics.spreadsheet.graph/src/org/simantics/spreadsheet/graph/SpreadsheetBook.java @@ -42,7 +42,9 @@ public class SpreadsheetBook implements StandardNodeManagerSupport, S private static final long serialVersionUID = 7417208688311691396L; - public Serializable NotAvailableError = new Serializable() {}; + public Serializable NotAvailableError = new Serializable() { + private static final long serialVersionUID = 2535371785498129460L; + }; public Long2ObjectOpenHashMap referenceMap = new Long2ObjectOpenHashMap<>(); -- 2.47.1