Added Quality of Life features to assume role policies. Added Find and Replace, Undo & Redo, and line wrapping for the Session Policy text field.

[JAEKts]

No description provided.

[jakekarnes42]

Thanks for the PR! Please review the comments provide and let me know if you have any questions.

[jakekarnes42]

I'm a little wary that we're adding these listeners every time it gains focus. If a user clicks in and out, I think that'll add multiple listeners to the component, and I'm not sure if they would conflict with each other in anyway. I might be just overthinking it, but is there a way to add the listener only once?

[JAEKts]

Just tested this and you are completely correct. The undo/redo functionality stops working after switching text fields a lot since a bunch of listeners are added

[jakekarnes42]

This part is still a bit off. A couple specific things:

1. The newly added if block doesn't container the logic for the redo action. It looks like we might have close the block early.
2. The code within the if block isn't indented.

[jakekarnes42]

This is somewhat related to the comment above, but is this class the appropriate place to add this functionality?

This TextComponentFocusListener class is used across a lot of different text fields, not just the session policy text field. You can search across AWSSignerController for more examples, but here are just a few:

AWSSigner/src/main/java/com/netspi/awssigner/controller/AWSSignerController.java

Lines 251 to 258 in 0c2cace

	//Profile Region text field
	view.profileRegionTextField.addFocusListener(new TextComponentFocusListener<>(this, "Region", Profile::setRegion));
	//Profile Service text field
	view.profileServiceTextField.addFocusListener(new TextComponentFocusListener<>(this, "Service", Profile::setService));
	//Profile Key Id text field
	view.profileKeyIdTextField.addFocusListener(new TextComponentFocusListener<>(this, "Key Id", Profile::setKeyId));

If I'm understanding the PR correctly, we're intending to add these functions to the Session Policy text field, but I think this will apply the functionality to lots of different text fields in the application. Can we investigate that and confirm? I might just be missing something.

[JAEKts]

This is correct, all text fields utilizing this listener will now have undo/redo capability. I haven't seen any issues with this during the testing, each text field has its own history and even after switching from text field to text field, the history is working as expected.

[jakekarnes42]

It looks like these 2 keystrokes are the same. Maybe we wanted:

        KeyStroke redoKeyStroke1 = KeyStroke.getKeyStroke(KeyEvent.VK_Y, menuShortcutKeyMask);
        KeyStroke redoKeyStroke2 = KeyStroke.getKeyStroke(KeyEvent.VK_Z, menuShortcutKeyMask | InputEvent.SHIFT_DOWN_MASK);

That should make the 2 redo hotkeys: CTRL+Y and CTRL+SHIFT+Z (or using Command on Mac). I think that should match people's expectations.

[jakekarnes42]

I think we made the right choice by reverting most of the changes in this file, but we're back to the case where a new DocumentListener is added each time the text field gains focus. I think this might still work, but I don't know if we'd run into issues like before where someone clicks in and out of the field over and over.

I think there are two routes for this class. If we want to keep the API signature the same, we could do this:

package com.netspi.awssigner.controller;

import static com.netspi.awssigner.log.LogWriter.*;
import com.netspi.awssigner.model.Profile;
import java.awt.event.FocusEvent;
import java.awt.event.FocusListener;
import java.util.Optional;
import java.util.function.BiConsumer;
import javax.swing.event.DocumentEvent;
import javax.swing.event.DocumentListener;
import javax.swing.text.JTextComponent;
import java.util.Set;
import java.util.HashSet;

class TextComponentFocusListener<T extends Profile> implements FocusListener {

    private final AWSSignerController controller;
    private final String propertyLoggingName;
    private final BiConsumer<T, String> updateFunction;
    private Optional<Profile> currentProfileOptional = Optional.empty();
    private String previousValue = "";
    private static final Set<JTextComponent> initializedTextComponents = new HashSet<>();

    public TextComponentFocusListener(AWSSignerController controller, String propertyLoggingName, BiConsumer<T, String> updateFunction) {
        this.controller = controller;
        this.propertyLoggingName = propertyLoggingName;
        this.updateFunction = updateFunction;
    }

