-
-
Notifications
You must be signed in to change notification settings - Fork 716
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
client: Export Client Map #6040
base: main
Are you sure you want to change the base?
Conversation
Switched to draft, there's some obvious things I need to clean up. |
This comment has been minimized.
This comment has been minimized.
7a5b30f
to
85773f8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely need a good set of unit tests.
addOns/client/src/main/java/org/zaproxy/addon/client/ui/PopupMenuExportClientMap.java
Outdated
Show resolved
Hide resolved
addOns/client/src/main/java/org/zaproxy/addon/client/ui/PopupMenuExportClientMap.java
Outdated
Show resolved
Hide resolved
FYI I did validate via: https://jsonformatter.org/yaml-validator |
995975b
to
3094c2b
Compare
Now excludes text on storage events. |
addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientSideComponent.java
Outdated
Show resolved
Hide resolved
d404916
to
e1e741e
Compare
public static void exportClientMap(File file) throws IOException { | ||
try (FileWriter fw = new FileWriter(file, false)) { | ||
exportClientMap(fw); | ||
} | ||
} | ||
|
||
public static void exportClientMap(Writer fw) throws IOException { | ||
exportClientMap(fw, extension.getClientTree()); | ||
} | ||
|
||
public static void exportClientMap(Writer fw, ClientMap clientMap) throws IOException { | ||
try (BufferedWriter bw = new BufferedWriter(fw)) { | ||
outputNode(bw, clientMap.getRoot(), 0); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I reduce visibility on these?
5588fb8
to
1b4cc93
Compare
4979437
to
9d33816
Compare
Now includes simple help content. |
addOns/client/src/test/java/org/zaproxy/addon/client/internal/ClientSideComponenetUnitTest.java
Outdated
Show resolved
Hide resolved
import org.zaproxy.zap.testutils.TestUtils; | ||
|
||
/* Unit Tests for {@code ClientSideComponenetUnitTest} | ||
* SoretedSets are used for compareTo testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would have been better to test each field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can also add tests which do that, it was on my todo I just hadn't managed to get to that yet.
addOns/client/src/test/java/org/zaproxy/addon/client/internal/ClientSideComponenetUnitTest.java
Outdated
Show resolved
Hide resolved
addOns/client/src/test/java/org/zaproxy/addon/client/internal/ClientSideComponenetUnitTest.java
Outdated
Show resolved
Hide resolved
addOns/client/src/test/java/org/zaproxy/addon/client/internal/ClientSideComponenetUnitTest.java
Outdated
Show resolved
Hide resolved
addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientMapWriter.java
Outdated
Show resolved
Hide resolved
addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientMapWriter.java
Outdated
Show resolved
Hide resolved
&& "nodeAdded".equalsIgnoreCase(component.getType())))) { | ||
continue; | ||
} | ||
first = outputKV(fw, indent, first, "nodeType", component.getTypeForDisplay()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't be using i18n'ed strings in the export.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand your point and I'm happy to change it somehow, this is what was asked for.
Shall I switch display types to an enum so the export can use standard labels/names while the UI uses i18n values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be better but it depends how much time that adds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm open to suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there are no time constraints anymore +1 to enum.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious why you feel an enum will be slow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or do you mean implementation time?
addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientMapWriter.java
Outdated
Show resolved
Hide resolved
addOns/client/src/main/java/org/zaproxy/addon/client/internal/ClientMapWriter.java
Outdated
Show resolved
Hide resolved
66e785c
to
4f0c7c3
Compare
Signed-off-by: kingthorin <[email protected]>
Overview
Add a context menu to facilitate exporting the Client Map as YAML.
Related Issues
N/A
Checklist
./gradlew spotlessApply
for code formattingexample output
FYI I did validate via: https://jsonformatter.org/yaml-validator