-
Notifications
You must be signed in to change notification settings - Fork 1.6k
chore(ui): reduce intial loading with assets via adding compression #25576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
a1d69ca
chore(ui): reduce intial loading with assets via adding compression
chirag-madlani 224ac39
fix: resolve checkstyle and CodeQL security issues
gitar-bot b7eb5bb
enable compressed api response for saving load time
chirag-madlani af9e332
fix: address code review findings in OpenMetadataAssetServlet
gitar-bot f8ba11e
fix build issue
chirag-madlani b130500
Merge branch 'main' into fix-initial-load-time
chirag-madlani 4a1c723
add options to compression
chirag-madlani 4baebd3
Merge branch 'main' into fix-initial-load-time
chirag-madlani File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
187 changes: 187 additions & 0 deletions
187
...a-service/src/test/java/org/openmetadata/service/socket/OpenMetadataAssetServletTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,187 @@ | ||
| package org.openmetadata.service.socket; | ||
|
|
||
| import static org.mockito.ArgumentMatchers.anyString; | ||
| import static org.mockito.ArgumentMatchers.eq; | ||
| import static org.mockito.Mockito.*; | ||
|
|
||
| import jakarta.servlet.ServletContext; | ||
| import jakarta.servlet.ServletOutputStream; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import java.net.URL; | ||
| import java.util.Collections; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.mockito.Mock; | ||
| import org.mockito.MockitoAnnotations; | ||
| import org.openmetadata.service.config.OMWebConfiguration; | ||
|
|
||
| public class OpenMetadataAssetServletTest { | ||
|
|
||
| private OpenMetadataAssetServlet servlet; | ||
|
|
||
| @Mock private HttpServletRequest request; | ||
| @Mock private HttpServletResponse response; | ||
| @Mock private ServletContext servletContext; | ||
| @Mock private OMWebConfiguration webConfiguration; | ||
| @Mock private ServletOutputStream outputStream; | ||
|
|
||
| @BeforeEach | ||
| public void setup() throws Exception { | ||
| MockitoAnnotations.openMocks(this); | ||
| when(request.getServletContext()).thenReturn(servletContext); | ||
| when(response.getOutputStream()).thenReturn(outputStream); | ||
|
|
||
| // Initialize servlet with /assets as resource path | ||
| servlet = new OpenMetadataAssetServlet("/", "/assets", "/", "index.html", webConfiguration); | ||
| } | ||
|
|
||
| @Test | ||
| public void testServeGzipAsset() throws Exception { | ||
| // Setup request for test.js | ||
| String path = "/test.js"; | ||
| when(request.getRequestURI()).thenReturn(path); | ||
| when(request.getContextPath()).thenReturn(""); | ||
| when(request.getPathInfo()).thenReturn(path); | ||
| when(request.getServletPath()).thenReturn(""); | ||
| when(request.getHeader("Accept-Encoding")).thenReturn("gzip, deflate"); | ||
| when(request.getMethod()).thenReturn("GET"); | ||
| when(request.getDateHeader(anyString())).thenReturn(-1L); | ||
| when(request.getHeader("If-None-Match")).thenReturn(null); | ||
| when(request.getHeader("If-Modified-Since")).thenReturn(null); | ||
| when(servletContext.getMimeType(anyString())).thenReturn("application/javascript"); | ||
|
|
||
| try { | ||
| servlet.doGet(request, response); | ||
| } catch (Exception e) { | ||
| e.printStackTrace(); | ||
| throw e; | ||
| } | ||
|
|
||
| // Verify Content-Encoding header is set | ||
| verify(response).setHeader("Content-Encoding", "gzip"); | ||
| // Verify Content-Type is set to javascript (not octet-stream or whatever .gz might imply) | ||
| verify(response).setContentType("application/javascript"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testServeNormalAssetIfGzipMissing() throws Exception { | ||
| // Setup request for normal.js (which has no .gz version) | ||
| String path = "/normal.js"; | ||
| when(request.getRequestURI()).thenReturn(path); | ||
| when(request.getContextPath()).thenReturn(""); | ||
| when(request.getPathInfo()).thenReturn(path); | ||
| when(request.getServletPath()).thenReturn(""); | ||
| when(request.getHeader("Accept-Encoding")).thenReturn("gzip, deflate"); | ||
| when(request.getMethod()).thenReturn("GET"); | ||
| when(request.getDateHeader(anyString())).thenReturn(-1L); | ||
| when(request.getHeader("If-None-Match")).thenReturn(null); | ||
| when(request.getHeader("If-Modified-Since")).thenReturn(null); | ||
|
|
||
| servlet.doGet(request, response); | ||
|
|
||
| // Verify Content-Encoding is NOT set | ||
| verify(response, never()).setHeader(eq("Content-Encoding"), anyString()); | ||
| } | ||
|
|
||
| @Test | ||
| public void testServeNormalAssetIfGzipNotAccepted() throws Exception { | ||
| // Setup request for test.js (which HAS .gz version) | ||
| String path = "/test.js"; | ||
| when(request.getRequestURI()).thenReturn(path); | ||
| when(request.getContextPath()).thenReturn(""); | ||
| when(request.getPathInfo()).thenReturn(path); | ||
| when(request.getServletPath()).thenReturn(""); | ||
| when(request.getHeader("Accept-Encoding")).thenReturn(null); | ||
| when(request.getMethod()).thenReturn("GET"); | ||
| when(request.getDateHeader(anyString())).thenReturn(-1L); | ||
| when(request.getHeader("If-None-Match")).thenReturn(null); | ||
| when(request.getHeader("If-Modified-Since")).thenReturn(null); | ||
|
|
||
| servlet.doGet(request, response); | ||
|
|
||
| // Verify Content-Encoding is NOT set | ||
| verify(response, never()).setHeader(eq("Content-Encoding"), anyString()); | ||
| } | ||
|
|
||
| @Test | ||
| public void testServeBrotliAsset() throws Exception { | ||
| // Setup request for test.js (which has .br version) | ||
| String path = "/test.js"; | ||
| when(request.getRequestURI()).thenReturn(path); | ||
| when(request.getContextPath()).thenReturn(""); | ||
| when(request.getPathInfo()).thenReturn(path); | ||
| when(request.getServletPath()).thenReturn(""); | ||
| when(request.getHeader("Accept-Encoding")).thenReturn("br"); // Only asking for br | ||
| when(request.getMethod()).thenReturn("GET"); | ||
| when(request.getDateHeader(anyString())).thenReturn(-1L); | ||
| when(request.getHeader("If-None-Match")).thenReturn(null); | ||
| when(request.getHeader("If-Modified-Since")).thenReturn(null); | ||
| when(servletContext.getMimeType(anyString())).thenReturn("application/javascript"); | ||
|
|
||
| try { | ||
| servlet.doGet(request, response); | ||
| } catch (Exception e) { | ||
| e.printStackTrace(); | ||
| throw e; | ||
| } | ||
|
|
||
| // Verify Content-Encoding is br | ||
| verify(response).setHeader("Content-Encoding", "br"); | ||
| verify(response).setContentType("application/javascript"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testPrioritizeBrotliOverGzip() throws Exception { | ||
| // Setup request for test.js (which has BOTH .br and .gz) | ||
| String path = "/test.js"; | ||
| when(request.getRequestURI()).thenReturn(path); | ||
| when(request.getContextPath()).thenReturn(""); | ||
| when(request.getPathInfo()).thenReturn(path); | ||
| when(request.getServletPath()).thenReturn(""); | ||
| when(request.getHeader("Accept-Encoding")).thenReturn("gzip, deflate, br"); // Asking for both | ||
| when(request.getMethod()).thenReturn("GET"); | ||
| when(request.getDateHeader(anyString())).thenReturn(-1L); | ||
| when(request.getHeader("If-None-Match")).thenReturn(null); | ||
| when(request.getHeader("If-Modified-Since")).thenReturn(null); | ||
| when(servletContext.getMimeType(anyString())).thenReturn("application/javascript"); | ||
|
|
||
| try { | ||
| servlet.doGet(request, response); | ||
| } catch (Exception e) { | ||
| e.printStackTrace(); | ||
| throw e; | ||
| } | ||
|
|
||
| // Verify Content-Encoding is br (Prioritized) | ||
| verify(response).setHeader("Content-Encoding", "br"); | ||
| verify(response).setContentType("application/javascript"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testFallbackToGzipIfBrotliMissing() throws Exception { | ||
| // Setup request for gzip_only.js (which has .gz but NO .br) | ||
| String path = "/gzip_only.js"; | ||
| when(request.getRequestURI()).thenReturn(path); | ||
| when(request.getContextPath()).thenReturn(""); | ||
| when(request.getPathInfo()).thenReturn(path); | ||
| when(request.getServletPath()).thenReturn(""); | ||
| when(request.getHeader("Accept-Encoding")).thenReturn("gzip, deflate, br"); // Asking for both | ||
| when(request.getMethod()).thenReturn("GET"); | ||
| when(request.getDateHeader(anyString())).thenReturn(-1L); | ||
| when(request.getHeader("If-None-Match")).thenReturn(null); | ||
| when(request.getHeader("If-Modified-Since")).thenReturn(null); | ||
| when(servletContext.getMimeType(anyString())).thenReturn("application/javascript"); | ||
|
|
||
| try { | ||
| servlet.doGet(request, response); | ||
| } catch (Exception e) { | ||
| e.printStackTrace(); | ||
| throw e; | ||
| } | ||
|
|
||
| // Verify Content-Encoding is gzip (Fallback) | ||
| verify(response).setHeader("Content-Encoding", "gzip"); | ||
| verify(response).setContentType("application/javascript"); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| console.log('gzip only'); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| console.log('gzip only compressed'); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| console.log('only normal'); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| console.log('normal'); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| console.log('brotli compressed'); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| console.log('compressed'); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.