    @Override
    public void focusGained(FocusEvent e) {
        JTextComponent textComponent = (JTextComponent) e.getComponent();
        currentProfileOptional = controller.getCurrentSelectedProfile();
        previousValue = textComponent.getText();

        // Add DocumentListener only once per component
        if (!initializedTextComponents.contains(textComponent)) {
            textComponent.getDocument().addDocumentListener(new DocumentListener() {
                @Override
                public void insertUpdate(DocumentEvent e) {
                    textChanged(textComponent);
                }

                @Override
                public void removeUpdate(DocumentEvent e) {
                    textChanged(textComponent);
                }

                @Override
                public void changedUpdate(DocumentEvent e) {
                    // Typically not used for plain text components
                }
            });
            initializedTextComponents.add(textComponent);
        }

        logDebug("Profile " + propertyLoggingName + " Text Field focus gained. Cause: " + e.getCause() + " ID:" + e.getID() + " Current value: " + previousValue);
    }

    @Override
    public void focusLost(FocusEvent e) {
        JTextComponent textComponent = (JTextComponent) e.getComponent();
        logDebug("Profile " + propertyLoggingName + " Text Field focus lost. Cause: " + e.getCause() + " ID:" + e.getID());
        String currentText = textComponent.getText();
        if (!previousValue.equals(currentText)) {
            updateProfile(currentText);
        }
    }

    private void textChanged(JTextComponent textComponent) {
        String currentText = textComponent.getText();
        if (!previousValue.equals(currentText)) {
            previousValue = currentText;
            updateProfile(currentText);
        }
    }

    private void updateProfile(String currentText) {
        if (currentProfileOptional.isPresent()) {
            Profile currentProfile = currentProfileOptional.get();
            logInfo("Profile " + currentProfile.getName() + " " + propertyLoggingName + " text changed. New Value: " + currentText);
            updateFunction.accept((T) currentProfile, currentText);
            controller.updateProfileStatus();
        } else {
            logDebug("Profile " + propertyLoggingName + " changed, but no profile selected. Ignoring.");
        }
    }
}

The above should work but the use of a Set for tracking feels like a bit of overkill. The advantage is that it's "drop-in" for the current class and wouldn't require other changes. The alternative would be to a larger change to the class like so:

package com.netspi.awssigner.controller;

import static com.netspi.awssigner.log.LogWriter.*;
import com.netspi.awssigner.model.Profile;
import java.awt.event.FocusEvent;
import java.awt.event.FocusListener;
import java.util.Optional;
import java.util.function.BiConsumer;
import javax.swing.event.DocumentEvent;
import javax.swing.event.DocumentListener;
import javax.swing.text.JTextComponent;

class TextComponentChangeListener<T extends Profile> implements FocusListener, DocumentListener {

    private final AWSSignerController controller;
    private final JTextComponent textComponent;
    private final String propertyLoggingName;
    private final BiConsumer<T, String> updateFunction;
    private String previousValue = "";

    public TextComponentChangeListener(AWSSignerController controller, JTextComponent textComponent, String propertyLoggingName, BiConsumer<T, String> updateFunction) {
        this.controller = controller;
        this.textComponent = textComponent;
        this.propertyLoggingName = propertyLoggingName;
        this.updateFunction = updateFunction;

        // Initialize previousValue and add listeners once
        this.previousValue = textComponent.getText();
        this.textComponent.addFocusListener(this);
        this.textComponent.getDocument().addDocumentListener(this);
    }

    @Override
    public void focusGained(FocusEvent e) {
        previousValue = textComponent.getText(); // Update previous value on focus gain
        logDebug("Profile " + propertyLoggingName + " Text Field focus gained. Cause: " + e.getCause() + " ID:" + e.getID() + " Current value: " + previousValue);
    }

    @Override
    public void focusLost(FocusEvent e) {
        logDebug("Profile " + propertyLoggingName + " Text Field focus lost. Cause: " + e.getCause() + " ID:" + e.getID());
        String currentText = textComponent.getText();
        if (!previousValue.equals(currentText)) {
            updateProfile(currentText);
        }
    }

