Skip to content

Commit

Permalink
Fix some issues detected by sonar (#873)
Browse files Browse the repository at this point in the history 
* Improve visibility and isolation in pmtiles

* Format code with spotless

* throw dedicated exceptions

* Remove unused methods

* Remove duplicate code

* Add default to switch statements

* Fix boxed boolean checks

* Suppress warnings

* implement equals and hashCode in adapters

* Use unary operator instead of function

* Use parametrized types

* Add private constructors

* Improve use of instanceof

* Cleanup test

* Merge cases in switch statements

* Merge if statements

* Use records instead of classes

* Use final when appropriate

* Improve use of Optional

* Format test assertions
  • Loading branch information
@bchapuis
bchapuis authored Jun 13, 2024
1 parent 47b3922 commit fc64ee1
Show file tree
Hide file tree
Showing 68 changed files with 561 additions and 929 deletions.
11 changes: 6 additions & 5 deletions baremaps-core/src/main/java/org/apache/baremaps/database/DiffService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public class DiffService implements Callable<List<TileCoord>> {
private final int srid;
private final int zoom;

@SuppressWarnings("squid:S107")
public DiffService(
Map<Long, Coordinate> coordinateMap,
Map<Long, List<Long>> referenceMap,
Expand All @@ -79,8 +80,8 @@ public List<TileCoord> call() throws Exception {
logger.info("Importing changes");

var header = headerRepository.selectLatest();
var replicationUrl = header.getReplicationUrl();
var sequenceNumber = header.getReplicationSequenceNumber() + 1;
var replicationUrl = header.replicationUrl();
var sequenceNumber = header.replicationSequenceNumber() + 1;
var changeUrl = resolve(replicationUrl, sequenceNumber, "osc.gz");

var projectionTransformer = new ProjectionTransformer(srid, 4326);
Expand All @@ -100,7 +101,7 @@ private Stream<TileCoord> tilesForGeometry(Geometry geometry) {
}

private Stream<Geometry> geometriesForChange(Change change) {
switch (change.getType()) {
switch (change.type()) {
case CREATE:
return geometriesForNextVersion(change);
case DELETE:
Expand All @@ -114,7 +115,7 @@ private Stream<Geometry> geometriesForChange(Change change) {
}

private Stream<Geometry> geometriesForPreviousVersion(Change change) {
return change.getEntities().stream().map(this::geometriesForPreviousVersion)
return change.entities().stream().map(this::geometriesForPreviousVersion)
.flatMap(Optional::stream);
}

Expand All @@ -138,7 +139,7 @@ private Optional<Geometry> geometriesForPreviousVersion(Entity entity) {
}

private Stream<Geometry> geometriesForNextVersion(Change change) {
return change.getEntities().stream()
return change.entities().stream()
.map(consumeThenReturn(new EntityGeometryBuilder(coordinateMap, referenceMap)))
.flatMap(new EntityToGeometryMapper().andThen(Optional::stream));
}
Expand Down
6 changes: 3 additions & 3 deletions ...maps-core/src/main/java/org/apache/baremaps/database/function/ChangeElementsImporter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ public ChangeElementsImporter(
/** {@inheritDoc} */
@Override
public void accept(Change change) {
switch (change.getType()) {
switch (change.type()) {
case CREATE, MODIFY -> put(change);
case DELETE -> delete(change);
}
}

private void put(Change change) {
var nodes = change.getEntities().stream()
var nodes = change.entities().stream()
.filter(type::isInstance)
.map(type::cast)
.toList();
Expand All @@ -70,7 +70,7 @@ private void put(Change change) {
}

private void delete(Change change) {
var nodes = change.getEntities().stream()
var nodes = change.entities().stream()
.filter(type::isInstance)
.map(type::cast)
.map(Element::getId)
Expand Down
66 changes: 28 additions & 38 deletions baremaps-core/src/main/java/org/apache/baremaps/database/function/CopyChangeImporter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,11 @@
package org.apache.baremaps.database.function;


import java.util.List;
import java.util.function.Consumer;
import org.apache.baremaps.database.postgres.Repository;
import org.apache.baremaps.database.postgres.RepositoryException;
import org.apache.baremaps.openstreetmap.model.Change;
import org.apache.baremaps.openstreetmap.model.Node;
import org.apache.baremaps.openstreetmap.model.Relation;
import org.apache.baremaps.openstreetmap.model.Way;
import org.apache.baremaps.openstreetmap.model.*;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -56,57 +54,49 @@ public CopyChangeImporter(
/** {@inheritDoc} */
@Override
public void accept(Change change) {
var nodes = change.getEntities().stream()
var nodes = change.entities().stream()
.filter(entity -> entity instanceof Node)
.map(entity -> (Node) entity)
.toList();
var nodeIds = nodes.stream().map(Node::getId).toList();
var ways = change.getEntities().stream()
var ways = change.entities().stream()
.filter(entity -> entity instanceof Way)
.map(entity -> (Way) entity)
.toList();
var wayIds = ways.stream().map(Way::getId).toList();
var relations = change.getEntities().stream()
var relations = change.entities().stream()
.filter(entity -> entity instanceof Relation)
.map(entity -> (Relation) entity)
.toList();
var relationIds = relations.stream().map(Relation::getId).toList();
try {
switch (change.getType()) {
switch (change.type()) {
case CREATE, MODIFY -> {
if (!nodes.isEmpty()) {
logger.trace("Creating {} nodes", nodes.size());
nodeRepository.delete(nodeIds);
nodeRepository.copy(nodes);
}
if (!ways.isEmpty()) {
logger.trace("Creating {} ways", ways.size());
wayRepository.delete(wayIds);
wayRepository.copy(ways);
}
if (!relations.isEmpty()) {
logger.trace("Creating {} relations", relations.size());
relationRepository.delete(relationIds);
relationRepository.copy(relations);
}
copy(nodeRepository, nodes);
copy(wayRepository, ways);
copy(relationRepository, relations);
}
case DELETE -> {
if (!nodes.isEmpty()) {
logger.trace("Deleting {} nodes", nodes.size());
nodeRepository.delete(nodeIds);
}
if (!ways.isEmpty()) {
logger.trace("Deleting {} ways", ways.size());
wayRepository.delete(wayIds);
}
if (!relations.isEmpty()) {
logger.trace("Deleting {} relations", relations.size());
relationRepository.delete(relationIds);
}
delete(nodeRepository, nodes);
delete(wayRepository, ways);
delete(relationRepository, relations);
}
}
} catch (RepositoryException e) {
logger.error("Error while saving changes", e);
}
}

private <T extends Element> void copy(Repository<Long, T> repository, List<T> entities)
throws RepositoryException {
if (!entities.isEmpty()) {
delete(repository, entities);
repository.copy(entities);
}
}

private <T extends Element> void delete(Repository<Long, T> repository, List<T> entities)
throws RepositoryException {
List<Long> ids = entities.stream().map(Element::getId).toList();
if (!ids.isEmpty()) {
repository.delete(ids);
}
}
}
63 changes: 28 additions & 35 deletions baremaps-core/src/main/java/org/apache/baremaps/database/function/PutChangeImporter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,11 @@



import java.util.List;
import java.util.function.Consumer;
import org.apache.baremaps.database.postgres.Repository;
import org.apache.baremaps.database.postgres.RepositoryException;
import org.apache.baremaps.openstreetmap.model.Change;
import org.apache.baremaps.openstreetmap.model.Node;
import org.apache.baremaps.openstreetmap.model.Relation;
import org.apache.baremaps.openstreetmap.model.Way;
import org.apache.baremaps.openstreetmap.model.*;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -57,54 +55,49 @@ public PutChangeImporter(
/** {@inheritDoc} */
@Override
public void accept(Change change) {
var nodes = change.getEntities().stream()
var nodes = change.entities().stream()
.filter(entity -> entity instanceof Node)
.map(entity -> (Node) entity)
.toList();
var nodeIds = nodes.stream().map(Node::getId).toList();
var ways = change.getEntities().stream()
var ways = change.entities().stream()
.filter(entity -> entity instanceof Way)
.map(entity -> (Way) entity)
.toList();
var wayIds = ways.stream().map(Way::getId).toList();
var relations = change.getEntities().stream()
var relations = change.entities().stream()
.filter(entity -> entity instanceof Relation)
.map(entity -> (Relation) entity)
.toList();
var relationIds = relations.stream().map(Relation::getId).toList();
try {
switch (change.getType()) {
switch (change.type()) {
case CREATE, MODIFY -> {
if (!nodes.isEmpty()) {
logger.trace("Creating {} nodes", nodes.size());
nodeRepository.put(nodes);
}
if (!ways.isEmpty()) {
logger.trace("Creating {} ways", ways.size());
wayRepository.put(ways);
}
if (!relations.isEmpty()) {
logger.trace("Creating {} relations", relations.size());
relationRepository.put(relations);
}
put(nodeRepository, nodes);
put(wayRepository, ways);
put(relationRepository, relations);
}
case DELETE -> {
if (!nodes.isEmpty()) {
logger.trace("Deleting {} nodes", nodes.size());
nodeRepository.delete(nodeIds);
}
if (!ways.isEmpty()) {
logger.trace("Deleting {} ways", ways.size());
wayRepository.delete(wayIds);
}
if (!relations.isEmpty()) {
logger.trace("Deleting {} relations", relations.size());
relationRepository.delete(relationIds);
}
delete(nodeRepository, nodes);
delete(wayRepository, ways);
delete(relationRepository, relations);
}
}
} catch (RepositoryException e) {
logger.error("Error while saving changes", e);
}
}

private <T extends Element> void put(Repository<Long, T> repository, List<T> entities)
throws RepositoryException {
if (!entities.isEmpty()) {
repository.put(entities);
}
}

private <T extends Element> void delete(Repository<Long, T> repository, List<T> entities)
throws RepositoryException {
List<Long> ids = entities.stream().map(Element::getId).toList();
if (!ids.isEmpty()) {
repository.delete(ids);
}
}

}
45 changes: 29 additions & 16 deletions baremaps-core/src/main/java/org/apache/baremaps/database/postgres/HeaderRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,15 @@ public class HeaderRepository implements Repository<Long, Header> {
* @param dataSource
*/
public HeaderRepository(DataSource dataSource) {
this(dataSource, "public", "osm_headers", "replication_sequence_number",
this(
dataSource,
"public",
"osm_headers",
"replication_sequence_number",
"replication_timestamp",
"replication_url", "source", "writing_program");
"replication_url",
"source",
"writing_program");
}

/**
Expand All @@ -81,9 +87,16 @@ public HeaderRepository(DataSource dataSource) {
* @param sourceColumn
* @param writingProgramColumn
*/
public HeaderRepository(DataSource dataSource, String schema, String table,
String replicationSequenceNumberColumn, String replicationTimestampColumn,
String replicationUrlColumn, String sourceColumn, String writingProgramColumn) {
@SuppressWarnings("squid:S107")
public HeaderRepository(
DataSource dataSource,
String schema,
String table,
String replicationSequenceNumberColumn,
String replicationTimestampColumn,
String replicationUrlColumn,
String sourceColumn,
String writingProgramColumn) {
var fullTableName = String.format("%1$s.%2$s", schema, table);
this.dataSource = dataSource;
this.createTable = String.format("""
Expand Down Expand Up @@ -219,7 +232,7 @@ public List<Header> get(List<Long> keys) throws RepositoryException {
Map<Long, Header> values = new HashMap<>();
while (result.next()) {
Header value = getValue(result);
values.put(value.getReplicationSequenceNumber(), value);
values.put(value.replicationSequenceNumber(), value);
}
return keys.stream().map(values::get).toList();
}
Expand Down Expand Up @@ -302,11 +315,11 @@ public void copy(List<Header> values) throws RepositoryException {
writer.writeHeader();
for (Header value : values) {
writer.startRow(5);
writer.writeLong(value.getReplicationSequenceNumber());
writer.writeLocalDateTime(value.getReplicationTimestamp());
writer.write(value.getReplicationUrl());
writer.write(value.getSource());
writer.write(value.getWritingProgram());
writer.writeLong(value.replicationSequenceNumber());
writer.writeLocalDateTime(value.replicationTimestamp());
writer.write(value.replicationUrl());
writer.write(value.source());
writer.write(value.writingProgram());
}
}
} catch (IOException | SQLException e) {
Expand All @@ -325,10 +338,10 @@ private Header getValue(ResultSet resultSet) throws SQLException {
}

private void setValue(PreparedStatement statement, Header value) throws SQLException {
statement.setObject(1, value.getReplicationSequenceNumber());
statement.setObject(2, value.getReplicationTimestamp());
statement.setObject(3, value.getReplicationUrl());
statement.setObject(4, value.getSource());
statement.setObject(5, value.getWritingProgram());
statement.setObject(1, value.replicationSequenceNumber());
statement.setObject(2, value.replicationTimestamp());
statement.setObject(3, value.replicationUrl());
statement.setObject(4, value.source());
statement.setObject(5, value.writingProgram());
}
}
12 changes: 6 additions & 6 deletions baremaps-core/src/main/java/org/apache/baremaps/database/postgres/RelationRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,11 +315,11 @@ public void copy(List<Relation> values) throws RepositoryException {
writer.writeLong(value.getInfo().getChangeset());
writer.writeJsonb(JsonbMapper.toJson(value.getTags()));
writer.writeLongList(
value.getMembers().stream().map(Member::getRef).toList());
writer.writeIntegerList(value.getMembers().stream().map(Member::getType)
value.getMembers().stream().map(Member::ref).toList());
writer.writeIntegerList(value.getMembers().stream().map(Member::type)
.map(MemberType::ordinal).toList());
writer
.write(value.getMembers().stream().map(Member::getRole).toList());
.write(value.getMembers().stream().map(Member::role).toList());
writer.writeGeometry(value.getGeometry());
}
}
Expand Down Expand Up @@ -355,12 +355,12 @@ private void setValue(PreparedStatement statement, Relation value)
statement.setObject(4, value.getInfo().getTimestamp());
statement.setObject(5, value.getInfo().getChangeset());
statement.setObject(6, JsonbMapper.toJson(value.getTags()));
Object[] refs = value.getMembers().stream().map(Member::getRef).toArray();
Object[] refs = value.getMembers().stream().map(Member::ref).toArray();
statement.setObject(7, statement.getConnection().createArrayOf("bigint", refs));
Object[] types =
value.getMembers().stream().map(Member::getType).map(MemberType::ordinal).toArray();
value.getMembers().stream().map(Member::type).map(MemberType::ordinal).toArray();
statement.setObject(8, statement.getConnection().createArrayOf("int", types));
Object[] roles = value.getMembers().stream().map(Member::getRole).toArray();
Object[] roles = value.getMembers().stream().map(Member::role).toArray();
statement.setObject(9, statement.getConnection().createArrayOf("varchar", roles));
statement.setBytes(10, GeometryUtils.serialize(value.getGeometry()));
}
Expand Down
2 changes: 1 addition & 1 deletion baremaps-core/src/main/java/org/apache/baremaps/storage/flatgeobuf/FlatGeoBufDataTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public class FlatGeoBufDataTable implements DataTable {

private final Path file;

private DataSchema schema;
private final DataSchema schema;

/**
* Constructs a table from a flatgeobuf file (used for reading).
Expand Down
Loading

0 comments on commit fc64ee1

Please sign in to comment.