Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions app/client/craco.build.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ const plugins = [];
plugins.push(
new WorkboxPlugin.InjectManifest({
swSrc: "./src/serviceWorker.ts",
mode: "development",
mode: "production",
swDest: "./pageService.js",
maximumFileSizeToCacheInBytes: 11 * 1024 * 1024,
exclude: [
// Don’t cache source maps and PWA manifests.
// (These are the default values of the `exclude` option: https://developer.chrome.com/docs/workbox/reference/workbox-build/#type-WebpackPartial,
Expand All @@ -32,9 +31,8 @@ plugins.push(
// one by one (as the service worker does it) keeps the network busy for a long time
// and delays the service worker installation
/\/*\.svg$/,
/\.(js|css|html|png|jpg|jpeg|gif)$/, // Exclude JS, CSS, HTML, and image files
],
// Don’t cache-bust JS and CSS chunks
dontCacheBustURLsMatching: /\.[0-9a-zA-Z]{8}\.chunk\.(js|css)$/,
}),
);

Expand Down
55 changes: 6 additions & 49 deletions app/client/src/serviceWorker.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
import { precacheAndRoute } from "workbox-precaching";
import { clientsClaim, setCacheNameDetails, skipWaiting } from "workbox-core";
import { clientsClaim, skipWaiting } from "workbox-core";
import { registerRoute, Route } from "workbox-routing";
import {
CacheFirst,
NetworkOnly,
StaleWhileRevalidate,
} from "workbox-strategies";
import { NetworkOnly } from "workbox-strategies";
import {
cachedApiUrlRegex,
getApplicationParamsFromUrl,
Expand All @@ -14,32 +9,11 @@ import {
} from "ee/utils/serviceWorkerUtils";
import type { RouteHandlerCallback } from "workbox-core/types";

setCacheNameDetails({
prefix: "appsmith",
suffix: "",
precache: "precache-v1",
runtime: "runtime",
googleAnalytics: "appsmith-ga",
});

const regexMap = {
appViewPage: new RegExp(/api\/v1\/pages\/\w+\/view$/),
static3PAssets: new RegExp(
/(tiny.cloud|googleapis|gstatic|cloudfront).*.(js|css|woff2)/,
),
shims: new RegExp(/shims\/.*.js/),
profile: new RegExp(/v1\/(users\/profile|workspaces)/),
};

/* eslint-disable no-restricted-globals */
// Note: if you need to filter out some files from precaching,
// eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/no-explicit-any
const wbManifest = (self as any).__WB_MANIFEST;

// do that in craco.build.config.js → workbox webpack plugin options
// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const toPrecache = (self as any).__WB_MANIFEST;

precacheAndRoute(toPrecache);
// Delete the old pre-fetch cache. All static files are now cached by cache control headers.
caches.delete("appsmith-precache-v1");

self.__WB_DISABLE_DEV_LOGS = true;
skipWaiting();
Expand Down Expand Up @@ -75,23 +49,6 @@ const htmlRouteHandlerCallback: RouteHandlerCallback = async ({
return networkHandler.handle({ event, request });
};

// This route's caching seems too aggressive.
// TODO(abhinav): Figure out if this is really necessary.
// Maybe add the assets locally?
registerRoute(({ url }) => {
return (
regexMap.shims.test(url.pathname) || regexMap.static3PAssets.test(url.href)
);
}, new CacheFirst());

registerRoute(({ url }) => {
return regexMap.profile.test(url.pathname);
}, new NetworkOnly());

registerRoute(({ url }) => {
return regexMap.appViewPage.test(url.pathname);
}, new StaleWhileRevalidate());

registerRoute(
new Route(({ request, sameOrigin }) => {
return sameOrigin && request.destination === "document";
Expand Down
8 changes: 8 additions & 0 deletions app/client/start-https.sh
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,14 @@ $(if [[ $use_https == 1 ]]; then echo "

location /api {
proxy_pass $backend;

gzip off; # Etag stripped from upstream if gzip is off.
# Ref1: https://forum.nginx.org/read.php?2,242807,242810#msg-242810
# Ref2: https://www.ruby-forum.com/t/reverse-proxy-deleting-etag-header-from-response/246209/2
# Delete the Cache-Control header set in the server block above.
add_header Cache-Control '' always;
# Proxy pass the Cache-Control header from the upstream.
proxy_pass_header Cache-Control;
}

location /oauth2 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,5 @@ public class ConsolidatedApiSpanNamesCE {
public static final String DATASOURCES_SPAN = "datasources";
public static final String FORM_CONFIG_SPAN = "form_config";
public static final String MOCK_DATASOURCES_SPAN = "mock_datasources";
public static final String ETAG_SPAN = CONSOLIDATED_API_PREFIX + VIEW + "compute_etag";
}
5 changes: 5 additions & 0 deletions app/server/appsmith-server/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,11 @@
<version>2.14.2.Final</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.datatype</groupId>
<artifactId>jackson-datatype-jsr310</artifactId>
<version>2.17.0</version>
</dependency>
Comment on lines +415 to +419

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Use property reference for jackson-datatype-jsr310 version

The version should reference the property ${jackson-bom.version} instead of hard-coding it, maintaining consistency with other Jackson dependencies in the project.

 <dependency>
     <groupId>com.fasterxml.jackson.datatype</groupId>
     <artifactId>jackson-datatype-jsr310</artifactId>
-    <version>2.17.0</version>
+    <version>${jackson-bom.version}</version>
 </dependency>
🔗 Analysis chain

Verify jackson-datatype-jsr310 version and management

The addition of this dependency is appropriate for handling Java 8 date/time types in the ETag implementation. However, there are a few considerations:

  1. Version 2.17.0 needs verification as it appears to be a future version
  2. The version should ideally be managed through the properties section for better maintenance

Run this script to check the latest stable version and current usage:

Consider managing the version in properties:

 <properties>
     <jmh.version>1.35</jmh.version>
     <org.modelmapper.version>2.4.4</org.modelmapper.version>
+    <jackson.version>2.17.0</jackson.version>
 </properties>

 <dependency>
     <groupId>com.fasterxml.jackson.datatype</groupId>
     <artifactId>jackson-datatype-jsr310</artifactId>
-    <version>2.17.0</version>
+    <version>${jackson.version}</version>
 </dependency>
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check latest version from Maven Central
curl -s "https://search.maven.org/solrsearch/select?q=g:com.fasterxml.jackson.datatype+AND+a:jackson-datatype-jsr310&rows=1&wt=json" | jq -r '.response.docs[0].latestVersion'

# Check if version is already managed by Spring Boot
rg "jackson.*version" -g "pom.xml"

Length of output: 3399

</dependencies>

<repositories>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@
import com.fasterxml.jackson.annotation.JsonView;
import io.micrometer.observation.ObservationRegistry;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.util.StringUtils;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestHeader;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
Expand All @@ -24,6 +27,7 @@

import static com.appsmith.external.constants.spans.ConsolidatedApiSpanNames.CONSOLIDATED_API_ROOT_EDIT;
import static com.appsmith.external.constants.spans.ConsolidatedApiSpanNames.CONSOLIDATED_API_ROOT_VIEW;
import static org.apache.commons.lang3.StringUtils.isBlank;

@Slf4j
@RestController
Expand Down Expand Up @@ -78,13 +82,13 @@ public Mono<ResponseDTO<ConsolidatedAPIResponseDTO>> getAllDataForFirstPageLoadF

@JsonView(Views.Public.class)
@GetMapping("/view")
public Mono<ResponseDTO<ConsolidatedAPIResponseDTO>> getAllDataForFirstPageLoadForViewMode(
public Mono<ResponseEntity<ResponseDTO<ConsolidatedAPIResponseDTO>>> getAllDataForFirstPageLoadForViewMode(
@RequestParam(required = false) String applicationId,
@RequestParam(required = false) String defaultPageId,
@RequestParam(required = false, defaultValue = "branch") RefType refType,
@RequestParam(required = false) String refName,
@RequestParam(required = false) String branchName) {

@RequestParam(required = false) String branchName,
@RequestHeader(required = false, name = "if-none-match") String ifNoneMatch) {
if (!StringUtils.hasLength(refName)) {
refName = branchName;
}
Expand All @@ -100,8 +104,37 @@ public Mono<ResponseDTO<ConsolidatedAPIResponseDTO>> getAllDataForFirstPageLoadF
return consolidatedAPIService
.getConsolidatedInfoForPageLoad(
defaultPageId, applicationId, refType, refName, ApplicationMode.PUBLISHED)
.map(consolidatedAPIResponseDTO ->
new ResponseDTO<>(HttpStatus.OK.value(), consolidatedAPIResponseDTO, null))
.map(consolidatedAPIResponseDTO -> {
long startTime = System.currentTimeMillis();

String responseHash = consolidatedAPIService.computeConsolidatedAPIResponseEtag(
consolidatedAPIResponseDTO, defaultPageId, applicationId);
long endTime = System.currentTimeMillis();
long duration = endTime - startTime;
log.debug("Time taken to compute ETag: {} ms", duration);

// if defaultPageId and applicationId are both null, then don't compute ETag
if (isBlank(responseHash)) {
ResponseDTO<ConsolidatedAPIResponseDTO> responseDTO =
new ResponseDTO<>(HttpStatus.OK.value(), consolidatedAPIResponseDTO, null);
return new ResponseEntity<>(responseDTO, HttpStatus.OK);
}

HttpHeaders headers = new HttpHeaders();
headers.add("ETag", responseHash);
headers.add("Cache-Control", "private, must-revalidate");

if (ifNoneMatch != null && ifNoneMatch.equals(responseHash)) {
ResponseDTO<ConsolidatedAPIResponseDTO> responseDTO =
new ResponseDTO<>(HttpStatus.NOT_MODIFIED.value(), null, null);
return new ResponseEntity<>(responseDTO, headers, HttpStatus.NOT_MODIFIED);
}

ResponseDTO<ConsolidatedAPIResponseDTO> responseDTO =
new ResponseDTO<>(HttpStatus.OK.value(), consolidatedAPIResponseDTO, null);

return new ResponseEntity<>(responseDTO, headers, HttpStatus.OK);
})
.tag("pageId", Objects.toString(defaultPageId))
.tag("applicationId", Objects.toString(applicationId))
.tag("refType", Objects.toString(refType))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.appsmith.server.services;

import com.appsmith.external.helpers.ObservationHelper;
import com.appsmith.server.actioncollections.base.ActionCollectionService;
import com.appsmith.server.applications.base.ApplicationService;
import com.appsmith.server.datasources.base.DatasourceService;
Expand Down Expand Up @@ -35,7 +36,8 @@ public ConsolidatedAPIServiceImpl(
DatasourceService datasourceService,
MockDataService mockDataService,
ObservationRegistry observationRegistry,
CacheableRepositoryHelper cacheableRepositoryHelper) {
CacheableRepositoryHelper cacheableRepositoryHelper,
ObservationHelper observationHelper) {
super(
sessionUserService,
userService,
Expand All @@ -53,6 +55,7 @@ public ConsolidatedAPIServiceImpl(
datasourceService,
mockDataService,
observationRegistry,
cacheableRepositoryHelper);
cacheableRepositoryHelper,
observationHelper);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,7 @@ public interface ConsolidatedAPIServiceCE {

Mono<ConsolidatedAPIResponseDTO> getConsolidatedInfoForPageLoad(
String defaultPageId, String applicationId, RefType refType, String refName, ApplicationMode mode);

String computeConsolidatedAPIResponseEtag(
ConsolidatedAPIResponseDTO consolidatedAPIResponseDTO, String defaultPageId, String applicationId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.appsmith.external.exceptions.ErrorDTO;
import com.appsmith.external.git.constants.ce.RefType;
import com.appsmith.external.helpers.ObservationHelper;
import com.appsmith.external.models.CreatorContextType;
import com.appsmith.external.models.Datasource;
import com.appsmith.server.actioncollections.base.ActionCollectionService;
Expand Down Expand Up @@ -32,9 +33,13 @@
import com.appsmith.server.services.UserDataService;
import com.appsmith.server.services.UserService;
import com.appsmith.server.themes.base.ThemeService;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
import io.micrometer.observation.ObservationRegistry;
import io.micrometer.tracing.Span;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.jetbrains.annotations.NotNull;
import org.springframework.data.util.Pair;
import org.springframework.http.HttpStatus;
import org.springframework.stereotype.Service;
Expand All @@ -46,7 +51,11 @@
import reactor.core.publisher.Mono;
import reactor.util.function.Tuple2;

import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.util.ArrayList;
import java.util.Base64;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand All @@ -64,6 +73,7 @@
import static com.appsmith.external.constants.spans.ConsolidatedApiSpanNames.CURRENT_THEME_SPAN;
import static com.appsmith.external.constants.spans.ConsolidatedApiSpanNames.CUSTOM_JS_LIB_SPAN;
import static com.appsmith.external.constants.spans.ConsolidatedApiSpanNames.DATASOURCES_SPAN;
import static com.appsmith.external.constants.spans.ConsolidatedApiSpanNames.ETAG_SPAN;
import static com.appsmith.external.constants.spans.ConsolidatedApiSpanNames.FEATURE_FLAG_SPAN;
import static com.appsmith.external.constants.spans.ConsolidatedApiSpanNames.FORM_CONFIG_SPAN;
import static com.appsmith.external.constants.spans.ConsolidatedApiSpanNames.MOCK_DATASOURCES_SPAN;
Expand Down Expand Up @@ -106,6 +116,7 @@ public class ConsolidatedAPIServiceCEImpl implements ConsolidatedAPIServiceCE {
private final MockDataService mockDataService;
private final ObservationRegistry observationRegistry;
private final CacheableRepositoryHelper cacheableRepositoryHelper;
private final ObservationHelper observationHelper;

protected <T> ResponseDTO<T> getSuccessResponse(T data) {
return new ResponseDTO<>(HttpStatus.OK.value(), data, null);
Expand Down Expand Up @@ -633,4 +644,60 @@ protected Mono<Tuple2<Application, NewPage>> getApplicationAndPageTupleMono(
private boolean isPossibleToCreateQueryWithoutDatasource(Plugin plugin) {
return PLUGINS_THAT_ALLOW_QUERY_CREATION_WITHOUT_DATASOURCE.contains(plugin.getPackageName());
}

@NotNull public String computeConsolidatedAPIResponseEtag(
ConsolidatedAPIResponseDTO consolidatedAPIResponseDTO, String defaultPageId, String applicationId) {
if (isBlank(defaultPageId) && isBlank(applicationId)) {
return "";
}

Span computeEtagSpan = observationHelper.createSpan(ETAG_SPAN).start();

try {
String lastDeployedAt = consolidatedAPIResponseDTO.getPages() != null
? consolidatedAPIResponseDTO
.getPages()
.getData()
.getApplication()
.getLastDeployedAt()
.toString()
: null;

if (lastDeployedAt == null) {
return "";
}

Object currentTheme = consolidatedAPIResponseDTO.getCurrentTheme() != null
? consolidatedAPIResponseDTO.getCurrentTheme()
: "";
Object themes = consolidatedAPIResponseDTO.getThemes() != null
? consolidatedAPIResponseDTO.getThemes()
: Collections.emptyList();

Map<String, Object> consolidateAPISignature = Map.of(
"userProfile", consolidatedAPIResponseDTO.getUserProfile(),
"featureFlags", consolidatedAPIResponseDTO.getFeatureFlags(),
"tenantConfig", consolidatedAPIResponseDTO.getTenantConfig(),
"productAlert", consolidatedAPIResponseDTO.getProductAlert(),
"currentTheme", currentTheme,
"themes", themes,
"lastDeployedAt", lastDeployedAt);

ObjectMapper objectMapper = new ObjectMapper();
objectMapper.registerModule(new JavaTimeModule());

String consolidateAPISignatureJSON = objectMapper.writeValueAsString(consolidateAPISignature);

MessageDigest digest = MessageDigest.getInstance("SHA-256");
byte[] hashBytes = digest.digest(consolidateAPISignatureJSON.getBytes(StandardCharsets.UTF_8));
String etag = Base64.getEncoder().encodeToString(hashBytes);

return etag;
} catch (Exception e) {
log.error("Error while computing etag for ConsolidatedAPIResponseDTO", e);
return "";
} finally {
observationHelper.endSpan(computeEtagSpan, true);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.appsmith.server.services.ce_compatible;

import com.appsmith.external.helpers.ObservationHelper;
import com.appsmith.server.actioncollections.base.ActionCollectionService;
import com.appsmith.server.applications.base.ApplicationService;
import com.appsmith.server.datasources.base.DatasourceService;
Expand Down Expand Up @@ -38,7 +39,8 @@ public ConsolidatedAPIServiceCECompatibleImpl(
DatasourceService datasourceService,
MockDataService mockDataService,
ObservationRegistry observationRegistry,
CacheableRepositoryHelper cacheableRepositoryHelper) {
CacheableRepositoryHelper cacheableRepositoryHelper,
ObservationHelper observationHelper) {
super(
sessionUserService,
userService,
Expand All @@ -56,6 +58,7 @@ public ConsolidatedAPIServiceCECompatibleImpl(
datasourceService,
mockDataService,
observationRegistry,
cacheableRepositoryHelper);
cacheableRepositoryHelper,
observationHelper);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ github_repo = ${APPSMITH_GITHUB_REPO:}
# The following configurations are to help support prometheus scraping for monitoring
management.endpoints.web.exposure.include=prometheus,metrics
management.tracing.enabled=${APPSMITH_TRACING_ENABLED:false}
management.otlp.tracing.endpoint=${APPSMITH_NEW_RELIC_OTEL_EXPORTER_OTLP_ENDPOINT:http://localhost:4318}/v1/traces
management.otlp.tracing.endpoint=${APPSMITH_NEW_RELIC_OTEL_EXPORTER_OTLP_ENDPOINT:https://otlp.nr-data.net:4317}/v1/traces
management.otlp.tracing.headers.api-key=${APPSMITH_NEW_RELIC_OTLP_LICENSE_KEY:}
management.opentelemetry.resource-attributes.service.name=appsmith-server
management.opentelemetry.resource-attributes.service.instance.id=${HOSTNAME:appsmith-0}
Expand Down
Loading