    // DocumentListener methods
    @Override
    public void insertUpdate(DocumentEvent e) {
        textChanged();
    }

    @Override
    public void removeUpdate(DocumentEvent e) {
        textChanged();
    }

    @Override
    public void changedUpdate(DocumentEvent e) {
        // Typically not used for plain text components
    }

    private void textChanged() {
        String currentText = textComponent.getText();
        if (!previousValue.equals(currentText)) {
            previousValue = currentText;
            updateProfile(currentText);
        }
    }

    private void updateProfile(String currentText) {
        Optional<Profile> currentProfileOptional = controller.getCurrentSelectedProfile();
        if (currentProfileOptional.isPresent()) {
            Profile currentProfile = currentProfileOptional.get();
            try {
                @SuppressWarnings("unchecked")
                T profile = (T) currentProfile;
                logInfo("Profile " + currentProfile.getName() + " " + propertyLoggingName + " text changed. New Value: " + currentText);
                updateFunction.accept(profile, currentText);
                controller.updateProfileStatus();
            } catch (ClassCastException e) {
                logError("Type mismatch: Cannot cast " + currentProfile.getClass().getName() + " to the expected type.");
            }
        } else {
            logDebug("Profile " + propertyLoggingName + " changed, but no profile selected. Ignoring.");
        }
    }
}

I think this is a bit better because now the class is both a FocusListener and DocumentListener. We've already renamed the class to TextComponentChangeListener to reflect this. Because we're changing that class, we'd need to rename the file and update how it's used in AWSSignerController too:

private void initListeners() {
    // ...

    // Profile Region text field
    new TextComponentChangeListener<>(this, view.profileRegionTextField, "Region", Profile::setRegion);

    // Profile Service text field
    new TextComponentChangeListener<>(this, view.profileServiceTextField, "Service", Profile::setService);

    // Profile Key Id text field
    new TextComponentChangeListener<>(this, view.profileKeyIdTextField, "Key Id", Profile::setKeyId);

    // Static Credentials Access Key text field
    new TextComponentChangeListener<>(this, view.staticAccessKeyTextField, "Static Credentials Access Key", StaticCredentialsProfile::setAccessKey);

    // Static Credentials Secret Key text field
    new TextComponentChangeListener<>(this, view.staticSecretKeyTextField, "Static Credentials Secret Key", StaticCredentialsProfile::setSecretKey);

    // Static Credentials Session Token text field
    new TextComponentChangeListener<>(this, view.staticSessionTokenTextField, "Static Credentials Session Token", StaticCredentialsProfile::setSessionToken);

    // AssumeRole Role ARN text field
    new TextComponentChangeListener<>(this, view.assumeRoleRoleArnTextField, "AssumeRole Role ARN", AssumeRoleProfile::setRoleArn);

    // AssumeRole Session Name text field
    new TextComponentChangeListener<>(this, view.assumeRoleSessionNameTextField, "AssumeRole Session Name", AssumeRoleProfile::setSessionName);

    // AssumeRole External Id text field
    new TextComponentChangeListener<>(this, view.assumeRoleExternalIdTextField, "AssumeRole External Id", AssumeRoleProfile::setExternalId);

    // AssumeRole Duration text field
    new TextComponentChangeListener<>(this, view.assumeRoleDurationTextField, "AssumeRole Duration Seconds", AssumeRoleProfile::setDurationSecondsFromText);

    // AssumeRole Session Policy text area
    new TextComponentChangeListener<>(this, view.assumeRoleSessionPolicyTextArea, "AssumeRole Session Policy", AssumeRoleProfile::setSessionPolicy);

    // Command Command text field
    new TextComponentChangeListener<>(this, view.commandCommandTextField, "Command Command", CommandProfile::setCommand);

    // Command Duration text field
    new TextComponentChangeListener<>(this, view.commandDurationTextField, "Command Duration Seconds", CommandProfile::setDurationSecondsFromText);

    // ...
}