partition id validations in coral and legacy

Signed-off-by: khatibtamal <[email protected]>
Aiven-Open · Oct 3, 2024 · 129a4ff · 129a4ff
1 parent 961df4b
commit 129a4ff
Show file tree

Hide file tree

Showing 5 changed files with 109 additions and 22 deletions.
diff --git a/coral/src/app/features/topics/details/messages/TopicMessages.test.tsx b/coral/src/app/features/topics/details/messages/TopicMessages.test.tsx
@@ -12,6 +12,12 @@ const mockGetTopicMessages = getTopicMessages as jest.MockedFunction<
   typeof getTopicMessages
 >;
 
+const mockTopicOverview = {
+  topicInfo: {
+    noOfPartitions: 5,
+  },
+};
+
 const mockGetTopicMessagesResponse = {
   0: "HELLO",
   1: "WORLD",
@@ -24,7 +30,15 @@ const mockGetTopicMessagesNoContentResponse = {
 const selectModeOptions = ["Default", "Custom", "Range"];
 
 function DummyParent() {
-  return <Outlet context={{ topicName: "test", environmentId: "2" }} />;
+  return (
+    <Outlet
+      context={{
+        topicName: "test",
+        environmentId: "2",
+        topicOverview: mockTopicOverview,
+      }}
+    />
+  );
 }
 
 describe("TopicMessages", () => {

diff --git a/coral/src/app/features/topics/details/messages/TopicMessages.tsx b/coral/src/app/features/topics/details/messages/TopicMessages.tsx
@@ -32,7 +32,8 @@ function isNoContentResult(
 }
 
 function TopicMessages() {
-  const { topicName, environmentId } = useTopicDetails();
+  const { topicName, environmentId, topicOverview } = useTopicDetails();
+  const numberOfPartitions = topicOverview.topicInfo.noOfPartitions;
 
   const {
     validateFilters,
@@ -76,7 +77,7 @@ function TopicMessages() {
   const isConsuming = isInitialLoading || isRefetching;
 
   function handleUpdateResultClick(): void {
-    const isValid = validateFilters();
+    const isValid = validateFilters(numberOfPartitions);
 
     if (isValid) {
       updateResults();

diff --git a/coral/src/app/features/topics/details/messages/useMessagesFilters.test.tsx b/coral/src/app/features/topics/details/messages/useMessagesFilters.test.tsx
@@ -189,7 +189,7 @@ describe("useMessagesFilters.tsx", () => {
       let isValid;
 
       act(() => {
-        isValid = result.current.validateFilters();
+        isValid = result.current.validateFilters(5);
       });
 
       expect(isValid).toBe(true);
@@ -214,7 +214,7 @@ describe("useMessagesFilters.tsx", () => {
       let isValid;
 
       act(() => {
-        isValid = result.current.validateFilters();
+        isValid = result.current.validateFilters(5);
       });
 
       expect(isValid).toBe(false);
@@ -226,12 +226,67 @@ describe("useMessagesFilters.tsx", () => {
         rangeOffsetEndFilters: null,
       });
     });
+    it("validateFilters returns false (negative partitionId)", () => {
+      const { result } = renderHook(() => useMessagesFilters(), {
+        wrapper: ({ children }) => (
+          <MemoryRouter
+            initialEntries={[
+              "/?defaultOffset=custom&customOffset=100&partitionId=-1",
+            ]}
+          >
+            {children}
+          </MemoryRouter>
+        ),
+      });
+
+      let isValid;
+
+      act(() => {
+        isValid = result.current.validateFilters(5);
+      });
+
+      expect(isValid).toBe(false);
+
+      expect(result.current.filterErrors).toStrictEqual({
+        customOffsetFilters: null,
+        partitionIdFilters: "Partition ID cannot be negative",
+        rangeOffsetStartFilters: null,
+        rangeOffsetEndFilters: null,
+      });
+    });
+    it("validateFilters returns false (invalid partitionId)", () => {
+      const { result } = renderHook(() => useMessagesFilters(), {
+        wrapper: ({ children }) => (
+          <MemoryRouter
+            initialEntries={[
+              "/?defaultOffset=custom&customOffset=100&partitionId=6",
+            ]}
+          >
+            {children}
+          </MemoryRouter>
+        ),
+      });
+
+      let isValid;
 
+      act(() => {
+        isValid = result.current.validateFilters(5);
+      });
+
+      expect(isValid).toBe(false);
+
+      expect(result.current.filterErrors).toStrictEqual({
+        customOffsetFilters: null,
+        partitionIdFilters: "Invalid partition ID",
+        rangeOffsetStartFilters: null,
+        rangeOffsetEndFilters: null,
+      });
+    });
     it("validateFilters returns false (missing customOffset)", () => {
       const { result } = renderHook(() => useMessagesFilters(), {
         wrapper: ({ children }) => (
           <MemoryRouter
-            initialEntries={["/?defaultOffset=custom&partitionId=100"]}
+            initialEntries={["/?defaultOffset=custom&partitionId=1"]}
           >
             {children}
           </MemoryRouter>
@@ -241,7 +296,7 @@ describe("useMessagesFilters.tsx", () => {
       let isValid;
 
       act(() => {
-        isValid = result.current.validateFilters();
+        isValid = result.current.validateFilters(5);
       });
 
       expect(isValid).toBe(false);
@@ -259,7 +314,7 @@ describe("useMessagesFilters.tsx", () => {
         wrapper: ({ children }) => (
           <MemoryRouter
             initialEntries={[
-              "/?defaultOffset=custom&customOffset=9999&partitionId=100",
+              "/?defaultOffset=custom&customOffset=9999&partitionId=1",
             ]}
           >
             {children}
@@ -270,7 +325,7 @@ describe("useMessagesFilters.tsx", () => {
       let isValid;
 
       act(() => {
-        isValid = result.current.validateFilters();
+        isValid = result.current.validateFilters(5);
       });
 
       expect(isValid).toBe(false);
@@ -433,7 +488,7 @@ describe("useMessagesFilters.tsx", () => {
       let isValid;
 
       act(() => {
-        isValid = result.current.validateFilters();
+        isValid = result.current.validateFilters(5);
       });
 
       expect(isValid).toBe(true);
@@ -460,7 +515,7 @@ describe("useMessagesFilters.tsx", () => {
       let isValid;
 
       act(() => {
-        isValid = result.current.validateFilters();
+        isValid = result.current.validateFilters(5);
       });
 
       expect(isValid).toBe(false);
@@ -486,7 +541,7 @@ describe("useMessagesFilters.tsx", () => {
       let isValid;
 
       act(() => {
-        isValid = result.current.validateFilters();
+        isValid = result.current.validateFilters(5);
       });
 
       expect(isValid).toBe(false);
@@ -514,7 +569,7 @@ describe("useMessagesFilters.tsx", () => {
       let isValid;
 
       act(() => {
-        isValid = result.current.validateFilters();
+        isValid = result.current.validateFilters(5);
       });
 
       expect(isValid).toBe(false);
@@ -542,7 +597,7 @@ describe("useMessagesFilters.tsx", () => {
       let isValid;
 
       act(() => {
-        isValid = result.current.validateFilters();
+        isValid = result.current.validateFilters(5);
       });
 
       expect(isValid).toBe(false);

diff --git a/coral/src/app/features/topics/details/messages/useMessagesFilters.tsx b/coral/src/app/features/topics/details/messages/useMessagesFilters.tsx
@@ -24,7 +24,7 @@ const NAMES = {
 const initialDefaultOffset: (typeof defaultOffsets)[0] = "5";
 
 interface OffsetFilters {
-  validateFilters: () => boolean;
+  validateFilters: (totalNumberOfPartitions: number) => boolean;
   filterErrors: FilterErrors;
   getFetchingMode: () => TopicMessagesFetchModeTypes;
   defaultOffsetFilters: {
@@ -70,15 +70,19 @@ function useMessagesFilters(): OffsetFilters {
     rangeOffsetEndFilters: null,
   });
 
-  function validateFilters() {
+  function validateFilters(totalNumberOfPartitions: number) {
     if (getFetchingMode() === "default") {
       return true;
     }
 
     const partitionIdFiltersError =
       getPartitionId() === "" || getPartitionId() === null
         ? "Please enter a partition ID"
-        : null;
+        : Number(getPartitionId()) < 0
+          ? "Partition ID cannot be negative"
+          : Number(getPartitionId()) >= totalNumberOfPartitions
+            ? "Invalid partition ID"
+            : null;
 
     let customOffsetFiltersError = null;
     let rangeOffsetStartFiltersError = null;
@@ -269,7 +273,7 @@ function useMessagesFilters(): OffsetFilters {
   }
 
   function setPartitionId(partitionId: string): void {
-    if (getDefaultOffset() !== "custom") {
+    if (getDefaultOffset() !== "custom" && getDefaultOffset() !== "range") {
       setDefaultOffset("custom");
     }
     if (partitionId.length === 0) {

diff --git a/core/src/main/resources/static/js/browseAcls.js b/core/src/main/resources/static/js/browseAcls.js
@@ -898,6 +898,13 @@ app.controller("browseAclsCtrl", function($scope, $http, $location, $window) {
                         return;
                     }
 
+                    if($scope.selectedPartitionId >= $scope.topicOverview[0].noOfPartitions)
+                    {
+                        $scope.alert = "Please fill in a valid partition id.";
+                        $scope.showAlertToast();
+                        return;
+                    }
+
                     if(!$scope.selectedNumberOfOffsets || $scope.selectedNumberOfOffsets === ""){
                         $scope.alert = "Please fill how many events/offsets to be displayed";
                         $scope.showAlertToast();
@@ -928,16 +935,22 @@ app.controller("browseAclsCtrl", function($scope, $http, $location, $window) {
                          return;
                      }
 
-                     if(!$scope.selectedOffsetRangeStart || $scope.selectedOffsetRangeStart === "" ||
-                        !$scope.selectedOffsetRangeEnd || $scope.selectedOffsetRangeEnd === ""
+                    if($scope.selectedPartitionId >= $scope.topicOverview[0].noOfPartitions)
+                    {
+                        $scope.alert = "Please fill in a valid partition id.";
+                        $scope.showAlertToast();
+                        return;
+                    }
+
+                     if($scope.selectedOffsetRangeStart === "" || $scope.selectedOffsetRangeEnd === ""
                      ){
                          $scope.alert = "Please fill how many offsets range start and end.";
                          $scope.showAlertToast();
                          return;
                      }
 
-                     if($scope.selectedOffsetRangeStart <= 0 || isNaN($scope.selectedOffsetRangeStart) ||
-                     $scope.selectedOffsetRangeEnd <= 0 || isNaN($scope.selectedOffsetRangeEnd))
+                     if($scope.selectedOffsetRangeStart < 0 || isNaN($scope.selectedOffsetRangeStart) ||
+                     $scope.selectedOffsetRangeEnd < 0 || isNaN($scope.selectedOffsetRangeEnd))
                      {
                          $scope.alert = "Please fill in a valid number topic offsets start and end.";
                          $scope.showAlertToast();