Skip to content

Commit 9785c04

Browse files
sergio-esperalta-bisergio-esperalta-bi
authored
Refactor ProjectsFacade for enhanced cluster management (#11)
* Refactor `ProjectsFacade`: improve cluster handling, ensure proper merging and sanitization, and add fallback logic. Update tests and documentation accordingly. * Remove redundant test stubbing in `ProjectsFacadeTest` to clean up test logic. * Refactor `ProjectsFacade`: rename methods to clarify handling of the first available cluster. * Improve exception logging in `ProjectsFacade` and adjust cluster assertion in tests * Improve exception logging in `ProjectsFacade`
1 parent 01953ab commit 9785c04

3 files changed

Lines changed: 161 additions & 53 deletions

File tree

‎README.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ Do the following:
3636
- Browse and select the `src/main/resources/application-local.env` you just created
3737
- At the end of the process, you should have a configuration similar to the following:
3838

39+
### 2.3. Modify (if needed) the "Build and run using" and "Run tests" using options in IntelliJ"
40+
41+
Do the following:
42+
- Go to Settings / Build, Execution and Deployment / Build tools / Gradle
43+
- Change the options "Build and run using" and "Run tests" to "IntelliJ IDEA"
44+
3945
## 3. Customize application-local.yml file
4046

4147
The `application.yml` file takes some property values from the env vars, and `application-local.yml` config file

‎src/main/java/org/opendevstack/projects_info_service/server/facade/ProjectsFacade.java‎

Lines changed: 66 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package org.opendevstack.projects_info_service.server.facade;
22

3+
import jakarta.annotation.PostConstruct;
4+
import lombok.extern.slf4j.Slf4j;
5+
import org.apache.commons.lang3.StringUtils;
36
import org.opendevstack.projects_info_service.configuration.ClusterConfiguration;
47
import org.opendevstack.projects_info_service.server.annotations.CacheableWithFallback;
58
import org.opendevstack.projects_info_service.server.client.AzureGraphClient;
@@ -8,15 +11,13 @@
811
import org.opendevstack.projects_info_service.server.dto.ProjectInfo;
912
import org.opendevstack.projects_info_service.server.dto.ProjectPlatforms;
1013
import org.opendevstack.projects_info_service.server.dto.Section;
14+
import org.opendevstack.projects_info_service.server.model.OpenshiftProjectCluster;
1115
import org.opendevstack.projects_info_service.server.model.PlatformsWithTitle;
1216
import org.opendevstack.projects_info_service.server.security.GroupValidatorService;
1317
import org.opendevstack.projects_info_service.server.service.EdpProjectsService;
1418
import org.opendevstack.projects_info_service.server.service.MocksService;
1519
import org.opendevstack.projects_info_service.server.service.OpenShiftProjectService;
1620
import org.opendevstack.projects_info_service.server.service.PlatformService;
17-
import jakarta.annotation.PostConstruct;
18-
import lombok.extern.slf4j.Slf4j;
19-
import org.apache.commons.lang3.StringUtils;
2021
import org.springframework.stereotype.Component;
2122

2223
import java.util.*;
@@ -36,13 +37,13 @@ public class ProjectsFacade {
3637

3738
private final PlatformService platformService;
3839

39-
private final GroupValidatorService groupValidatorService;
40+
private final GroupValidatorService groupValidatorService;
4041

4142
private final ClusterConfiguration clusterConfiguration;
4243

4344
private Map<String, String> clusterMapper;
4445

45-
private final ProjectWhitelistYmlClient projectWhitelistYmlClient;
46+
private final ProjectWhitelistYmlClient projectWhitelistYmlClient;
4647

4748
public ProjectsFacade(AzureGraphClient azureGraphClient,
4849
OpenShiftProjectService openShiftProjectService,
@@ -68,7 +69,7 @@ void initializeClusterMapper() {
6869

6970
Map<String, String> result = new HashMap<>();
7071

71-
mapper.forEach( (key, value) -> {
72+
mapper.forEach((key, value) -> {
7273
var values = value.split(",");
7374

7475
for (String val : values) {
@@ -97,19 +98,19 @@ public Map<String, ProjectInfo> getProjects(String token) {
9798
Map<String, ProjectInfo> result = new HashMap<>();
9899

99100
edpProjects.forEach(edpProject -> {
100-
if (result.containsKey(edpProject.getProjectKey())) {
101-
var resultProjectClusters = result.get(edpProject.getProjectKey()).getClusters();
102-
var edpProjectClusters = edpProject.getClusters();
103-
104-
// Merge clusters if project already exists
105-
var mergedClusters = Stream.concat(resultProjectClusters.stream(), edpProjectClusters.stream())
106-
.distinct()
107-
.toList();
108-
109-
result.get(edpProject.getProjectKey()).setClusters(mergedClusters);
110-
} else {
111-
result.put(edpProject.getProjectKey(), edpProject);
112-
}
101+
if (result.containsKey(edpProject.getProjectKey())) {
102+
var resultProjectClusters = result.get(edpProject.getProjectKey()).getClusters();
103+
var edpProjectClusters = edpProject.getClusters();
104+
105+
// Merge clusters if project already exists
106+
var mergedClusters = Stream.concat(resultProjectClusters.stream(), edpProjectClusters.stream())
107+
.distinct()
108+
.toList();
109+
110+
result.get(edpProject.getProjectKey()).setClusters(mergedClusters);
111+
} else {
112+
result.put(edpProject.getProjectKey(), edpProject);
113+
}
113114
});
114115

115116
for (Map.Entry<String, ProjectInfo> mockEntry : mockProjects.entrySet()) {
@@ -129,36 +130,39 @@ public ProjectPlatforms getProjectPlatforms(String projectKey) {
129130
var allEdpProjectsInfo = openShiftProjectService.fetchProjects();
130131
var mockProjectsAndClusters = mocksService.getDefaultProjectsAndClusters();
131132

132-
var edProjectInfo = allEdpProjectsInfo.stream()
133+
var edpProjectsInfo = allEdpProjectsInfo.stream()
133134
.filter(p -> p.getProject().equals(projectKey))
134-
.findFirst();
135+
.toList();
135136

136137
var mockClusters = mockProjectsAndClusters.entrySet().stream()
137138
.filter(e -> e.getValue().getProjectKey().equals(projectKey))
138139
.flatMap(e -> e.getValue().getClusters().stream())
139140
.toList();
140141

141142
// If EDP project exists, add its clusters to the front of the list, so we prioritize them
142-
var mergedClusters = edProjectInfo.map(projectInfo -> {
143-
List<String> clusters = new ArrayList<>();
144-
145-
clusters.add(projectInfo.getCluster());
146-
147-
clusters.addAll(mockClusters);
143+
var mergedClusters = new ArrayList<String>();
144+
145+
if (!edpProjectsInfo.isEmpty()) {
146+
mergedClusters.addAll(
147+
edpProjectsInfo.stream()
148+
.map(OpenshiftProjectCluster::getCluster)
149+
.toList()
150+
);
151+
}
148152

149-
return List.copyOf(clusters); // We always prefer immutable lists
150-
}).orElse(mockClusters);
153+
mergedClusters.addAll(mockClusters);
154+
var sanitizedClusters = sanitizeClusters(mergedClusters);
151155

152-
if (mergedClusters.isEmpty()) {
156+
if (sanitizedClusters.isEmpty()) {
153157
log.debug("Project not found: {}", projectKey);
154158

155159
return null;
156160
} else {
157-
log.debug("Project found: {}, returning ProjectPlatforms for clusters: {}.", projectKey, mergedClusters);
158-
159-
List<Section> sections = new ArrayList<>(platformService.getSections(projectKey, mergedClusters.getFirst()));
161+
log.debug("Project found: {}, returning ProjectPlatforms for clusters: {}.", projectKey, sanitizedClusters);
162+
var clusters = getClusterBySanitizedValue(clusterMapper, sanitizedClusters.getFirst());
163+
List<Section> sections = getSectionFromFirstAvailableCluster(projectKey, clusters);
160164
var disabledPlatforms = platformService.getDisabledPlatforms(projectKey);
161-
var platformsWithTitle = platformService.getPlatforms(projectKey, mergedClusters.getFirst());
165+
var platformsWithTitle = getPlatformsWithTitleFromFirstAvailableCluster(projectKey, clusters);
162166

163167
var firstSection = componseFirstSection(platformsWithTitle, disabledPlatforms);
164168

@@ -223,4 +227,32 @@ private List<String> sanitizeClusters(List<String> clusters) {
223227
return new ArrayList<>(sanitizedClusters);
224228
}
225229

230+
private <K, V> List<K> getClusterBySanitizedValue(Map<K, V> map, V value) {
231+
return map.entrySet().stream()
232+
.filter(entry -> Objects.equals(entry.getValue(), value))
233+
.map(Map.Entry::getKey)
234+
.toList();
235+
}
236+
237+
private List<Section> getSectionFromFirstAvailableCluster(String projectKey, List<String> clusters) {
238+
for (String cluster : clusters) {
239+
try {
240+
return new ArrayList<>(platformService.getSections(projectKey, cluster));
241+
} catch (Exception e) {
242+
log.warn("In sections for projectKey {}, cluster is not configured: {}.", projectKey, cluster, e);
243+
}
244+
}
245+
return List.of();
246+
}
247+
248+
private PlatformsWithTitle getPlatformsWithTitleFromFirstAvailableCluster(String projectKey, List<String> clusters) {
249+
for (String cluster : clusters) {
250+
try {
251+
return platformService.getPlatforms(projectKey, cluster);
252+
} catch (Exception e) {
253+
log.warn("In platforms for projectKey {}, cluster is not configured: {}.", projectKey, cluster, e);
254+
}
255+
}
256+
return new PlatformsWithTitle();
257+
}
226258
}

‎src/test/java/org/opendevstack/projects_info_service/server/facade/ProjectsFacadeTest.java‎

Lines changed: 89 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,29 @@
11
package org.opendevstack.projects_info_service.server.facade;
22

3+
import org.junit.jupiter.api.BeforeEach;
4+
import org.junit.jupiter.api.Test;
5+
import org.junit.jupiter.api.extension.ExtendWith;
6+
import org.mockito.InjectMocks;
7+
import org.mockito.Mock;
8+
import org.mockito.junit.jupiter.MockitoExtension;
39
import org.opendevstack.projects_info_service.configuration.ClusterConfiguration;
410
import org.opendevstack.projects_info_service.server.client.AzureGraphClient;
511
import org.opendevstack.projects_info_service.server.client.ProjectWhitelistYmlClient;
612
import org.opendevstack.projects_info_service.server.dto.*;
7-
import org.opendevstack.projects_info_service.server.dto.ProjectInfoMother;
8-
import org.opendevstack.projects_info_service.server.dto.SectionMother;
9-
import org.opendevstack.projects_info_service.server.model.OpenshiftProjectCluster;
10-
import org.opendevstack.projects_info_service.server.model.OpenshiftProjectClusterMother;
11-
import org.opendevstack.projects_info_service.server.model.PlatformMother;
12-
import org.opendevstack.projects_info_service.server.model.PlatformsWithTitleMother;
13-
import org.opendevstack.projects_info_service.server.model.ProjectsWhitelisted;
13+
import org.opendevstack.projects_info_service.server.model.*;
1414
import org.opendevstack.projects_info_service.server.security.GroupValidatorService;
1515
import org.opendevstack.projects_info_service.server.service.EdpProjectsService;
1616
import org.opendevstack.projects_info_service.server.service.MocksService;
1717
import org.opendevstack.projects_info_service.server.service.OpenShiftProjectService;
1818
import org.opendevstack.projects_info_service.server.service.PlatformService;
19-
import org.junit.jupiter.api.BeforeEach;
20-
import org.junit.jupiter.api.Test;
21-
import org.junit.jupiter.api.extension.ExtendWith;
22-
import org.mockito.InjectMocks;
23-
import org.mockito.Mock;
24-
import org.mockito.junit.jupiter.MockitoExtension;
2519

2620
import java.util.Collections;
2721
import java.util.HashSet;
2822
import java.util.List;
2923
import java.util.Map;
3024

31-
import static org.assertj.core.api.Assertions.*;
25+
import static org.assertj.core.api.Assertions.assertThat;
26+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
3227
import static org.mockito.Mockito.when;
3328

3429
@ExtendWith(MockitoExtension.class)
@@ -67,7 +62,8 @@ void setUp() {
6762
Map<String, String> clusterConfigurationMapper = Map.of(
6863
"US_TEST", "us-test",
6964
"US", "us-dev, us-aws-east1",
70-
"EU", "eu-dev, eu-aws-west1, Europe"
65+
"EU", "eu-dev, eu-aws-west1, Europe",
66+
"MOTHER", "mother-cluster, mother-cluster-1, mother-cluster-2"
7167
);
7268

7369
when(clusterConfiguration.getMapper()).thenReturn(clusterConfigurationMapper);
@@ -83,8 +79,8 @@ void givenAnAzureToken_whenGetProjects_thenReturnListOfProjects() {
8379
var accessToken = "sample";
8480
var azureGroups = new HashSet<>(List.of("group1", "group2", "group3"));
8581
var edpProjectsInfo = List.of(
86-
new OpenshiftProjectCluster("edpc","eu_dev"),
87-
new OpenshiftProjectCluster("devstack","us_test")
82+
new OpenshiftProjectCluster("edpc", "eu_dev"),
83+
new OpenshiftProjectCluster("devstack", "us_test")
8884
);
8985

9086
var edpProjects = List.of(new ProjectInfo("ATLAS", Collections.emptyList()), new ProjectInfo("EDPP", Collections.emptyList()));
@@ -311,8 +307,8 @@ void givenWhitelist_whenGetProjects_thenOnlyWhitelistedProjectsAreReturned() {
311307
var accessToken = "sample";
312308
var azureGroups = new HashSet<>(List.of("g1", "g2"));
313309
var edpProjectsInfo = List.of(
314-
new OpenshiftProjectCluster("edpc","eu_dev"),
315-
new OpenshiftProjectCluster("devstack","us_test")
310+
new OpenshiftProjectCluster("edpc", "eu_dev"),
311+
new OpenshiftProjectCluster("devstack", "us_test")
316312
);
317313

318314
var edpProjects = List.of(
@@ -399,4 +395,78 @@ void givenUnknownClusterInProjects_whenGetProjects_thenThrowsIllegalArgumentExce
399395
.isInstanceOf(IllegalArgumentException.class)
400396
.hasMessageContaining("is not recognized");
401397
}
398+
399+
@Test
400+
void givenMultipleClustersForProject_whenGetProjects_thenClustersAreMergedAndSanitized() {
401+
// given
402+
var accessToken = "token";
403+
var userEmail = "user@example.com";
404+
var azureGroups = new HashSet<>(List.of("g1"));
405+
406+
var edpProjectsInfo = List.of(new OpenshiftProjectCluster("P1", "eu-dev"));
407+
var edpProjects = List.of(
408+
new ProjectInfo("P1", List.of("eu-dev")),
409+
new ProjectInfo("P1", List.of("us-test"))
410+
);
411+
412+
when(azureGraphClient.getUserEmail(accessToken)).thenReturn(userEmail);
413+
when(azureGraphClient.getUserGroups(accessToken)).thenReturn(azureGroups);
414+
when(openShiftProjectService.fetchProjects()).thenReturn(edpProjectsInfo);
415+
when(edpProjectsService.filterProjects(azureGroups, edpProjectsInfo)).thenReturn(new HashSet<>(edpProjects));
416+
when(mocksService.getProjectsAndClusters(userEmail)).thenReturn(Collections.emptyMap());
417+
418+
// when
419+
Map<String, ProjectInfo> projects = projectsFacade.getProjects(accessToken);
420+
421+
// then
422+
assertThat(projects).containsKey("P1");
423+
assertThat(projects.get("P1").getClusters()).containsExactly("EU", "US_TEST");
424+
}
425+
426+
@Test
427+
void givenEmptyClusterName_whenGetProjectPlatforms_thenIgnored() {
428+
// given
429+
var projectKey = "P1";
430+
var edpProjectsInfo = List.of(new OpenshiftProjectCluster(projectKey, "")); // Empty cluster
431+
432+
when(openShiftProjectService.fetchProjects()).thenReturn(edpProjectsInfo);
433+
434+
// when
435+
ProjectPlatforms result = projectsFacade.getProjectPlatforms(projectKey);
436+
437+
// then
438+
assertThat(result).isNull();
439+
}
440+
441+
@Test
442+
void givenClusterNotConfiguredInPlatformService_whenGetProjectPlatforms_thenFallbackToNextCluster() {
443+
// given
444+
var projectKey = "P1";
445+
// mapped to US_TEST and EU
446+
var edpProjectsInfo = List.of(
447+
new OpenshiftProjectCluster(projectKey, "us-test"),
448+
new OpenshiftProjectCluster(projectKey, "eu-dev")
449+
);
450+
451+
when(openShiftProjectService.fetchProjects()).thenReturn(edpProjectsInfo);
452+
when(platformService.getDisabledPlatforms(projectKey)).thenReturn(Collections.emptyList());
453+
454+
// us-test fails
455+
when(platformService.getSections(projectKey, "us-test")).thenThrow(new RuntimeException("Not configured"));
456+
when(platformService.getPlatforms(projectKey, "us-test")).thenThrow(new RuntimeException("Not configured"));
457+
458+
// eu-dev succeeds
459+
var expectedSections = List.of(SectionMother.of());
460+
var expectedPlatforms = PlatformsWithTitleMother.of();
461+
when(platformService.getSections(projectKey, "eu-dev")).thenReturn(expectedSections);
462+
when(platformService.getPlatforms(projectKey, "eu-dev")).thenReturn(expectedPlatforms);
463+
464+
// when
465+
ProjectPlatforms result = projectsFacade.getProjectPlatforms(projectKey);
466+
467+
// then
468+
assertThat(result).isNotNull();
469+
assertThat(result.getSections()).hasSize(2); // First section + expectedSection
470+
assertThat(result.getSections().get(1)).isEqualTo(expectedSections.get(0));
471+
}
402472
}

0 commit comments

Comments
 (0)