Index: trunk/src/org/openstreetmap/josm/tools/StreamUtils.java =================================================================== --- trunk/src/org/openstreetmap/josm/tools/StreamUtils.java (revision 10585) +++ trunk/src/org/openstreetmap/josm/tools/StreamUtils.java (revision 10585) @@ -0,0 +1,32 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.tools; + +import java.util.Iterator; +import java.util.Spliterator; +import java.util.Spliterators; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; + +/** + * Utility methods for streams. + * @author Michael Zangl + * @since 10585 + */ +public final class StreamUtils { + + /** + * Utility class + */ + private StreamUtils() {} + + /** + * Convert an iterator to a stream. + * @param The element type to iterate over + * @param iterator The iterator + * @return The stream of for that iterator. + */ + public static Stream toStream(Iterator iterator) { + Spliterator spliterator = Spliterators.spliteratorUnknownSize(iterator, Spliterator.ORDERED); + return StreamSupport.stream(spliterator, false); + } +} Index: trunk/src/org/openstreetmap/josm/tools/bugreport/BugReport.java =================================================================== --- trunk/src/org/openstreetmap/josm/tools/bugreport/BugReport.java (revision 10584) +++ trunk/src/org/openstreetmap/josm/tools/bugreport/BugReport.java (revision 10585) @@ -1,4 +1,10 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.tools.bugreport; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.concurrent.CopyOnWriteArrayList; + +import org.openstreetmap.josm.actions.ShowStatusReportAction; /** @@ -20,5 +26,5 @@ * try { * ... your code ... - * } catch (Throwable t) { + * } catch (RuntimeException t) { * throw BugReport.intercept(t).put("id", id).put("tag", tag); * } @@ -32,10 +38,117 @@ */ public final class BugReport { + private boolean includeStatusReport = true; + private boolean includeData = true; + private boolean includeAllStackTraces; + private ReportedException exception; + private final CopyOnWriteArrayList listeners = new CopyOnWriteArrayList<>(); + /** * Create a new bug report * @param e The {@link ReportedException} to use. No more data should be added after creating the report. */ - private BugReport(ReportedException e) { - // TODO: Use this class to create the bug report. + BugReport(ReportedException e) { + this.exception = e; + includeAllStackTraces = e.mayHaveConcurrentSource(); + } + + /** + * Get if this report should include a system status report + * @return true to include it. + * @since 10585 + */ + public boolean getIncludeStatusReport() { + return includeStatusReport; + } + + /** + * Set if this report should include a system status report + * @param includeStatusReport if the status report should be included + * @since 10585 + */ + public void setIncludeStatusReport(boolean includeStatusReport) { + this.includeStatusReport = includeStatusReport; + fireChange(); + } + + /** + * Get if this report should include the data that was traced. + * @return true to include it. + * @since 10585 + */ + public boolean getIncludeData() { + return includeData; + } + + /** + * Set if this report should include the data that was traced. + * @param includeData if data should be included + * @since 10585 + */ + public void setIncludeData(boolean includeData) { + this.includeData = includeData; + fireChange(); + } + + /** + * Get if this report should include the stack traces for all other threads. + * @return true to include it. + * @since 10585 + */ + public boolean getIncludeAllStackTraces() { + return includeAllStackTraces; + } + + /** + * Sets if this report should include the stack traces for all other threads. + * @param includeAllStackTraces if all stack traces should be included + * @since 10585 + */ + public void setIncludeAllStackTraces(boolean includeAllStackTraces) { + this.includeAllStackTraces = includeAllStackTraces; + fireChange(); + } + + /** + * Gets the full string that should be send as error report. + * @return The string. + * @since 10585 + */ + public String getReportText() { + StringWriter stringWriter = new StringWriter(); + PrintWriter out = new PrintWriter(stringWriter); + if (getIncludeStatusReport()) { + out.println(ShowStatusReportAction.getReportHeader()); + } + if (getIncludeData()) { + exception.printReportDataTo(out); + } + exception.printReportStackTo(out); + if (getIncludeAllStackTraces()) { + exception.printReportThreadsTo(out); + } + return stringWriter.toString().replaceAll("\r", ""); + } + + /** + * Add a new change listener. + * @param listener The listener + * @since 10585 + */ + public void addChangeListener(BugReportListener listener) { + listeners.add(listener); + } + + /** + * Remove a change listener. + * @param listener The listener + * @since 10585 + */ + public void removeChangeListener(BugReportListener listener) { + listeners.remove(listener); + } + + private void fireChange() { + listeners.stream().forEach(l -> l.bugReportChanged(this)); } @@ -75,3 +188,17 @@ return "?"; } + + /** + * A listener that listens to changes to this report. + * @author Michael Zangl + * @since 10585 + */ + @FunctionalInterface + public interface BugReportListener { + /** + * Called whenever this bug report was changed, e.g. the data to be included in it. + * @param report The report that was changed. + */ + void bugReportChanged(BugReport report); + } } Index: trunk/src/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandler.java =================================================================== --- trunk/src/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandler.java (revision 10584) +++ trunk/src/org/openstreetmap/josm/tools/bugreport/BugReportExceptionHandler.java (revision 10585) @@ -197,16 +197,13 @@ static JPanel buildPanel(final Throwable e) { - StringWriter stack = new StringWriter(); - PrintWriter writer = new PrintWriter(stack); + DebugTextDisplay textarea; if (e instanceof ReportedException) { // Temporary! - ((ReportedException) e).printReportDataTo(writer); - ((ReportedException) e).printReportStackTo(writer); + textarea = new DebugTextDisplay(new BugReport((ReportedException) e)); } else { - e.printStackTrace(writer); - } - - String text = ShowStatusReportAction.getReportHeader() + stack.getBuffer().toString(); - text = text.replaceAll("\r", ""); + StringWriter stack = new StringWriter(); + e.printStackTrace(new PrintWriter(stack)); + textarea = new DebugTextDisplay(ShowStatusReportAction.getReportHeader() + stack.getBuffer().toString()); + } JPanel p = new JPanel(new GridBagLayout()); @@ -220,5 +217,5 @@ "file a bug report in our bugtracker using this link:")), GBC.eol().fill(GridBagConstraints.HORIZONTAL)); - p.add(new JButton(new ReportBugAction(text)), GBC.eop().insets(8, 0, 0, 0)); + p.add(new JButton(new ReportBugAction(textarea.getCodeText())), GBC.eop().insets(8, 0, 0, 0)); p.add(new JMultilineLabel( tr("There the error information provided below should already be " + @@ -230,7 +227,4 @@ "below at this URL:")), GBC.eol().fill(GridBagConstraints.HORIZONTAL)); p.add(new UrlLabel(Main.getJOSMWebsite()+"/newticket", 2), GBC.eop().insets(8, 0, 0, 0)); - - // Wiki formatting for manual copy-paste - DebugTextDisplay textarea = new DebugTextDisplay(text); if (textarea.copyToClippboard()) { Index: trunk/src/org/openstreetmap/josm/tools/bugreport/BugReportSettingsPanel.java =================================================================== --- trunk/src/org/openstreetmap/josm/tools/bugreport/BugReportSettingsPanel.java (revision 10585) +++ trunk/src/org/openstreetmap/josm/tools/bugreport/BugReportSettingsPanel.java (revision 10585) @@ -0,0 +1,38 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.tools.bugreport; + +import static org.openstreetmap.josm.tools.I18n.tr; + +import javax.swing.BoxLayout; +import javax.swing.JCheckBox; +import javax.swing.JPanel; + +/** + * This panel displays the settings that can be changed before submitting a bug report to the web page. + * @author Michael Zangl + * @since 10585 + */ +public class BugReportSettingsPanel extends JPanel { + /** + * Creates the new settings panel. + * @param report The report this panel should influence. + */ + public BugReportSettingsPanel(BugReport report) { + setLayout(new BoxLayout(this, BoxLayout.Y_AXIS)); + + JCheckBox statusReport = new JCheckBox(tr("Include the system status report.")); + statusReport.setSelected(report.getIncludeStatusReport()); + statusReport.addChangeListener(e -> report.setIncludeStatusReport(statusReport.isSelected())); + add(statusReport); + + JCheckBox data = new JCheckBox(tr("Include information about the data that was worked on.")); + data.setSelected(report.getIncludeData()); + data.addChangeListener(e -> report.setIncludeData(data.isSelected())); + add(data); + + JCheckBox allStackTraces = new JCheckBox(tr("Include all stack traces.")); + allStackTraces.setSelected(report.getIncludeAllStackTraces()); + allStackTraces.addChangeListener(e -> report.setIncludeAllStackTraces(allStackTraces.isSelected())); + add(allStackTraces); + } +} Index: trunk/src/org/openstreetmap/josm/tools/bugreport/DebugTextDisplay.java =================================================================== --- trunk/src/org/openstreetmap/josm/tools/bugreport/DebugTextDisplay.java (revision 10584) +++ trunk/src/org/openstreetmap/josm/tools/bugreport/DebugTextDisplay.java (revision 10585) @@ -15,13 +15,14 @@ */ public class DebugTextDisplay extends JScrollPane { - private final String text; + private static final String CODE_PATTERN = "{{{%n%s%n}}}"; + private String text; + private JosmTextArea textArea; /** - * Creates a new text are with the fixed text - * @param textToDisplay The text to display. + * Creates a new text area. + * @since 10585 */ - public DebugTextDisplay(String textToDisplay) { - text = "{{{\n" + Utils.strip(textToDisplay) + "\n}}}"; - JosmTextArea textArea = new JosmTextArea(text); + private DebugTextDisplay() { + textArea = new JosmTextArea(); textArea.setCaretPosition(0); textArea.setEditable(false); @@ -31,9 +32,47 @@ /** - * Copies the debug text to the clippboard. + * Creates a new text area with an inital text to display + * @param textToDisplay The text to display. + */ + public DebugTextDisplay(String textToDisplay) { + this(); + setCodeText(textToDisplay); + } + + /** + * Creates a new text area that displays the bug report data + * @param report The bug report data to display. + * @since 10585 + */ + public DebugTextDisplay(BugReport report) { + this(); + setCodeText(report.getReportText()); + report.addChangeListener(e -> setCodeText(report.getReportText())); + } + + /** + * Sets the text that should be displayed in this view. + * @param textToDisplay The text + */ + private void setCodeText(String textToDisplay) { + text = Utils.strip(textToDisplay).replaceAll("\r", ""); + textArea.setText(String.format(CODE_PATTERN, text)); + } + + /** + * Copies the debug text to the clippboard. This includes the code tags for trac. * @return true if copy was successful */ public boolean copyToClippboard() { - return Utils.copyToClipboard(text); + return Utils.copyToClipboard(String.format(CODE_PATTERN, text)); + } + + /** + * Gets the text this are displays, without the code tag. + * @return The stripped text set by {@link #setCodeText(String)} + * @since 10585 + */ + public String getCodeText() { + return text; } } Index: trunk/src/org/openstreetmap/josm/tools/bugreport/ReportedException.java =================================================================== --- trunk/src/org/openstreetmap/josm/tools/bugreport/ReportedException.java (revision 10584) +++ trunk/src/org/openstreetmap/josm/tools/bugreport/ReportedException.java (revision 10585) @@ -3,15 +3,21 @@ import java.io.PrintWriter; +import java.io.Serializable; +import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.ConcurrentModificationException; import java.util.IdentityHashMap; +import java.util.Iterator; import java.util.LinkedList; import java.util.Map; import java.util.Map.Entry; +import java.util.NoSuchElementException; import java.util.Set; import org.openstreetmap.josm.Main; +import org.openstreetmap.josm.tools.StreamUtils; /** @@ -25,9 +31,11 @@ */ public class ReportedException extends RuntimeException { + /** + * How many entries of a collection to include in the bug report. + */ private static final int MAX_COLLECTION_ENTRIES = 30; - /** - * - */ + private static final long serialVersionUID = 737333873766201033L; + /** * We capture all stack traces on exception creation. This allows us to trace synchonization problems better. We cannot be really sure what @@ -35,5 +43,5 @@ */ private final transient Map allStackTraces; - private final transient LinkedList
sections = new LinkedList<>(); + private final LinkedList
sections = new LinkedList<>(); private final transient Thread caughtOnThread; private final Throwable exception; @@ -144,15 +152,13 @@ } - Set dejaVu = Collections.newSetFromMap(new IdentityHashMap()); - return hasSameStackTrace(dejaVu, this.exception, e.exception); - } - - private static boolean hasSameStackTrace(Set dejaVu, Throwable e1, Throwable e2) { - if (dejaVu.contains(e1)) { - // cycle. If it was the same until here, we assume both have that cycle. + return hasSameStackTrace(new CauseTraceIterator(), e.exception); + } + + private static boolean hasSameStackTrace(CauseTraceIterator causeTraceIterator, Throwable e2) { + if (!causeTraceIterator.hasNext()) { + // all done. return true; } - dejaVu.add(e1); - + Throwable e1 = causeTraceIterator.next(); StackTraceElement[] t1 = e1.getStackTrace(); StackTraceElement[] t2 = e2.getStackTrace(); @@ -167,5 +173,5 @@ return false; } else if (c1 != null) { - return hasSameStackTrace(dejaVu, c1, c2); + return hasSameStackTrace(causeTraceIterator, c2); } else { return true; @@ -228,5 +234,52 @@ } - private static class SectionEntry { + + /** + * Check if this exception may be caused by a threading issue. + * @return true if it is. + * @since 10585 + */ + public boolean mayHaveConcurrentSource() { + return StreamUtils.toStream(new CauseTraceIterator()) + .anyMatch(t -> t instanceof ConcurrentModificationException || t instanceof InvocationTargetException); + } + + /** + * Iterates over the causes for this exception. Ignores cycles and aborts iteration then. + * @author Michal Zangl + * @since 10585 + */ + private final class CauseTraceIterator implements Iterator { + private Throwable current = exception; + private final Set dejaVu = Collections.newSetFromMap(new IdentityHashMap()); + + @Override + public boolean hasNext() { + return current != null; + } + + @Override + public Throwable next() { + if (!hasNext()) { + throw new NoSuchElementException(); + } + Throwable toReturn = current; + advance(); + return toReturn; + } + + private void advance() { + dejaVu.add(current); + current = current.getCause(); + if (current != null && dejaVu.contains(current)) { + current = null; + } + } + } + + private static class SectionEntry implements Serializable { + + private static final long serialVersionUID = 1L; + private final String key; private final String value; @@ -249,5 +302,7 @@ } - private static class Section { + private static class Section implements Serializable { + + private static final long serialVersionUID = 1L; private final String sectionName;