-
Notifications
You must be signed in to change notification settings - Fork 13
Description
Hi Amazon Q Team,
I am a software engineer from the ABAP Development Tools team at SAP. I would like to propose an improvement to the updateAdtServer method in the AbapUtil class that would simplify the codebase and improve maintainability.
Current Issue
The current implementation works around Eclipse editor limitations by temporarily opening and closing editors to synchronize file contents with the ABAP ADT server. While functional, this adds unnecessary complexity and overhead.
Proposed Solution
Update the updateAdtServer method to directly modify the workspaceFile using workspaceFile.setContents(), which automatically triggers the necessary save operations to the ABAP ADT server. The validateEdit call via the Eclipse Semantic File System API ensures proper object locking on the ABAP server, preventing concurrent modification conflicts.
Benefits
- Cleaner Code: Removes unnecessary editor manipulation
- Better Performance: Direct file operations are more efficient
- Improved Reliability: Fewer potential failure points
- Proper Locking: Maintains backend locking semantics via
validateEdit
Reference Implementation
public static void updateAdtServer(final String filePath) {
Display.getDefault().asyncExec(() -> {
try {
if (!AbapUtil.isAbapFile(filePath)) {
return;
}
IWorkbenchWindow window = PlatformUI.getWorkbench().getActiveWorkbenchWindow();
if (window == null) {
return;
}
IFile workspaceFile = getWorkspaceFileFromCache(filePath);
if (workspaceFile == null || !workspaceFile.exists()) {
Activator.getLogger()
.info("File not found in workspace, new file may need to be configured with the workspace: "
+ filePath);
MessageDialog.openError(window.getShell(), "File Not Found in Workspace",
"File not found in workspace, new file may need to be configured with the workspace. See file updates at: "
+ filePath);
return;
}
String content = java.nio.file.Files.readString(java.nio.file.Path.of(filePath));
if (validateEdit(workspaceFile)) {
workspaceFile.setContents(new java.io.ByteArrayInputStream(content.getBytes()), true, true,
new NullProgressMonitor());
} else {
MessageDialog.openError(window.getShell(), "File Cannot be Locked",
"File cannot be modified because it is already locked by another user: " + filePath);
}
} catch (Exception e) {
Activator.getLogger().error("Failed to update ABAP ADT editor", e);
}
});
}
private static boolean validateEdit(IFile file) {
final boolean result[] = new boolean[] { false };
try {
final IFile fFile = file;
ResourcesPlugin.getWorkspace().run(new IWorkspaceRunnable() {
@Override
public void run(IProgressMonitor monitor) throws CoreException {
IStatus status = ResourcesPlugin.getWorkspace().validateEdit(new IFile[] { fFile }, null);
if (status != null && status.isOK()) {
result[0] = true;
}
}
}, new NullProgressMonitor());
} catch (CoreException e) {
Activator.getLogger().error(e.getMessage(), e);
}
return result[0];
}Additional Note: ABAP Class Creation Wizard
If you're interested in automatically triggering the ABAP class creation wizard when creating ABAP class files, I've included a code snippet below. However, please note the following considerations:
- An MCP tool might be a better approach since the wizard typically requires additional input (Package Name, Description, Transport Request)
- The current implementation uses reflection to populate the wizard's
classDataobject, which is not ideal - This reflection-based approach may break if ADT changes its internal API
Please treat this as an optional suggestion rather than a recommended implementation.
public static void createAbapClass(String filePath) {
if (filePath.endsWith(".aclass") || filePath.endsWith(".apclass")) {
Display.getDefault().asyncExec(new Runnable() {
@Override
public void run() {
IProject project = getProjectFromCache(filePath);
if (project == null) {
return;
}
String fileName = getFileName(filePath);
if (fileName == null || fileName.isEmpty()) {
return;
}
String CLASS_WIZARD_ID = "com.sap.adt.oo.ui.newWizards.NewClassWizard";
IWorkbench workbench = PlatformUI.getWorkbench();
IWizardDescriptor descriptor = workbench.getNewWizardRegistry().findWizard(CLASS_WIZARD_ID);
try {
IWorkbenchWizard wizard = descriptor.createWizard();
wizard.init(workbench, new StructuredSelection(project));
Object classData = wizard.getClass().getMethod("getClassData").invoke(wizard);
classData.getClass().getMethod("setName", String.class).invoke(classData, fileName);
WizardDialog dialog = new WizardDialog(workbench.getActiveWorkbenchWindow().getShell(), wizard);
dialog.open();
} catch (CoreException | NoSuchMethodException | InvocationTargetException
| IllegalAccessException e) {
Activator.getLogger().error(e.getMessage(), e);
}
}
private String getFileName(String filePath) {
String normalizedPath = filePath.replace("\\", "/").toLowerCase();
int lastSlashIndex = normalizedPath.lastIndexOf("/");
if (lastSlashIndex < 0) {
return null;
}
String fileName = normalizedPath.substring(lastSlashIndex + 1);
int dotIndex = fileName.indexOf(".");
if (dotIndex < 0) {
return null;
}
String name = fileName.substring(0, dotIndex);
return name.toUpperCase(Locale.ENGLISH);
}
});
}
}
private static IProject getProjectFromCache(String cachePath) {
String workspaceRelativePath = AbapUtil.convertCachePathToWorkspaceRelativePath(cachePath);
if (workspaceRelativePath != null) {
for (IProject project : ResourcesPlugin.getWorkspace().getRoot().getProjects()) {
if (project.isOpen()) {
if (StringUtils.startsWithIgnoreCase(workspaceRelativePath, project.getName())) {
return project;
}
}
}
}
return null;
}I believe this change would improve the overall quality of the Amazon Q plugin while maintaining all necessary functionality, particularly the critical backend locking behavior. I'm open to any feedback or questions you may have about this proposal.
Regards,
Tobias Melcher
ABAP Development Tools Team