From 96150f40301a5be6ac8ba39f54147cfc41116bdd Mon Sep 17 00:00:00 2001 From: Kamil Jarmusik Date: Fri, 10 May 2024 22:51:53 +0200 Subject: [PATCH] #2823 fixed consolidated chart not showing all data from report: - fixed sorting legend; - refactor: use TreeMap in PointTimeSeriesCollection for same key, which is to ensure data consistency for chart; - using the SeriesIdentifier class that implements Comparable, instead of String; --- .../mango/vo/report/DiscreteTimeSeries.java | 13 +-- .../mango/vo/report/ImageChartUtils.java | 81 +++++++++-------- .../vo/report/PointTimeSeriesCollection.java | 87 +++++++++++-------- .../mango/vo/report/ReportChartCreator.java | 27 +++--- .../web/servlet/AsyncImageChartServlet.java | 6 +- .../mango/web/servlet/ImageChartServlet.java | 3 +- 6 files changed, 120 insertions(+), 97 deletions(-) diff --git a/src/com/serotonin/mango/vo/report/DiscreteTimeSeries.java b/src/com/serotonin/mango/vo/report/DiscreteTimeSeries.java index cec87a6ffb..c52bbf98ee 100644 --- a/src/com/serotonin/mango/vo/report/DiscreteTimeSeries.java +++ b/src/com/serotonin/mango/vo/report/DiscreteTimeSeries.java @@ -31,17 +31,18 @@ * @author Matthew Lohbihler */ public class DiscreteTimeSeries { - private final String name; + private final Comparable name; private final TextRenderer textRenderer; private final Paint paint; - private final List valueTimes = new ArrayList(); - private final List valueDescriptions = new ArrayList(); + private final List valueTimes = new ArrayList<>(); + private final List valueDescriptions = new ArrayList<>(); - public DiscreteTimeSeries(String name, TextRenderer textRenderer) { + @Deprecated(since = "2.7.8") + public DiscreteTimeSeries(Comparable name, TextRenderer textRenderer) { this(name, textRenderer, null); } - public DiscreteTimeSeries(String name, TextRenderer textRenderer, Paint paint) { + public DiscreteTimeSeries(Comparable name, TextRenderer textRenderer, Paint paint) { this.name = name; this.textRenderer = textRenderer; this.paint = paint; @@ -69,7 +70,7 @@ public void addValueTime(PointValueTime pvt) { } } - public String getName() { + public Comparable getName() { return name; } diff --git a/src/com/serotonin/mango/vo/report/ImageChartUtils.java b/src/com/serotonin/mango/vo/report/ImageChartUtils.java index 78f8365bbc..aa584e6c7f 100644 --- a/src/com/serotonin/mango/vo/report/ImageChartUtils.java +++ b/src/com/serotonin/mango/vo/report/ImageChartUtils.java @@ -18,13 +18,12 @@ */ package com.serotonin.mango.vo.report; -import java.awt.Color; -import java.awt.Paint; +import java.awt.*; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; -import java.util.Date; +import java.util.*; import java.util.List; import java.util.stream.Collectors; @@ -32,13 +31,12 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.jfree.chart.ChartFactory; -import org.jfree.chart.ChartUtilities; -import org.jfree.chart.JFreeChart; +import org.jfree.chart.*; import org.jfree.chart.annotations.XYTextAnnotation; import org.jfree.chart.plot.XYPlot; import org.jfree.chart.renderer.xy.XYLineAndShapeRenderer; import org.jfree.chart.renderer.xy.XYStepRenderer; +import org.jfree.chart.title.LegendTitle; import org.jfree.data.general.SeriesException; import org.jfree.data.time.Second; import org.jfree.data.time.TimeSeries; @@ -56,7 +54,9 @@ /** * @author Matthew Lohbihler */ -public class ImageChartUtils { +public final class ImageChartUtils { + + private ImageChartUtils() {} private static final Log LOG = LogFactory.getLog(ImageChartUtils.class); @@ -99,12 +99,11 @@ public static void writeChart(PointTimeSeriesCollection pointTimeSeriesCollectio double numericMin = 0; double numericMax = 1; if (pointTimeSeriesCollection.hasNumericData()) { - // XYSplineRenderer numericRenderer = new XYSplineRenderer(); - // numericRenderer.setBaseShapesVisible(false); XYLineAndShapeRenderer numericRenderer = new XYLineAndShapeRenderer(true, false); + TimeSeriesCollection timeSeriesCollection = pointTimeSeriesCollection.getNumericTimeSeriesCollection(); - plot.setDataset(NUMERIC_DATA_INDEX, pointTimeSeriesCollection.getNumericTimeSeriesCollection()); plot.setRenderer(NUMERIC_DATA_INDEX, numericRenderer); + plot.setDataset(NUMERIC_DATA_INDEX, timeSeriesCollection); for (int i = 0; i < pointTimeSeriesCollection.getNumericPaint().size(); i++) { Paint paint = pointTimeSeriesCollection.getNumericPaint().get(i); @@ -131,20 +130,20 @@ public static void writeChart(PointTimeSeriesCollection pointTimeSeriesCollectio if (pointTimeSeriesCollection.hasDiscreteData()) { XYStepRenderer discreteRenderer = new XYStepRenderer(); + TimeSeriesCollection timeSeriesCollection = pointTimeSeriesCollection.createTimeSeriesCollection(numericMin, numericMax); + plot.setRenderer(DISCRETE_DATA_INDEX, discreteRenderer, false); + plot.setDataset(DISCRETE_DATA_INDEX, timeSeriesCollection); // Plot the data int discreteValueCount = pointTimeSeriesCollection.getDiscreteValueCount(); - double interval = (numericMax - numericMin) / (discreteValueCount + 1); - - TimeSeriesCollection timeSeriesCollection = pointTimeSeriesCollection.createTimeSeriesCollection(numericMin, interval); - - plot.setDataset(DISCRETE_DATA_INDEX, timeSeriesCollection); + int discreteSeriesCount = pointTimeSeriesCollection.getDiscreteSeriesCount(); + double interval = pointTimeSeriesCollection.getDiscreteInterval(numericMin, numericMax); // Add the value annotations. double annoX = plot.getDomainAxis().getLowerBound(); int intervalIndex = 1; - for (int i = 0; i < pointTimeSeriesCollection.getDiscreteSeriesCount(); i++) { + for (int i = 0; i < discreteSeriesCount; i++) { DiscreteTimeSeries dts = pointTimeSeriesCollection.getDiscreteTimeSeries(i); if (dts.getPaint() != null) discreteRenderer.setSeriesPaint(i, dts.getPaint()); @@ -165,30 +164,17 @@ public static void writeChart(PointTimeSeriesCollection pointTimeSeriesCollectio } } + if(showLegend && chart.getLegend() != null) { + LegendItemCollection legendItemCollection = getLegendItemCollectionSort(chart); + if(legendItemCollection != null) + plot.setFixedLegendItems(legendItemCollection); + } + + // Return the image. ChartUtilities.writeChartAsPNG(out, chart, width, height); } - // public static void writeChart(TimeSeries timeSeries, OutputStream out, int width, int height) throws IOException - // { - // writeChart(new TimeSeriesCollection(timeSeries), false, out, width, height); - // } - // - // public static void writeChart(TimeSeriesCollection timeSeriesCollection, boolean showLegend, OutputStream out, - // int width, int height) throws IOException { - // JFreeChart chart = ChartFactory.createTimeSeriesChart(null, null, null, timeSeriesCollection, showLegend, - // false, false); - // chart.setBackgroundPaint(Color.white); - // - // // Change the plot renderer - // // XYPlot plot = chart.getXYPlot(); - // // XYLineAndShapeRenderer renderer = new XYLineAndShapeRenderer(); - // // plot.setRenderer(renderer); - // - // // Return the image. - // ChartUtilities.writeChartAsPNG(out, chart, width, height); - // } - public static void writeChart(HttpServletResponse response, byte[] chartData) throws IOException { response.setContentType(getContentType()); StreamUtils.transfer(new ByteArrayInputStream(chartData), response.getOutputStream()); @@ -246,6 +232,27 @@ public static String calculatePointNameForReport(String extendedName) { } private static List toListNames(List pointStatistics) { - return pointStatistics.stream().map(ReportChartCreator.PointStatistics::getName).collect(Collectors.toList()); + return pointStatistics.stream().map(a -> a.getName().toString()).collect(Collectors.toList()); + } + + private static LegendItemCollection getLegendItemCollectionSort(JFreeChart chart) { + LegendTitle legend = chart.getLegend(); + LegendItemSource[] legendItemSource = legend.getSources(); + if(legendItemSource.length > 0) { + LegendItemCollection legendItemCollections = legendItemSource[0].getLegendItems(); + Iterator iterator = legendItemCollections.iterator(); + List legendItems = new ArrayList<>(); + while (iterator.hasNext()) { + LegendItem legendItem = (LegendItem) iterator.next(); + legendItems.add(legendItem); + } + legendItems.sort(Comparator.comparing(LegendItem::getSeriesKey)); + LegendItemCollection legendItemCollection = new LegendItemCollection(); + for (LegendItem legendItem : legendItems) { + legendItemCollection.add(legendItem); + } + return legendItemCollection; + } + return null; } } diff --git a/src/com/serotonin/mango/vo/report/PointTimeSeriesCollection.java b/src/com/serotonin/mango/vo/report/PointTimeSeriesCollection.java index 34423aa466..e83ad9d791 100644 --- a/src/com/serotonin/mango/vo/report/PointTimeSeriesCollection.java +++ b/src/com/serotonin/mango/vo/report/PointTimeSeriesCollection.java @@ -19,10 +19,7 @@ package com.serotonin.mango.vo.report; import java.awt.Paint; -import java.util.ArrayList; - -import java.util.Comparator; -import java.util.List; +import java.util.*; import org.jfree.data.time.Second; import org.jfree.data.time.TimeSeries; @@ -34,28 +31,29 @@ * @author Matthew Lohbihler */ public class PointTimeSeriesCollection { - private TimeSeriesCollection numericTimeSeriesCollection; - private List numericPaint; - private List discreteTimeSeriesCollection; - public void addNumericTimeSeries(TimeSeries numericTimeSeries) { - addNumericTimeSeries(numericTimeSeries, null); + private Map numericTimeSeriesCollection; + private Map discreteTimeSeriesCollection; + private Map numericPaint; + + public PointTimeSeriesCollection() { + this.numericTimeSeriesCollection = new TreeMap<>(); + this.discreteTimeSeriesCollection = new TreeMap<>(); + this.numericPaint = new TreeMap<>(); } - public void addNumericTimeSeries(TimeSeries numericTimeSeries, Paint paint) { - if (numericTimeSeriesCollection == null) { - numericTimeSeriesCollection = new TimeSeriesCollection(); - numericPaint = new ArrayList(); - } - numericTimeSeriesCollection.addSeries(numericTimeSeries); - numericPaint.add(paint); + @Deprecated(since = "2.7.8") + public void addNumericTimeSeries(TimeSeries timeSeries) { + addNumericTimeSeries(timeSeries, null); + } + + public void addNumericTimeSeries(TimeSeries timeSeries, Paint paint) { + numericTimeSeriesCollection.put(timeSeries.getKey(), timeSeries); + numericPaint.put(timeSeries.getKey(), paint); } public void addDiscreteTimeSeries(DiscreteTimeSeries discreteTimeSeries) { - if (discreteTimeSeriesCollection == null) - discreteTimeSeriesCollection = new ArrayList<>(); - discreteTimeSeriesCollection.add(discreteTimeSeries); - discreteTimeSeriesCollection.sort(Comparator.comparing(DiscreteTimeSeries::getName)); + discreteTimeSeriesCollection.put(discreteTimeSeries.getName(), discreteTimeSeries); } public boolean hasData() { @@ -63,49 +61,53 @@ public boolean hasData() { } public boolean hasNumericData() { - return numericTimeSeriesCollection != null; + return !numericTimeSeriesCollection.isEmpty(); } public boolean hasDiscreteData() { - return discreteTimeSeriesCollection != null; + return !discreteTimeSeriesCollection.isEmpty(); } public boolean hasMultiplePoints() { int count = 0; if (numericTimeSeriesCollection != null) - count += numericTimeSeriesCollection.getSeriesCount(); + count += numericTimeSeriesCollection.size(); if (discreteTimeSeriesCollection != null) count += discreteTimeSeriesCollection.size(); return count > 1; } public TimeSeriesCollection getNumericTimeSeriesCollection() { - return numericTimeSeriesCollection; + TimeSeriesCollection timeSeriesCollection = new TimeSeriesCollection(); + for(TimeSeries timeSeries: getTimeSeriesCollection()) { + timeSeriesCollection.addSeries(timeSeries); + } + return timeSeriesCollection; } public List getNumericPaint() { - return numericPaint; + return new ArrayList<>(numericPaint.values()); } public int getDiscreteValueCount() { int count = 0; - if (discreteTimeSeriesCollection != null) { - for (DiscreteTimeSeries dts : discreteTimeSeriesCollection) - count += dts.getDiscreteValueCount(); - } + + for (DiscreteTimeSeries dts : getDiscreteTimeSeriesCollection()) + count += dts.getDiscreteValueCount(); + return count; } - public TimeSeriesCollection createTimeSeriesCollection(double numericMin, double spacingInterval) { + public TimeSeriesCollection createTimeSeriesCollection(double numericMin, double numericMax) { + + double spacingInterval = getDiscreteInterval(numericMin, numericMax); TimeSeriesCollection timeSeriesCollection = new TimeSeriesCollection(); int intervalIndex = 1; - int index = 0; - for (DiscreteTimeSeries dts : discreteTimeSeriesCollection) { - SeriesIdentifier seriesIdentifier = new SeriesIdentifier(++index, dts.getName()); - TimeSeries ts = new TimeSeries(seriesIdentifier, null, null, Second.class); + for (DiscreteTimeSeries dts : getDiscreteTimeSeriesCollection()) { + TimeSeries ts = new TimeSeries(dts.getName(), null, null, Second.class); for (PointValueTime pvt : dts.getValueTimes()) ImageChartUtils.addSecond(ts, pvt.getTime(), numericMin @@ -120,10 +122,23 @@ public TimeSeriesCollection createTimeSeriesCollection(double numericMin, double } public int getDiscreteSeriesCount() { - return discreteTimeSeriesCollection.size(); + return getDiscreteTimeSeriesCollection().size(); } public DiscreteTimeSeries getDiscreteTimeSeries(int index) { - return discreteTimeSeriesCollection.get(index); + return getDiscreteTimeSeriesCollection().get(index); + } + + public double getDiscreteInterval(double numericMin, double numericMax) { + int discreteValueCount = getDiscreteValueCount(); + return (numericMax - numericMin) / (discreteValueCount + 1); + } + + private List getDiscreteTimeSeriesCollection() { + return new ArrayList<>(discreteTimeSeriesCollection.values()); + } + + private List getTimeSeriesCollection() { + return new ArrayList<>(numericTimeSeriesCollection.values()); } } diff --git a/src/com/serotonin/mango/vo/report/ReportChartCreator.java b/src/com/serotonin/mango/vo/report/ReportChartCreator.java index 1184128a31..c47cf31068 100644 --- a/src/com/serotonin/mango/vo/report/ReportChartCreator.java +++ b/src/com/serotonin/mango/vo/report/ReportChartCreator.java @@ -104,7 +104,7 @@ public void createContent(ReportInstance reportInstance, ReportDao reportDao, St reportDao.reportInstanceData(reportInstance.getId(), handler); pointStatistics = handler.getPointStatistics(); - pointStatistics.sort(Comparator.comparing(PointStatistics::getName)); + UsedImagesDirective inlineImages = new UsedImagesDirective(); // Prepare the model for the content rendering. @@ -269,7 +269,7 @@ public static int getLineLengthInLegendLimit() { public static class PointStatistics { private final int reportPointId; - private String name; + private Comparable name; private int dataType; private String dataTypeDescription; private String startValue; @@ -287,11 +287,11 @@ public PointStatistics(int reportPointId, String inlinePrefix) { this.inlinePrefix = inlinePrefix; } - public String getName() { + public Comparable getName() { return name; } - public void setName(String name) { + public void setName(Comparable name) { this.name = name; } @@ -453,7 +453,7 @@ class StreamHandler implements ReportDataStreamHandler, DataQuantizerCallback { File exportFile; private ReportCsvStreamer reportCsvStreamer; - private final List pointStatistics; + private final Map pointStatistics; private final PointTimeSeriesCollection pointTimeSeriesCollection; private PointStatistics point; @@ -462,7 +462,7 @@ class StreamHandler implements ReportDataStreamHandler, DataQuantizerCallback { private AbstractDataQuantizer quantizer; public StreamHandler(long start, long end, int imageWidth, boolean createExportFile, ResourceBundle bundle) { - pointStatistics = new ArrayList(); + pointStatistics = new TreeMap<>(); pointTimeSeriesCollection = new PointTimeSeriesCollection(); this.start = start; @@ -480,7 +480,7 @@ public StreamHandler(long start, long end, int imageWidth, boolean createExportF } public List getPointStatistics() { - return pointStatistics; + return new ArrayList<>(pointStatistics.values()); } public PointTimeSeriesCollection getPointTimeSeriesCollection() { @@ -489,9 +489,9 @@ public PointTimeSeriesCollection getPointTimeSeriesCollection() { public void startPoint(ReportPointInfo pointInfo) { donePoint(); - + SeriesIdentifier seriesIdentifier = new SeriesIdentifier(pointInfo.getReportPointId(), pointInfo.getExtendedNameForReport()); point = new PointStatistics(pointInfo.getReportPointId(), inlinePrefix); - point.setName(pointInfo.getExtendedNameForReport()); + point.setName(seriesIdentifier); point.setDataType(pointInfo.getDataType()); point.setDataTypeDescription(DataTypes.getDataTypeMessage(pointInfo.getDataType()).getLocalizedMessage( bundle)); @@ -499,8 +499,7 @@ public void startPoint(ReportPointInfo pointInfo) { if (pointInfo.getStartValue() != null) point.setStartValue(pointInfo.getTextRenderer().getText(pointInfo.getStartValue(), TextRenderer.HINT_FULL)); - pointStatistics.add(point); - pointStatistics.sort(Comparator.comparing(PointStatistics::getName)); + pointStatistics.put(point.getName(), point); Color colour = null; try { @@ -517,7 +516,7 @@ public void startPoint(ReportPointInfo pointInfo) { quantizer = new NumericDataQuantizer(start, end, imageWidth, this); discreteTimeSeries = null; - SeriesIdentifier seriesIdentifier = new SeriesIdentifier(pointInfo.getReportPointId(), pointInfo.getExtendedNameForReport()); + numericTimeSeries = new TimeSeries(seriesIdentifier, null, null); numericTimeSeries.setRangeDescription(point.getTextRenderer().getMetaText()); point.setNumericTimeSeries(numericTimeSeries); @@ -529,7 +528,7 @@ else if (pointInfo.getDataType() == DataTypes.MULTISTATE) { point.setStats(new StartsAndRuntimeList(pointInfo.getStartValue(), start, end)); quantizer = new MultistateDataQuantizer(start, end, imageWidth, this); - discreteTimeSeries = new DiscreteTimeSeries(pointInfo.getExtendedNameForReport(), pointInfo.getTextRenderer(), + discreteTimeSeries = new DiscreteTimeSeries(seriesIdentifier, pointInfo.getTextRenderer(), colour); point.setDiscreteTimeSeries(discreteTimeSeries); if (pointInfo.isConsolidatedChart()) @@ -540,7 +539,7 @@ else if (pointInfo.getDataType() == DataTypes.BINARY) { point.setStats(new StartsAndRuntimeList(pointInfo.getStartValue(), start, end)); quantizer = new BinaryDataQuantizer(start, end, imageWidth, this); - discreteTimeSeries = new DiscreteTimeSeries(pointInfo.getExtendedNameForReport(), pointInfo.getTextRenderer(), + discreteTimeSeries = new DiscreteTimeSeries(seriesIdentifier, pointInfo.getTextRenderer(), colour); point.setDiscreteTimeSeries(discreteTimeSeries); if (pointInfo.isConsolidatedChart()) diff --git a/src/com/serotonin/mango/web/servlet/AsyncImageChartServlet.java b/src/com/serotonin/mango/web/servlet/AsyncImageChartServlet.java index 67a8ac9e98..01734e8d86 100644 --- a/src/com/serotonin/mango/web/servlet/AsyncImageChartServlet.java +++ b/src/com/serotonin/mango/web/servlet/AsyncImageChartServlet.java @@ -198,21 +198,21 @@ public void run() { } int dataType = dp.getPointLocator().getDataTypeId(); + SeriesIdentifier seriesIdentifier = new SeriesIdentifier(dp.getId(), dp.getExtendedName()); if (dataType == DataTypes.NUMERIC) { - SeriesIdentifier seriesIdentifier = new SeriesIdentifier(dp.getId(), dp.getExtendedName()); ts = new TimeSeries(seriesIdentifier, null, null, Second.class); quantizer = new NumericDataQuantizer(from, to, imageWidth, this); loadData(quantizer, pointValueService, dataPointId, from, to); } else if (dataType == DataTypes.MULTISTATE) { quantizer = new MultistateDataQuantizer(from, to, imageWidth, this); - dts = new DiscreteTimeSeries(dp.getName(), dp.getTextRenderer(), colour); + dts = new DiscreteTimeSeries(seriesIdentifier, dp.getTextRenderer(), colour); loadData(quantizer, pointValueService, dataPointId, from, to); } else if (dataType == DataTypes.BINARY) { quantizer = new BinaryDataQuantizer(from, to, imageWidth, this); - dts = new DiscreteTimeSeries(dp.getName(), dp.getTextRenderer(), colour); + dts = new DiscreteTimeSeries(seriesIdentifier, dp.getTextRenderer(), colour); loadData(quantizer, pointValueService, dataPointId, from, to); } } diff --git a/src/com/serotonin/mango/web/servlet/ImageChartServlet.java b/src/com/serotonin/mango/web/servlet/ImageChartServlet.java index 94ef4225c8..1a9c39475e 100644 --- a/src/com/serotonin/mango/web/servlet/ImageChartServlet.java +++ b/src/com/serotonin/mango/web/servlet/ImageChartServlet.java @@ -161,7 +161,8 @@ else if (dp.getPointLocator().getDataTypeId() == DataTypes.NUMERIC) { ptsc.addNumericTimeSeries(ts, colour); } else { - DiscreteTimeSeries ts = new DiscreteTimeSeries(dp.getName(), dp.getTextRenderer(), colour); + SeriesIdentifier seriesIdentifier = new SeriesIdentifier(dp.getId(), dp.getExtendedName()); + DiscreteTimeSeries ts = new DiscreteTimeSeries(seriesIdentifier, dp.getTextRenderer(), colour); for (PointValueTime pv : data) ts.addValueTime(pv); ptsc.addDiscreteTimeSeries(ts);