From e219a04792eb60f48d243db1d0a71b0b038e2b08 Mon Sep 17 00:00:00 2001 From: Spencer Pearson Date: Thu, 9 Apr 2020 12:48:17 -0700 Subject: [PATCH 1/3] add ?from=...&to=... to Grafana --- .../impl/chrome/BaseScreenshotRenderer.java | 8 +++++--- .../impl/chrome/HtmlScreenshotRenderer.java | 2 +- .../impl/chrome/PdfScreenshotRenderer.java | 2 +- .../BaseGrafanaScreenshotRenderer.java | 13 ++++++++++-- .../grafana/PdfGrafanaScreenshotRenderer.java | 5 ----- .../chrome/BaseScreenshotRendererTest.java | 2 +- .../grafana/HtmlScreenshotRendererTest.java | 20 ++++++++++++++++--- .../grafana/PdfScreenshotRendererTest.java | 4 ++-- 8 files changed, 38 insertions(+), 18 deletions(-) diff --git a/app/com/arpnetworking/metrics/portal/reports/impl/chrome/BaseScreenshotRenderer.java b/app/com/arpnetworking/metrics/portal/reports/impl/chrome/BaseScreenshotRenderer.java index cad9b027c..791d90ed2 100644 --- a/app/com/arpnetworking/metrics/portal/reports/impl/chrome/BaseScreenshotRenderer.java +++ b/app/com/arpnetworking/metrics/portal/reports/impl/chrome/BaseScreenshotRenderer.java @@ -28,6 +28,7 @@ import java.net.URI; import java.time.Duration; +import java.time.Instant; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; import java.util.concurrent.ScheduledExecutorService; @@ -56,9 +57,10 @@ public abstract class BaseScreenshotRenderer validateRender(final S source, final F format) { - final URI uri = getUri(source); + final URI uri = getUri(source, new TimeRange(Instant.EPOCH, Instant.EPOCH)); if (!_devToolsFactory.getOriginConfigs().isNavigationAllowed(uri)) { return ImmutableList.of(new Problem.Builder() .setProblemCode("report_problem.DISALLOWED_URI") @@ -107,7 +109,7 @@ public ImmutableList validateRender(final S source, final F format) { .addData("format", format) .addData("timeRange", timeRange) .log(); - final CompletableFuture navigate = dts.navigate(getUri(source).toString()); + final CompletableFuture navigate = dts.navigate(getUri(source, timeRange).toString()); final CompletableFuture result = navigate .thenCompose(whatever -> { LOGGER.debug() diff --git a/app/com/arpnetworking/metrics/portal/reports/impl/chrome/HtmlScreenshotRenderer.java b/app/com/arpnetworking/metrics/portal/reports/impl/chrome/HtmlScreenshotRenderer.java index 735fa129c..0296fe8c5 100644 --- a/app/com/arpnetworking/metrics/portal/reports/impl/chrome/HtmlScreenshotRenderer.java +++ b/app/com/arpnetworking/metrics/portal/reports/impl/chrome/HtmlScreenshotRenderer.java @@ -39,7 +39,7 @@ protected boolean getIgnoreCertificateErrors(final WebPageReportSource source) { } @Override - protected URI getUri(final WebPageReportSource source) { + protected URI getUri(final WebPageReportSource source, final TimeRange timeRange) { return source.getUri(); } diff --git a/app/com/arpnetworking/metrics/portal/reports/impl/chrome/PdfScreenshotRenderer.java b/app/com/arpnetworking/metrics/portal/reports/impl/chrome/PdfScreenshotRenderer.java index 9100838c3..ea74cccf9 100644 --- a/app/com/arpnetworking/metrics/portal/reports/impl/chrome/PdfScreenshotRenderer.java +++ b/app/com/arpnetworking/metrics/portal/reports/impl/chrome/PdfScreenshotRenderer.java @@ -38,7 +38,7 @@ protected boolean getIgnoreCertificateErrors(final WebPageReportSource source) { } @Override - protected URI getUri(final WebPageReportSource source) { + protected URI getUri(final WebPageReportSource source, final TimeRange timeRange) { return source.getUri(); } diff --git a/app/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/BaseGrafanaScreenshotRenderer.java b/app/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/BaseGrafanaScreenshotRenderer.java index a0abaf849..c12723f39 100644 --- a/app/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/BaseGrafanaScreenshotRenderer.java +++ b/app/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/BaseGrafanaScreenshotRenderer.java @@ -16,6 +16,7 @@ package com.arpnetworking.metrics.portal.reports.impl.chrome.grafana; +import com.arpnetworking.metrics.apachehttpsinkextra.shaded.org.apache.http.client.utils.URIBuilder; import com.arpnetworking.metrics.portal.reports.RenderedReport; import com.arpnetworking.metrics.portal.reports.impl.chrome.DevToolsFactory; import com.arpnetworking.metrics.portal.reports.impl.chrome.DevToolsService; @@ -27,6 +28,7 @@ import models.internal.reports.ReportFormat; import java.net.URI; +import java.net.URISyntaxException; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.CompletionStage; @@ -67,8 +69,15 @@ public boolean getIgnoreCertificateErrors(final GrafanaReportPanelReportSource s } @Override - public URI getUri(final GrafanaReportPanelReportSource source) { - return source.getWebPageReportSource().getUri(); + public URI getUri(final GrafanaReportPanelReportSource source, final TimeRange timeRange) { + try { + return new URIBuilder(source.getWebPageReportSource().getUri()) + .addParameter("from", Long.toString(timeRange.getStart().getEpochSecond())) + .addParameter("to", Long.toString(timeRange.getEnd().getEpochSecond())) + .build(); + } catch (URISyntaxException e) { + throw new RuntimeException("should be impossible", e); + } } @Override diff --git a/app/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/PdfGrafanaScreenshotRenderer.java b/app/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/PdfGrafanaScreenshotRenderer.java index 820ecb361..b100168b9 100644 --- a/app/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/PdfGrafanaScreenshotRenderer.java +++ b/app/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/PdfGrafanaScreenshotRenderer.java @@ -39,11 +39,6 @@ public boolean getIgnoreCertificateErrors(final GrafanaReportPanelReportSource s return source.getWebPageReportSource().ignoresCertificateErrors(); } - @Override - public URI getUri(final GrafanaReportPanelReportSource source) { - return source.getWebPageReportSource().getUri(); - } - @Override public > CompletionStage whenReportRendered( final DevToolsService devToolsService, diff --git a/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/BaseScreenshotRendererTest.java b/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/BaseScreenshotRendererTest.java index 1c9c090ac..a151f21a3 100644 --- a/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/BaseScreenshotRendererTest.java +++ b/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/BaseScreenshotRendererTest.java @@ -175,7 +175,7 @@ protected boolean getIgnoreCertificateErrors(final WebPageReportSource source) { } @Override - protected URI getUri(final WebPageReportSource source) { + protected URI getUri(final WebPageReportSource source, final TimeRange timeRange) { return source.getUri(); } diff --git a/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/HtmlScreenshotRendererTest.java b/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/HtmlScreenshotRendererTest.java index 15a95f272..1865a6a98 100644 --- a/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/HtmlScreenshotRendererTest.java +++ b/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/HtmlScreenshotRendererTest.java @@ -20,6 +20,7 @@ import com.arpnetworking.metrics.portal.reports.impl.chrome.grafana.testing.Utils; import com.arpnetworking.metrics.portal.reports.impl.testing.MockRenderedReportBuilder; import com.github.tomakehurst.wiremock.common.Strings; +import models.internal.TimeRange; import models.internal.impl.GrafanaReportPanelReportSource; import models.internal.impl.HtmlReportFormat; import org.junit.Assert; @@ -30,12 +31,13 @@ import java.net.URI; import java.nio.charset.StandardCharsets; import java.time.Duration; +import java.time.Instant; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; import static com.github.tomakehurst.wiremock.client.WireMock.get; -import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static com.github.tomakehurst.wiremock.client.WireMock.urlPathMatching; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -52,7 +54,7 @@ private void runTestWithRenderDelay(final Duration renderDelay) throws Exception final MockRenderedReportBuilder builder = Mockito.mock(MockRenderedReportBuilder.class); _wireMock.givenThat( - get(urlEqualTo("/")) + get(urlPathMatching("/")) .willReturn(aResponse() .withHeader("Content-Type", "text/html") .withBody(Utils.mockGrafanaReportPanelPage(renderDelay, true)) @@ -99,7 +101,7 @@ public void testDelayedRenderingFailure() throws Exception { final MockRenderedReportBuilder builder = Mockito.mock(MockRenderedReportBuilder.class); _wireMock.givenThat( - get(urlEqualTo("/")) + get(urlPathMatching("/")) .willReturn(aResponse() .withHeader("Content-Type", "text/html") .withBody(Utils.mockGrafanaReportPanelPage(Duration.ofSeconds(2), false)) @@ -130,4 +132,16 @@ public void testDelayedRenderingFailure() throws Exception { assertTrue(exc.getCause() instanceof BaseGrafanaScreenshotRenderer.BrowserReportedFailure); } } + + @Test + public void testUriBuilding() throws Exception { + final HtmlGrafanaScreenshotRenderer renderer = new HtmlGrafanaScreenshotRenderer(getDevToolsFactory()); + final GrafanaReportPanelReportSource source = new GrafanaReportPanelReportSource.Builder() + .setWebPageReportSource(TestBeanFactory.createWebPageReportSourceBuilder() + .setUri(URI.create("http://example.com/grafana-page?foo=bar")) + .build()) + .build(); + final URI uri = renderer.getUri(source, new TimeRange(Instant.ofEpochSecond(1234567890L), Instant.ofEpochSecond(9876543210L))); + assertEquals(uri.getQuery(), "foo=bar&from=1234567890&to=9876543210"); + } } diff --git a/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/PdfScreenshotRendererTest.java b/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/PdfScreenshotRendererTest.java index bd9cf1fe1..23385764a 100644 --- a/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/PdfScreenshotRendererTest.java +++ b/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/PdfScreenshotRendererTest.java @@ -31,7 +31,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; import static com.github.tomakehurst.wiremock.client.WireMock.get; -import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static com.github.tomakehurst.wiremock.client.WireMock.urlPathMatching; import static org.junit.Assert.assertTrue; /** @@ -48,7 +48,7 @@ public void testRendering() throws Exception { final MockRenderedReportBuilder builder = Mockito.mock(MockRenderedReportBuilder.class); _wireMock.givenThat( - get(urlEqualTo("/")) + get(urlPathMatching("/")) .willReturn(aResponse() .withHeader("Content-Type", "text/html") .withBody(Utils.mockGrafanaReportPanelPage(Duration.ZERO, true)) From 1575206777ae0f9245c1f6023a71bf550f075146 Mon Sep 17 00:00:00 2001 From: Spencer Pearson Date: Fri, 10 Apr 2020 09:54:27 -0700 Subject: [PATCH 2/3] lint --- .../portal/reports/impl/chrome/BaseScreenshotRenderer.java | 2 +- .../impl/chrome/grafana/BaseGrafanaScreenshotRenderer.java | 2 +- .../impl/chrome/grafana/PdfGrafanaScreenshotRenderer.java | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/com/arpnetworking/metrics/portal/reports/impl/chrome/BaseScreenshotRenderer.java b/app/com/arpnetworking/metrics/portal/reports/impl/chrome/BaseScreenshotRenderer.java index 791d90ed2..640ebe346 100644 --- a/app/com/arpnetworking/metrics/portal/reports/impl/chrome/BaseScreenshotRenderer.java +++ b/app/com/arpnetworking/metrics/portal/reports/impl/chrome/BaseScreenshotRenderer.java @@ -60,7 +60,7 @@ public abstract class BaseScreenshotRenderer Date: Fri, 10 Apr 2020 14:30:33 -0700 Subject: [PATCH 3/3] setParameter instead of addParameter --- .../impl/chrome/grafana/BaseGrafanaScreenshotRenderer.java | 4 ++-- .../impl/chrome/grafana/HtmlScreenshotRendererTest.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/BaseGrafanaScreenshotRenderer.java b/app/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/BaseGrafanaScreenshotRenderer.java index 953924062..ddfb772d9 100644 --- a/app/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/BaseGrafanaScreenshotRenderer.java +++ b/app/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/BaseGrafanaScreenshotRenderer.java @@ -72,8 +72,8 @@ public boolean getIgnoreCertificateErrors(final GrafanaReportPanelReportSource s public URI getUri(final GrafanaReportPanelReportSource source, final TimeRange timeRange) { try { return new URIBuilder(source.getWebPageReportSource().getUri()) - .addParameter("from", Long.toString(timeRange.getStart().getEpochSecond())) - .addParameter("to", Long.toString(timeRange.getEnd().getEpochSecond())) + .setParameter("from", Long.toString(timeRange.getStart().getEpochSecond())) + .setParameter("to", Long.toString(timeRange.getEnd().getEpochSecond())) .build(); } catch (final URISyntaxException e) { throw new RuntimeException("should be impossible", e); diff --git a/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/HtmlScreenshotRendererTest.java b/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/HtmlScreenshotRendererTest.java index 1865a6a98..b57fa8396 100644 --- a/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/HtmlScreenshotRendererTest.java +++ b/test/java/com/arpnetworking/metrics/portal/reports/impl/chrome/grafana/HtmlScreenshotRendererTest.java @@ -138,7 +138,7 @@ public void testUriBuilding() throws Exception { final HtmlGrafanaScreenshotRenderer renderer = new HtmlGrafanaScreenshotRenderer(getDevToolsFactory()); final GrafanaReportPanelReportSource source = new GrafanaReportPanelReportSource.Builder() .setWebPageReportSource(TestBeanFactory.createWebPageReportSourceBuilder() - .setUri(URI.create("http://example.com/grafana-page?foo=bar")) + .setUri(URI.create("http://example.com/grafana-page?foo=bar&from=override-this")) .build()) .build(); final URI uri = renderer.getUri(source, new TimeRange(Instant.ofEpochSecond(1234567890L), Instant.ofEpochSecond(9876543210L)));