Skip to content
Merged
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,171 @@ public void testFindPublicNamespaceForAssociatedNamespace() {
assertThat(namespaceKey2).isEqualTo(Arrays.asList("k1", "k2", "k3"));
}

@Test
public void testLoadNamespaceBOWithDeletedItems() {
ReleaseDTO releaseDTO = createReleaseDTO();
when(releaseService.loadLatestRelease(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(releaseDTO);

List<ItemDTO> itemDTOList = createItems();
when(itemService.findItems(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(itemDTOList);

List<ItemDTO> deletedItemDTOList = Lists.newArrayList();
ItemDTO deletedItemDTO = new ItemDTO();
deletedItemDTO.setKey("deleted-key");
deletedItemDTOList.add(deletedItemDTO);
when(itemService.findDeletedItems(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(deletedItemDTOList);

when(namespaceAPI.loadNamespace(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(createNamespace(testAppId, testClusterName, testNamespaceName));
when(appNamespaceService.findByAppIdAndName(testAppId, testNamespaceName)).thenReturn(createAppNamespace(testAppId, testNamespaceName, false));

NamespaceBO namespaceBO = namespaceService.loadNamespaceBO(testAppId, testEnv, testClusterName, testNamespaceName, true);

assertNotNull(namespaceBO);
assertEquals(testAppId, namespaceBO.getBaseInfo().getAppId());
assertEquals(testClusterName, namespaceBO.getBaseInfo().getClusterName());
assertEquals(testNamespaceName, namespaceBO.getBaseInfo().getNamespaceName());
assertEquals(3, namespaceBO.getItems().size());
verify(itemService, times(1)).findDeletedItems(testAppId, testEnv, testClusterName, testNamespaceName);
verify(additionalUserInfoEnrichService, times(1)).enrichAdditionalUserInfo(any(), any());
}

@Test
public void testLoadNamespaceBOWithoutDeletedItems() {
ReleaseDTO releaseDTO = createReleaseDTO();
when(releaseService.loadLatestRelease(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(releaseDTO);

List<ItemDTO> itemDTOList = createItems();
when(itemService.findItems(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(itemDTOList);

when(namespaceAPI.loadNamespace(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(createNamespace(testAppId, testClusterName, testNamespaceName));
when(appNamespaceService.findByAppIdAndName(testAppId, testNamespaceName)).thenReturn(createAppNamespace(testAppId, testNamespaceName, false));

NamespaceBO namespaceBO = namespaceService.loadNamespaceBO(testAppId, testEnv, testClusterName, testNamespaceName, false);

assertNotNull(namespaceBO);
assertEquals(testAppId, namespaceBO.getBaseInfo().getAppId());
assertEquals(testClusterName, namespaceBO.getBaseInfo().getClusterName());
assertEquals(testNamespaceName, namespaceBO.getBaseInfo().getNamespaceName());
assertEquals(2, namespaceBO.getItems().size());
verify(itemService, times(0)).findDeletedItems(any(), any(), any(), any());
verify(additionalUserInfoEnrichService, times(1)).enrichAdditionalUserInfo(any(), any());
}

@Test
public void testLoadNamespaceBONamespaceNotFound() {
when(namespaceAPI.loadNamespace(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(null);

assertThatExceptionOfType(BadRequestException.class)
.isThrownBy(() -> namespaceService.loadNamespaceBO(testAppId, testEnv, testClusterName, testNamespaceName));
}

@Test
public void testLoadNamespaceBONoLatestRelease() {
when(namespaceAPI.loadNamespace(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(createNamespace(testAppId, testClusterName, testNamespaceName));
when(releaseService.loadLatestRelease(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(null);
when(appNamespaceService.findByAppIdAndName(testAppId, testNamespaceName)).thenReturn(createAppNamespace(testAppId, testNamespaceName, false));
when(itemService.findItems(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(createItems());

NamespaceBO namespaceBO = namespaceService.loadNamespaceBO(testAppId, testEnv, testClusterName, testNamespaceName);
Comment on lines +359 to +365
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Stub deleted items or disable them in these tests.

These methods call the 4-arg loadNamespaceBO(...), which defaults includeDeletedItems to true. transformNamespace2BO(...) then does itemService.findDeletedItems(...).stream(), so the unstubbed mock returns null and these tests fail before the assertions run.

🔧 Minimal fix
-    NamespaceBO namespaceBO = namespaceService.loadNamespaceBO(testAppId, testEnv, testClusterName, testNamespaceName);
+    NamespaceBO namespaceBO =
+        namespaceService.loadNamespaceBO(
+            testAppId, testEnv, testClusterName, testNamespaceName, true, false);

Apply the same change in:

  • testLoadNamespaceBONoLatestRelease
  • testLoadNamespaceBOWithPublicNamespace
  • testLoadNamespaceBOWithPrivateNamespace
  • testLoadNamespaceBOWithDirtyAppNamespace

If you want to keep exercising the default overload instead, stub findDeletedItems(...) to return Collections.emptyList() in each test.

Also applies to: 394-405, 413-438

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/service/NamespaceServiceTest.java`
around lines 359 - 365, The tests (testLoadNamespaceBONoLatestRelease,
testLoadNamespaceBOWithPublicNamespace, testLoadNamespaceBOWithPrivateNamespace,
testLoadNamespaceBOWithDirtyAppNamespace) call
NamespaceService.loadNamespaceBO(...) which by default includes deleted items
and causes transformNamespace2BO(...) to call itemService.findDeletedItems(...);
stub that interaction in each affected test to return Collections.emptyList()
(or disable deleted-items behavior) so the mock does not return null; update the
mocked behavior for itemService.findDeletedItems(testAppId, testEnv,
testClusterName, testNamespaceName) in those tests to return an empty list.


assertNotNull(namespaceBO);
assertEquals(2, namespaceBO.getItems().size());
assertTrue(namespaceBO.getItems().get(0).isModified());
assertTrue(namespaceBO.getItems().get(1).isModified());
}

@Test
public void testLoadNamespaceBONoItems() {
ReleaseDTO releaseDTO = createReleaseDTO();
when(releaseService.loadLatestRelease(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(releaseDTO);
when(namespaceAPI.loadNamespace(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(createNamespace(testAppId, testClusterName, testNamespaceName));
when(itemService.findItems(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(Lists.newArrayList());
when(appNamespaceService.findByAppIdAndName(testAppId, testNamespaceName)).thenReturn(createAppNamespace(testAppId, testNamespaceName, false));

ItemDTO deletedItemDTO = new ItemDTO();
deletedItemDTO.setKey("deleted-key");
when(itemService.findDeletedItems(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(Lists.newArrayList(deletedItemDTO));

NamespaceBO namespaceBO = namespaceService.loadNamespaceBO(testAppId, testEnv, testClusterName, testNamespaceName);

assertNotNull(namespaceBO);
assertEquals(3, namespaceBO.getItems().size());
assertTrue(namespaceBO.getItems().get(0).isDeleted());
assertEquals("k1", namespaceBO.getItems().get(0).getItem().getKey());
}
Comment on lines +405 to +409
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fragile assertion depends on non-deterministic iteration order.

The assertion at line 386 (assertEquals("k1", namespaceBO.getItems().get(0).getItem().getKey())) assumes k1 is the first deleted item. However, parseDeletedItems iterates over releaseItems.entrySet() (a HashMap), whose iteration order is not guaranteed. This test may pass or fail depending on JVM/runtime behavior.

🔧 Proposed fix to use order-independent assertion
     assertNotNull(namespaceBO);
     assertEquals(3, namespaceBO.getItems().size());
-    assertTrue(namespaceBO.getItems().get(0).isDeleted());
-    assertEquals("k1", namespaceBO.getItems().get(0).getItem().getKey());
+    // All items should be marked as deleted since items list is empty
+    assertTrue(namespaceBO.getItems().stream().allMatch(item -> item.isDeleted()));
+    // Verify expected keys are present (order is non-deterministic)
+    List<String> keys = namespaceBO.getItems().stream()
+        .map(item -> item.getItem().getKey())
+        .collect(Collectors.toList());
+    assertThat(keys).asList().containsExactlyInAnyOrder("k1", "k2", "k3");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
assertNotNull(namespaceBO);
assertEquals(3, namespaceBO.getItems().size());
assertTrue(namespaceBO.getItems().get(0).isDeleted());
assertEquals("k1", namespaceBO.getItems().get(0).getItem().getKey());
}
assertNotNull(namespaceBO);
assertEquals(3, namespaceBO.getItems().size());
// All items should be marked as deleted since items list is empty
assertTrue(namespaceBO.getItems().stream().allMatch(item -> item.isDeleted()));
// Verify expected keys are present (order is non-deterministic)
Set<String> keys = namespaceBO.getItems().stream()
.map(item -> item.getItem().getKey())
.collect(Collectors.toSet());
assertEquals(Set.of("k1", "k2", "k3"), keys);
}
🧰 Tools
🪛 GitHub Actions: build

[error] 383-383: Compilation error: cannot find symbol method assertNotNull(com.ctrip.framework.apollo.portal.entity.bo.NamespaceBO).


[error] 385-385: Compilation error: cannot find symbol method assertTrue(boolean).

🪛 GitHub Actions: portal-login-e2e

[error] 383-383: Cannot find symbol: method assertNotNull(NamespaceBO).


[error] 385-385: Cannot find symbol: method assertTrue(boolean).

🪛 GitHub Actions: portal-ui-e2e

[error] 385-385: Cannot resolve symbol: NamespaceBO in assertion context (NamespaceServiceTest).


[error] 385-385: Cannot resolve symbol: assertTrue(boolean) in test code.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/service/NamespaceServiceTest.java`
around lines 383 - 387, The test in NamespaceServiceTest assumes a deterministic
order when checking namespaceBO.getItems().get(0).getItem().getKey(); instead,
make the assertion order-independent by inspecting the collection produced by
parseDeletedItems/releaseItems.entrySet(): for example, assert that
namespaceBO.getItems() contains an element whose item.getKey() equals "k1" and
isDeleted() is true (or extract keys into a Set and assert it contains "k1"), or
sort namespaceBO.getItems() by item.getKey() before asserting the first element;
update the assertion that currently references namespaceBO.getItems().get(0)
accordingly.


@Test
public void testLoadNamespaceBOWithPublicNamespace() {
AppNamespace publicAppNamespace = createAppNamespace("public-app", testNamespaceName, true);
when(namespaceAPI.loadNamespace(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(createNamespace(testAppId, testClusterName, testNamespaceName));
when(appNamespaceService.findByAppIdAndName(testAppId, testNamespaceName)).thenReturn(null);
when(appNamespaceService.findPublicAppNamespace(testNamespaceName)).thenReturn(publicAppNamespace);

ReleaseDTO releaseDTO = createReleaseDTO();
when(releaseService.loadLatestRelease(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(releaseDTO);
when(itemService.findItems(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(createItems());

NamespaceBO namespaceBO = namespaceService.loadNamespaceBO(testAppId, testEnv, testClusterName, testNamespaceName);

assertNotNull(namespaceBO);
assertTrue(namespaceBO.isPublic());
assertEquals("public-app", namespaceBO.getParentAppId());
verify(appNamespaceService, times(1)).findPublicAppNamespace(testNamespaceName);
}

@Test
public void testLoadNamespaceBOWithPrivateNamespace() {
AppNamespace privateAppNamespace = createAppNamespace(testAppId, testNamespaceName, false);
when(namespaceAPI.loadNamespace(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(createNamespace(testAppId, testClusterName, testNamespaceName));
when(appNamespaceService.findByAppIdAndName(testAppId, testNamespaceName)).thenReturn(privateAppNamespace);

ReleaseDTO releaseDTO = createReleaseDTO();
when(releaseService.loadLatestRelease(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(releaseDTO);
when(itemService.findItems(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(createItems());

NamespaceBO namespaceBO = namespaceService.loadNamespaceBO(testAppId, testEnv, testClusterName, testNamespaceName);

assertNotNull(namespaceBO);
assertEquals(testAppId, namespaceBO.getParentAppId());
}

@Test
public void testLoadNamespaceBOWithDirtyAppNamespace() {
when(namespaceAPI.loadNamespace(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(createNamespace(testAppId, testClusterName, testNamespaceName));
when(appNamespaceService.findByAppIdAndName(testAppId, testNamespaceName)).thenReturn(null);
when(appNamespaceService.findPublicAppNamespace(testNamespaceName)).thenReturn(null);

ReleaseDTO releaseDTO = createReleaseDTO();
when(releaseService.loadLatestRelease(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(releaseDTO);
when(itemService.findItems(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(createItems());

NamespaceBO namespaceBO = namespaceService.loadNamespaceBO(testAppId, testEnv, testClusterName, testNamespaceName);

assertNotNull(namespaceBO);
assertTrue(namespaceBO.isPublic());
}

@Test
public void testLoadNamespaceBOItemModifiedCountCalculation() {
ReleaseDTO releaseDTO = createReleaseDTO();
when(namespaceAPI.loadNamespace(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(createNamespace(testAppId, testClusterName, testNamespaceName));
when(releaseService.loadLatestRelease(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(releaseDTO);
when(appNamespaceService.findByAppIdAndName(testAppId, testNamespaceName)).thenReturn(createAppNamespace(testAppId, testNamespaceName, false));

List<ItemDTO> itemDTOList = createItems();
when(itemService.findItems(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(itemDTOList);

ItemDTO deletedItemDTO = new ItemDTO();
deletedItemDTO.setKey("deleted-key");
when(itemService.findDeletedItems(testAppId, testEnv, testClusterName, testNamespaceName)).thenReturn(Lists.newArrayList(deletedItemDTO));

NamespaceBO namespaceBO = namespaceService.loadNamespaceBO(testAppId, testEnv, testClusterName, testNamespaceName, true);

assertNotNull(namespaceBO);
assertEquals(3, namespaceBO.getItemModifiedCnt());
}


private ReleaseDTO createReleaseDTO() {
ReleaseDTO releaseDTO = new ReleaseDTO();
releaseDTO.setConfigurations("{\"k1\":\"k1\",\"k2\":\"k2\", \"k3\":\"\"}");
Expand Down
Loading