Skip to content

Commit 9612316

Browse files
fmontesclaude
andcommitted
fix: address code review issues in REST endpoint
- Fix site identification using WebAPILocator instead of getServerName() - Add @consumes(APPLICATION_JSON) annotation to POST endpoint - Return generic error message instead of leaking exception details - Add null check for request body before processing - Cache GoogleAnalyticsService per site to avoid recreating on each request Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4f77477 commit 9612316

1 file changed

Lines changed: 25 additions & 6 deletions

File tree

src/main/java/com/dotcms/google/analytics/rest/GoogleAnalyticsResource.java

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
package com.dotcms.google.analytics.rest;
22

3-
import com.dotcms.google.analytics.app.AnalyticsApp;
43
import com.dotcms.google.analytics.app.AnalyticsAppService;
54
import com.dotcms.google.analytics.model.AnalyticsRequest;
65
import com.dotcms.google.analytics.model.FilterRequest;
76
import com.dotcms.google.analytics.service.GoogleAnalyticsService;
87
import com.dotcms.rest.WebResource;
8+
import com.dotmarketing.beans.Host;
9+
import com.dotmarketing.business.web.WebAPILocator;
910
import com.dotmarketing.util.Logger;
1011
import com.google.analytics.data.v1beta.DimensionValue;
1112
import com.google.analytics.data.v1beta.MetricValue;
@@ -14,6 +15,7 @@
1415

1516
import javax.servlet.http.HttpServletRequest;
1617
import javax.servlet.http.HttpServletResponse;
18+
import javax.ws.rs.Consumes;
1719
import javax.ws.rs.POST;
1820
import javax.ws.rs.Path;
1921
import javax.ws.rs.Produces;
@@ -24,6 +26,7 @@
2426
import java.util.HashMap;
2527
import java.util.List;
2628
import java.util.Map;
29+
import java.util.concurrent.ConcurrentHashMap;
2730
import java.util.stream.Collectors;
2831

2932
/**
@@ -36,6 +39,7 @@ public class GoogleAnalyticsResource {
3639

3740
private final WebResource webResource = new WebResource();
3841
private final AnalyticsAppService analyticsAppService = new AnalyticsAppService();
42+
private final Map<String, GoogleAnalyticsService> googleAnalyticsServiceMap = new ConcurrentHashMap<>();
3943

4044
/**
4145
* Query Google Analytics 4 data via REST API.
@@ -58,13 +62,21 @@ public class GoogleAnalyticsResource {
5862
*/
5963
@POST
6064
@Path("/query")
65+
@Consumes(MediaType.APPLICATION_JSON)
6166
@Produces(MediaType.APPLICATION_JSON)
6267
public Response query(
6368
@Context final HttpServletRequest request,
6469
@Context final HttpServletResponse response,
6570
final GoogleAnalyticsQueryRequest queryRequest) {
6671

6772
try {
73+
// Validate request body
74+
if (queryRequest == null) {
75+
return Response.status(Response.Status.BAD_REQUEST)
76+
.entity(Map.of("error", "Request body is required"))
77+
.build();
78+
}
79+
6880
// Authenticate user
6981
final User user = new WebResource.InitBuilder(webResource)
7082
.requiredBackendUser(true)
@@ -84,12 +96,19 @@ public Response query(
8496
}
8597

8698
// Get analytics app configuration for current site
87-
final String siteId = request.getServerName(); // Or extract from request
88-
final AnalyticsApp analyticsApp = analyticsAppService.getAnalyticsApp(siteId);
99+
final Host currentHost = WebAPILocator.getHostWebAPI().getHost(request);
100+
final String siteId = currentHost.getIdentifier();
89101

90-
// Create Google Analytics service
102+
// Get or create cached Google Analytics service
91103
final GoogleAnalyticsService analyticsService =
92-
new GoogleAnalyticsService(analyticsApp.getJsonKeyFile());
104+
this.googleAnalyticsServiceMap.computeIfAbsent(siteId, key -> {
105+
try {
106+
final AnalyticsApp analyticsApp = analyticsAppService.getAnalyticsApp(siteId);
107+
return new GoogleAnalyticsService(analyticsApp.getJsonKeyFile());
108+
} catch (Exception e) {
109+
throw new RuntimeException(e);
110+
}
111+
});
93112

94113
// Build analytics request
95114
final AnalyticsRequest analyticsRequest =
@@ -203,7 +222,7 @@ public Response query(
203222
} catch (Exception e) {
204223
Logger.error(this, "Error querying Google Analytics", e);
205224
return Response.status(Response.Status.INTERNAL_SERVER_ERROR)
206-
.entity(Map.of("error", e.getMessage()))
225+
.entity(Map.of("error", "Error querying Google Analytics"))
207226
.build();
208227
}
209228
}

0 commit comments

Comments
 (0)