Skip to content
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

Resolve several code smells identified by SonarCloud #391

Merged
merged 16 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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 @@ -41,15 +41,15 @@ void testRESTDefaultContentType() throws ParseException {
ResponseEntity<String> response =
testGetRequestContentType("/gwc/rest/layers", APPLICATION_JSON);
JsonElement parsed = JsonParser.parseString(response.getBody());
assertThat(parsed.isJsonArray());
assertThat(parsed.isJsonArray()).isTrue();
}

@Test
void testRESTPathExtensionContentNegotiation() {
ResponseEntity<String> response =
testGetRequestContentType("/gwc/rest/layers.json", APPLICATION_JSON);
JsonElement parsed = JsonParser.parseString(response.getBody());
assertThat(parsed.isJsonArray());
assertThat(parsed.isJsonArray()).isTrue();

testGetRequestContentType("/gwc/rest/layers.xml", APPLICATION_XML);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ WPSXStreamLoader wpsServiceLoader(GeoServerResourceLoader resourceLoader) {

@ConditionalOnMissingBean(WPSFactoryExtension.class)
@Bean
WPSFactoryExtension WPSFactoryExtension() {
WPSFactoryExtension wpsFactoryExtension() {
log(WPSFactoryExtension.class);
return new WPSFactoryExtension() {}; // constructor is protected!
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,14 @@
import org.geotools.http.AbstractHTTPClientFactory;
import org.geotools.http.HTTPBehavior;
import org.geotools.http.HTTPClient;
import org.geotools.http.HTTPConnectionPooling;
import org.geotools.http.LoggingHTTPClient;

import java.util.List;

/** */
public class SpringEnvironmentAwareGeoToolsHttpClientFactory extends AbstractHTTPClientFactory {

private static @Setter(value = AccessLevel.PACKAGE)
GeoToolsHttpClientProxyConfigurationProperties proxyConfig =
@Setter(value = AccessLevel.PACKAGE)
private static GeoToolsHttpClientProxyConfigurationProperties proxyConfig =
new GeoToolsHttpClientProxyConfigurationProperties();

@Override
Expand All @@ -31,33 +29,4 @@ public List<Class<?>> clientClasses() {
public final HTTPClient createClient(List<Class<? extends HTTPBehavior>> behaviors) {
return new SpringEnvironmentAwareGeoToolsHttpClient(proxyConfig);
}

protected @Override HTTPClient createLogging(HTTPClient client) {
return new LoggingConnectionPoolingHTTPClient(client);
}

static class LoggingConnectionPoolingHTTPClient extends LoggingHTTPClient
implements HTTPConnectionPooling {

public LoggingConnectionPoolingHTTPClient(HTTPClient delegate) {
super(delegate);
}

public LoggingConnectionPoolingHTTPClient(HTTPClient delegate, String charset) {
super(delegate, charset);
}

@Override
public int getMaxConnections() {
return ((HTTPConnectionPooling) delegate).getMaxConnections();
}

@Override
public void setMaxConnections(int maxConnections) {
((HTTPConnectionPooling) delegate).setMaxConnections(maxConnections);
}

@Override
public void close() {}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import org.geoserver.catalog.Catalog;
import org.geoserver.catalog.CatalogInfo;
import org.geoserver.catalog.Info;
import org.geoserver.catalog.ResourceInfo;
import org.geoserver.catalog.ResourcePool;
import org.geoserver.catalog.ResourcePool.CacheClearingListener;
Expand Down Expand Up @@ -69,7 +68,7 @@ public void onCatalogRemoteModifyEvent(CatalogInfoModified event) {
() -> log.trace("Ignoring event from self: {}", event));
}

private void evictFromResourcePool(InfoEvent<? extends Info> event) {
private void evictFromResourcePool(InfoEvent event) {
final String id = event.getObjectId();
final ConfigInfoType infoType = event.getObjectType();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,12 @@ void default_admin_bad_credentials() {
context -> {
assertThat(context).hasNotFailed();
Authentication token = userNamePasswordToken("admin", "badPWD");
EnvironmentAdminAuthenticationProvider envAuthProvider =
envAuthProvider(context);

assertThrows(
InternalAuthenticationServiceException.class,
() -> envAuthProvider(context).authenticate(token));
() -> envAuthProvider.authenticate(token));
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ public class RemoteEventDataDirectoryProcessor {
private final @NonNull RepositoryGeoServerFacade configFacade;
private final @NonNull ExtendedCatalogFacade catalogFacade;

@SuppressWarnings({"rawtypes"})
@EventListener(classes = {UpdateSequenceEvent.class})
public void onUpdateSequenceEvent(UpdateSequenceEvent updateSequenceEvent) {
if (updateSequenceEvent.isRemote()) {
Expand All @@ -61,7 +60,7 @@ public void onUpdateSequenceEvent(UpdateSequenceEvent updateSequenceEvent) {
}

@EventListener(InfoRemoved.class)
public void onRemoteRemoveEvent(InfoRemoved<? extends Info> event) {
public void onRemoteRemoveEvent(InfoRemoved event) {
if (event.isLocal()) {
return;
}
Expand Down Expand Up @@ -130,16 +129,10 @@ public void onRemoteAddEvent(InfoAdded<? extends Info> event) {
case WORKSPACE:
catalogFacade.add((WorkspaceInfo) object);
break;
case COVERAGE:
case FEATURETYPE:
case WMSLAYER:
case WMTSLAYER:
case COVERAGE, FEATURETYPE, WMSLAYER, WMTSLAYER:
catalogFacade.add((ResourceInfo) object);
break;
case COVERAGESTORE:
case DATASTORE:
case WMSSTORE:
case WMTSSTORE:
case COVERAGESTORE, DATASTORE, WMSSTORE, WMTSSTORE:
catalogFacade.add((StoreInfo) object);
break;
case LAYERGROUP:
Expand Down Expand Up @@ -197,7 +190,7 @@ public void onRemoteDefaultDataStoreEvent(DefaultDataStoreSet event) {
}

@EventListener(InfoModified.class)
public void onRemoteModifyEvent(InfoModified<? extends Info> event) {
public void onRemoteModifyEvent(InfoModified event) {
if (event.isLocal()) {
return;
}
Expand All @@ -222,16 +215,10 @@ public void onRemoteModifyEvent(InfoModified<? extends Info> event) {
case WORKSPACE:
info = catalogFacade.getWorkspace(objectId);
break;
case COVERAGE:
case FEATURETYPE:
case WMSLAYER:
case WMTSLAYER:
case COVERAGE, FEATURETYPE, WMSLAYER, WMTSLAYER:
info = catalogFacade.getResource(objectId, ResourceInfo.class);
break;
case COVERAGESTORE:
case DATASTORE:
case WMSSTORE:
case WMTSSTORE:
case COVERAGESTORE, DATASTORE, WMSSTORE, WMTSSTORE:
info = catalogFacade.getStore(objectId, StoreInfo.class);
break;
case LAYERGROUP:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import lombok.extern.slf4j.Slf4j;

import org.geoserver.catalog.CatalogInfo;
import org.geoserver.catalog.Info;
import org.geoserver.cloud.event.info.ConfigInfoType;
import org.geoserver.cloud.event.info.InfoEvent;
import org.geoserver.cloud.event.info.InfoModified;
Expand All @@ -28,16 +27,16 @@ public class RemoteEventJdbcConfigProcessor {
private final @NonNull ConfigDatabase jdbcConfigDatabase;

@EventListener(InfoRemoved.class)
public void onRemoteRemoveEvent(InfoRemoved<? extends Info> event) {
public void onRemoteRemoveEvent(InfoRemoved event) {
evictConfigDatabaseEntry(event);
}

@EventListener(InfoModified.class)
public void onRemoteModifyEvent(InfoModified<? extends Info> event) {
public void onRemoteModifyEvent(InfoModified event) {
evictConfigDatabaseEntry(event);
}

private void evictConfigDatabaseEntry(InfoEvent<? extends Info> event) {
private void evictConfigDatabaseEntry(InfoEvent event) {
event.remote()
.ifPresent(
remoteEvent -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import org.geoserver.config.plugin.RepositoryGeoServerFacade;
import org.springframework.jdbc.core.JdbcTemplate;

import java.util.function.Function;
import java.util.function.UnaryOperator;

import javax.sql.DataSource;

Expand Down Expand Up @@ -58,10 +58,11 @@ public ExtendedCatalogFacade createCatalogFacade(Catalog catalog) {

public static ExtendedCatalogFacade createResolvingCatalogFacade(
Catalog catalog, PgsqlCatalogFacade rawFacade) {
Function<CatalogInfo, CatalogInfo> resolvingFunction =
UnaryOperator<CatalogInfo> resolvingFunction =
CatalogPropertyResolver.<CatalogInfo>of(catalog)
.andThen(ResolvingProxyResolver.<CatalogInfo>of(catalog))
.andThen(CollectionPropertiesInitializer.instance());
.andThen(ResolvingProxyResolver.<CatalogInfo>of(catalog))
.andThen(CollectionPropertiesInitializer.instance())
::apply;

ResolvingCatalogFacadeDecorator resolving = new ResolvingCatalogFacadeDecorator(rawFacade);
resolving.setOutboundResolver(resolvingFunction);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ public static Filter adapt(Filter filter) {

@Override
public Object visit(PropertyName expression, Object extraData) {
boolean matchCase = (extraData instanceof Boolean match) ? match.booleanValue() : true;
boolean matchCase = true;
if (extraData instanceof Boolean match) matchCase = match;
if (!matchCase) {
return getFactory(null).function("strToLowerCase", expression);
}
Expand All @@ -50,7 +51,8 @@ public Object visit(PropertyName expression, Object extraData) {

@Override
public Object visit(Literal expression, Object extraData) {
boolean matchCase = (extraData instanceof Boolean match) ? match.booleanValue() : true;
boolean matchCase = true;
if (extraData instanceof Boolean match) matchCase = match;
if (!matchCase) {
return getFactory(null).function("strToLowerCase", expression);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ private String applyTypeFilter(String sql, @NonNull Class<? extends T> type) {
return sql;
}

protected <U extends T> String applyOffsetLimit(String sql, Integer offset, Integer limit) {
protected String applyOffsetLimit(String sql, Integer offset, Integer limit) {
if (null != offset) sql += " OFFSET %d".formatted(offset);
if (null != limit) sql += " LIMIT %d".formatted(limit);
return sql;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,6 @@ void testAdd() {
info.setName("ws1");
repo.add(info);
Optional<WorkspaceInfo> found = repo.findById(info.getId(), repo.getContentType());
assertThat(found.isPresent());
assertThat(found).isPresent();
}
}
Loading
Loading