Skip to content

Commit

Permalink
Merge pull request #499 from groldan/qa/code_smells
Browse files Browse the repository at this point in the history 
fix code smells reported by sonarlint
  • Loading branch information
@groldan
groldan authored Jul 28, 2024
2 parents bf0c138 + a0a3245 commit 3a7fc64
Show file tree
Hide file tree
Showing 22 changed files with 132 additions and 139 deletions.
25 changes: 12 additions & 13 deletions .../java/org/geoserver/cloud/security/gateway/sharedauth/GatewaySharedAuhenticationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ static void registerRoutes(DynamicPropertyRegistry registry) {
private URI getcapabilities;

@BeforeEach
void setUp(WireMockRuntimeInfo runtimeInfo) throws Exception {
void setUp(WireMockRuntimeInfo runtimeInfo) {
StubMapping weblogin = buildFrom(WEB_LOGIN_SPEC);
StubMapping weblogout = buildFrom(WEB_LOGOUT_SPEC);
StubMapping wmscaps = buildFrom(WMS_GETCAPS);
Expand All @@ -199,9 +199,9 @@ void setUp(WireMockRuntimeInfo runtimeInfo) throws Exception {
wireMock.register(wmscaps);
wireMock.register(buildFrom(DEFAULT_RESPONSE));

login = gatewayUriOf(runtimeInfo, weblogin);
logout = gatewayUriOf(runtimeInfo, weblogout);
getcapabilities = gatewayUriOf(runtimeInfo, wmscaps);
login = gatewayUriOf(weblogin);
logout = gatewayUriOf(weblogout);
getcapabilities = gatewayUriOf(wmscaps);
}

/**
Expand Down Expand Up @@ -245,8 +245,8 @@ void preFilterRemovesIncomingSharedAuthHeaders(WireMockRuntimeInfo runtimeInfo)
void preFilterAppendsRequestHeadersFromSession(WireMockRuntimeInfo runtimeInfo) {
// preflight, make sure the webui responsed with the headers and they're in the
// session
ResponseEntity<Void> login = login();
final String gatewaySessionId = getGatewaySessionId(login.getHeaders());
ResponseEntity<Void> loginResponse = login();
final String gatewaySessionId = getGatewaySessionId(loginResponse.getHeaders());
assertUserAndRolesStoredInSession(gatewaySessionId);

// query the wms service with the gateway session id
Expand Down Expand Up @@ -287,8 +287,8 @@ void preFilterAppendsRequestHeadersFromSession(WireMockRuntimeInfo runtimeInfo)
@Order(3)
@DisplayName("post-filter saves user and roles in session")
void postFilterSavesUserAndRolesInSession(WireMockRuntimeInfo runtimeInfo) {
ResponseEntity<Void> login = login();
final String gatewaySessionId = getGatewaySessionId(login.getHeaders());
ResponseEntity<Void> loginResponse = login();
final String gatewaySessionId = getGatewaySessionId(loginResponse.getHeaders());

assertUserAndRolesStoredInSession(gatewaySessionId);
}
Expand All @@ -299,8 +299,8 @@ void postFilterSavesUserAndRolesInSession(WireMockRuntimeInfo runtimeInfo) {
void postFilterRemovesUserAndRolesFromSessionOnEmptyUserResponseHeader(
WireMockRuntimeInfo runtimeInfo) {
// preflight, have a session and the user and roles stored
ResponseEntity<Void> login = login();
final String gatewaySessionId = getGatewaySessionId(login.getHeaders());
ResponseEntity<Void> loginResponse = login();
final String gatewaySessionId = getGatewaySessionId(loginResponse.getHeaders());
assertUserAndRolesStoredInSession(gatewaySessionId);

// make a request that returns and empty string on the x-gsc-username response header
Expand Down Expand Up @@ -354,11 +354,10 @@ private void assertUserAndRolesStoredInSession(final String gatewaySessionId) {
private Map<String, Object> getSessionAttributes(final String gatewaySessionId) {
WebSessionStore sessionStore = webSessionManager.getSessionStore();
WebSession session = sessionStore.retrieveSession(gatewaySessionId).block();
Map<String, Object> attributes = session.getAttributes();
return attributes;
return session.getAttributes();
}

private URI gatewayUriOf(WireMockRuntimeInfo runtimeInfo, StubMapping mapping) {
private URI gatewayUriOf(StubMapping mapping) {
return URI.create(mapping.getRequest().getUrl());
}

Expand Down
4 changes: 1 addition & 3 deletions ...rg/geoserver/cloud/autoconfigure/security/EnvironmentAdminAuthenticationProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,6 @@ protected UsernamePasswordAuthenticationToken userNamePasswordToken(

protected EnvironmentAdminAuthenticationProvider envAuthProvider(
AssertableApplicationContext context) {
EnvironmentAdminAuthenticationProvider provider =
context.getBean(EnvironmentAdminAuthenticationProvider.class);
return provider;
return context.getBean(EnvironmentAdminAuthenticationProvider.class);
}
}
2 changes: 1 addition & 1 deletion ...mmon/src/test/java/org/geoserver/security/impl/GsCloudLayerGroupContainmentCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ private static void addWorkspaceNamespace(String wsName) {
}

@BeforeEach
public void setupLayerGrups() throws InterruptedException {
public void setupLayerGrups() {
LayerInfo lakes = catalog.getLayerByName(getLayerId(MockData.LAKES));
LayerInfo forests = catalog.getLayerByName(getLayerId(MockData.FORESTS));
LayerInfo roads = catalog.getLayerByName(getLayerId(MockData.ROAD_SEGMENTS));
Expand Down
7 changes: 3 additions & 4 deletions .../org/geoserver/cloud/backend/pgconfig/config/PgconfigConfigRepositoryConformanceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import org.geoserver.cloud.backend.pgconfig.support.PgConfigTestContainer;
import org.geoserver.config.GeoServer;
import org.geoserver.config.GeoServerConfigConformanceTest;
import org.geoserver.config.plugin.GeoServerImpl;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.testcontainers.junit.jupiter.Container;
Expand All @@ -19,6 +18,7 @@
* @since 1.4
*/
@Testcontainers(disabledWithoutDocker = true)
@SuppressWarnings("java:S2187")
class PgconfigConfigRepositoryConformanceTest extends GeoServerConfigConformanceTest {

@Container static PgConfigTestContainer<?> container = new PgConfigTestContainer<>();
Expand All @@ -31,14 +31,13 @@ public void setUp() {
}

@AfterEach
void cleanDb() throws Exception {
void cleanDb() {
container.tearDown();
}

protected @Override GeoServer createGeoServer() {
PgconfigBackendBuilder builder = new PgconfigBackendBuilder(container.getDataSource());
Catalog catalog = builder.createCatalog();
GeoServerImpl gs = builder.createGeoServer(catalog);
return gs;
return builder.createGeoServer(catalog);
}
}
3 changes: 2 additions & 1 deletion ...src/test/java/org/geoserver/cloud/backend/pgconfig/config/PgconfigUpdateSequenceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* @since 1.4
*/
@Testcontainers(disabledWithoutDocker = true)
@SuppressWarnings("java:S2187")
class PgconfigUpdateSequenceTest implements UpdateSequenceConformanceTest {

@Container static PgConfigTestContainer<?> container = new PgConfigTestContainer<>();
Expand All @@ -35,7 +36,7 @@ public void init() {
}

@AfterEach
void cleanDb() throws Exception {
void cleanDb() {
container.tearDown();
}

Expand Down
10 changes: 3 additions & 7 deletions ...fig/src/test/java/org/geoserver/cloud/backend/pgconfig/resource/PgconfigResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@
@RunWith(Theories.class)
public class PgconfigResourceTest extends ResourceTheoryTest {

public @ClassRule static PgConfigTestContainer<?> container = new PgConfigTestContainer<>();
@ClassRule public static PgConfigTestContainer<?> container = new PgConfigTestContainer<>();

public @Rule TemporaryFolder tmpDir = new TemporaryFolder();
@Rule public TemporaryFolder tmpDir = new TemporaryFolder();
private PgconfigResourceStore store;
private File cacheDirectory;

Expand Down Expand Up @@ -445,11 +445,7 @@ INSERT INTO resourcestore (parentid, "type", path, content)

private Set<String> replace(String oldParent, String newParent, Set<String> children) {
return children.stream()
.map(
p -> {
String newpath = newParent + p.substring(oldParent.length());
return newpath;
})
.map(p -> newParent + p.substring(oldParent.length()))
.collect(Collectors.toCollection(TreeSet::new));
}

Expand Down
1 change: 1 addition & 0 deletions ...fig/src/test/java/org/geoserver/cloud/backend/pgconfig/support/PgConfigTestContainer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
*
* @since 1.6
*/
@SuppressWarnings("java:S119")
public class PgConfigTestContainer<SELF extends PostgreSQLContainer<SELF>>
extends PostgreSQLContainer<SELF> {

Expand Down
60 changes: 30 additions & 30 deletions ...log/cache/src/test/java/org/geoserver/cloud/catalog/cache/CachingGeoServerFacadeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class CachingGeoServerFacadeTest {
private @Autowired CacheManager cacheManager;
private Cache cache;
private GeoServerInfo global;
private WorkspaceInfo ws;
private WorkspaceInfo workspace;
private SettingsInfo settings;
private TestService1 service1;
private TestService2 service2;
Expand All @@ -96,9 +96,9 @@ public static interface TestService2 extends ServiceInfo {}

public @BeforeEach void before() {
global = stub(GeoServerInfo.class);
ws = stub(WorkspaceInfo.class);
workspace = stub(WorkspaceInfo.class);
settings = stub(SettingsInfo.class);
when(settings.getWorkspace()).thenReturn(ws);
when(settings.getWorkspace()).thenReturn(workspace);
logging = stub(LoggingInfo.class);

service1 = stub(TestService1.class, 1);
Expand All @@ -110,8 +110,8 @@ public static interface TestService2 extends ServiceInfo {}
wsService2 = stub(TestService2.class, 2);
when(wsService1.getName()).thenReturn("service1");
when(wsService2.getName()).thenReturn("service2");
when(wsService1.getWorkspace()).thenReturn(ws);
when(wsService2.getWorkspace()).thenReturn(ws);
when(wsService1.getWorkspace()).thenReturn(workspace);
when(wsService2.getWorkspace()).thenReturn(workspace);

when(mock.getGlobal()).thenReturn(global);
when(mock.getLogging()).thenReturn(logging);
Expand All @@ -124,25 +124,25 @@ public static interface TestService2 extends ServiceInfo {}
when(mock.getService(service2.getId(), TestService2.class)).thenReturn(service2);
doReturn(Arrays.asList(service1, service2)).when(mock).getServices();

when(mock.getService(ws, TestService1.class)).thenReturn(wsService1);
when(mock.getService(ws, TestService2.class)).thenReturn(wsService2);
doReturn(Arrays.asList(wsService1, wsService2)).when(mock).getServices(ws);
when(mock.getService(workspace, TestService1.class)).thenReturn(wsService1);
when(mock.getService(workspace, TestService2.class)).thenReturn(wsService2);
doReturn(Arrays.asList(wsService1, wsService2)).when(mock).getServices(workspace);

when(mock.getServiceByName(service1.getName(), ServiceInfo.class)).thenReturn(service1);
when(mock.getServiceByName(service1.getName(), TestService1.class)).thenReturn(service1);
when(mock.getServiceByName(service2.getName(), ServiceInfo.class)).thenReturn(service1);
when(mock.getServiceByName(service2.getName(), TestService2.class)).thenReturn(service2);

when(mock.getServiceByName(wsService1.getName(), ws, ServiceInfo.class))
when(mock.getServiceByName(wsService1.getName(), workspace, ServiceInfo.class))
.thenReturn(wsService1);
when(mock.getServiceByName(wsService1.getName(), ws, TestService1.class))
when(mock.getServiceByName(wsService1.getName(), workspace, TestService1.class))
.thenReturn(wsService1);
when(mock.getServiceByName(wsService2.getName(), ws, ServiceInfo.class))
when(mock.getServiceByName(wsService2.getName(), workspace, ServiceInfo.class))
.thenReturn(wsService1);
when(mock.getServiceByName(wsService2.getName(), ws, TestService2.class))
when(mock.getServiceByName(wsService2.getName(), workspace, TestService2.class))
.thenReturn(wsService2);

when(mock.getSettings(ws)).thenReturn(settings);
when(mock.getSettings(workspace)).thenReturn(settings);
this.cache = cacheManager.getCache(CACHE_NAME);
this.cache.clear();
}
Expand Down Expand Up @@ -218,14 +218,14 @@ void onLoggingInfoSetEvent() {

@Test
void onSettingsInfoModifyEvent() {
caching.getSettings(ws);
caching.getSettings(workspace);
final Object idKey = settings.getId();
final Object wsKey = CachingGeoServerFacade.settingsKey(ws);
final Object wsKey = CachingGeoServerFacade.settingsKey(workspace);
assertThat(cache.get(idKey)).isNotNull();
assertThat(cache.get(wsKey)).isNotNull();

SettingsModified event = event(SettingsModified.class, settings.getId(), SETTINGS);
final String wsid = ws.getId();
final String wsid = workspace.getId();
when(event.getWorkspaceId()).thenReturn(wsid);

caching.onSettingsInfoModifyEvent(event);
Expand All @@ -235,14 +235,14 @@ void onSettingsInfoModifyEvent() {

@Test
void onSettingsInfoRemoveEvent() {
caching.getSettings(ws);
caching.getSettings(workspace);
final Object idKey = settings.getId();
final Object wsKey = CachingGeoServerFacade.settingsKey(ws);
final Object wsKey = CachingGeoServerFacade.settingsKey(workspace);
assertThat(cache.get(idKey)).isNotNull();
assertThat(cache.get(wsKey)).isNotNull();

SettingsRemoved event = event(SettingsRemoved.class, settings.getId(), SETTINGS);
final String wsid = ws.getId();
final String wsid = workspace.getId();
when(event.getWorkspaceId()).thenReturn(wsid);

caching.onSettingsInfoRemoveEvent(event);
Expand Down Expand Up @@ -391,29 +391,29 @@ void testSaveGeoServerInfo() {

@Test
void testGetSettings() {
assertSameTimesN(settings, () -> caching.getSettings(ws), 3);
verify(mock, times(1)).getSettings(ws);
assertNotNull(cache.get(CachingGeoServerFacade.settingsKey(ws)));
assertSameTimesN(settings, () -> caching.getSettings(workspace), 3);
verify(mock, times(1)).getSettings(workspace);
assertNotNull(cache.get(CachingGeoServerFacade.settingsKey(workspace)));
}

@Test
void testSaveSettingsInfo() {
assertSameTimesN(settings, () -> caching.getSettings(ws), 3);
verify(mock, times(1)).getSettings(ws);
assertNotNull(cache.get(CachingGeoServerFacade.settingsKey(ws)));
assertSameTimesN(settings, () -> caching.getSettings(workspace), 3);
verify(mock, times(1)).getSettings(workspace);
assertNotNull(cache.get(CachingGeoServerFacade.settingsKey(workspace)));

caching.save(settings);
assertNull(cache.get(CachingGeoServerFacade.settingsKey(ws)));
assertNull(cache.get(CachingGeoServerFacade.settingsKey(workspace)));
}

@Test
void testRemoveSettingsInfo() {
assertSameTimesN(settings, () -> caching.getSettings(ws), 3);
verify(mock, times(1)).getSettings(ws);
assertNotNull(cache.get(CachingGeoServerFacade.settingsKey(ws)));
assertSameTimesN(settings, () -> caching.getSettings(workspace), 3);
verify(mock, times(1)).getSettings(workspace);
assertNotNull(cache.get(CachingGeoServerFacade.settingsKey(workspace)));

caching.remove(settings);
assertNull(cache.get(CachingGeoServerFacade.settingsKey(ws)));
assertNull(cache.get(CachingGeoServerFacade.settingsKey(workspace)));
}

@Test
Expand Down
36 changes: 17 additions & 19 deletions ...atalog/event-bus/src/test/java/org/geoserver/cloud/event/bus/BusAmqpIntegrationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.geoserver.catalog.LayerGroupInfo;
import org.geoserver.catalog.LayerInfo;
import org.geoserver.catalog.NamespaceInfo;
import org.geoserver.catalog.PublishedInfo;
import org.geoserver.catalog.ResourceInfo;
import org.geoserver.catalog.StoreInfo;
import org.geoserver.catalog.StyleInfo;
Expand Down Expand Up @@ -264,8 +263,7 @@ protected <T extends Info> PropertyDiff resolveExpectedDiff(T proxy) {
List<Object> oldValues = h.getOldValues();
assertFalse(propertyNames.isEmpty(), "Test should change at least one property");

PropertyDiff expected = PropertyDiff.valueOf(propertyNames, oldValues, newValues);
return expected;
return PropertyDiff.valueOf(propertyNames, oldValues, newValues);
}

@SuppressWarnings("unchecked")
Expand Down Expand Up @@ -353,45 +351,45 @@ protected <T extends Info> void assertRemoteEvent(T info, RemoteGeoServerEvent b
assertThat(infoType.isInstance(object)).isTrue();
assertThat(object.getId()).isEqualTo(info.getId());

assertResolved(object, Info.class);
assertResolved(object);
}

if (event instanceof InfoModified modifyEvent) {
assertThat(modifyEvent.getPatch()).isNotNull();
}
}

protected void assertResolved(Info info, Class<? extends Info> expected) {
protected void assertResolved(Info info) {
if (null == info) return;
if (ProxyUtils.isResolvingProxy(info)) return;
switch (info) {
case StoreInfo store:
assertCatalogSet(store, store::getCatalog);
assertResolved(store.getWorkspace(), WorkspaceInfo.class);
assertResolved(store.getWorkspace());
break;
case ResourceInfo resource:
assertCatalogSet(resource, resource::getCatalog);
assertResolved(resource.getNamespace(), NamespaceInfo.class);
assertResolved(resource.getStore(), StoreInfo.class);
assertResolved(resource.getNamespace());
assertResolved(resource.getStore());
break;
case StyleInfo style:
assertResolved(style.getWorkspace(), WorkspaceInfo.class);
assertResolved(style.getWorkspace());
break;
case LayerInfo layer:
assertResolved(layer.getDefaultStyle(), StyleInfo.class);
assertResolved(layer.getResource(), ResourceInfo.class);
assertResolved(layer.getDefaultStyle());
assertResolved(layer.getResource());
break;
case LayerGroupInfo lg:
assertResolved(lg.getWorkspace(), WorkspaceInfo.class);
assertResolved(lg.getRootLayer(), LayerInfo.class);
assertResolved(lg.getRootLayerStyle(), StyleInfo.class);
lg.getStyles().forEach(s -> assertResolved(s, StyleInfo.class));
lg.getLayers().forEach(l -> assertResolved(l, PublishedInfo.class));
lg.getLayerGroupStyles().forEach(lgs -> assertResolved(lgs, LayerGroupStyle.class));
assertResolved(lg.getWorkspace());
assertResolved(lg.getRootLayer());
assertResolved(lg.getRootLayerStyle());
lg.getStyles().forEach(this::assertResolved);
lg.getLayers().forEach(this::assertResolved);
lg.getLayerGroupStyles().forEach(this::assertResolved);
break;
case LayerGroupStyle lgs:
lgs.getLayers().forEach(l -> assertResolved(l, PublishedInfo.class));
lgs.getStyles().forEach(s -> assertResolved(s, StyleInfo.class));
lgs.getLayers().forEach(this::assertResolved);
lgs.getStyles().forEach(this::assertResolved);
break;
case NamespaceInfo ns:
break;
Expand Down
Loading

0 comments on commit 3a7fc64

Please sign in to comment.