Skip to content

Commit fc64ee1

Browse files
bchapuisbchapuis
authored
Fix some issues detected by sonar (#873)
* 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
1 parent 47b3922 commit fc64ee1

File tree

68 files changed

+561
-929
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

68 files changed

+561
-929
lines changed

‎baremaps-core/src/main/java/org/apache/baremaps/database/DiffService.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ public class DiffService implements Callable<List<TileCoord>> {
5555
private final int srid;
5656
private final int zoom;
5757

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

8182
var header = headerRepository.selectLatest();
82-
var replicationUrl = header.getReplicationUrl();
83-
var sequenceNumber = header.getReplicationSequenceNumber() + 1;
83+
var replicationUrl = header.replicationUrl();
84+
var sequenceNumber = header.replicationSequenceNumber() + 1;
8485
var changeUrl = resolve(replicationUrl, sequenceNumber, "osc.gz");
8586

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

102103
private Stream<Geometry> geometriesForChange(Change change) {
103-
switch (change.getType()) {
104+
switch (change.type()) {
104105
case CREATE:
105106
return geometriesForNextVersion(change);
106107
case DELETE:
@@ -114,7 +115,7 @@ private Stream<Geometry> geometriesForChange(Change change) {
114115
}
115116

116117
private Stream<Geometry> geometriesForPreviousVersion(Change change) {
117-
return change.getEntities().stream().map(this::geometriesForPreviousVersion)
118+
return change.entities().stream().map(this::geometriesForPreviousVersion)
118119
.flatMap(Optional::stream);
119120
}
120121

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

140141
private Stream<Geometry> geometriesForNextVersion(Change change) {
141-
return change.getEntities().stream()
142+
return change.entities().stream()
142143
.map(consumeThenReturn(new EntityGeometryBuilder(coordinateMap, referenceMap)))
143144
.flatMap(new EntityToGeometryMapper().andThen(Optional::stream));
144145
}

‎baremaps-core/src/main/java/org/apache/baremaps/database/function/ChangeElementsImporter.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,14 @@ public ChangeElementsImporter(
4949
/** {@inheritDoc} */
5050
@Override
5151
public void accept(Change change) {
52-
switch (change.getType()) {
52+
switch (change.type()) {
5353
case CREATE, MODIFY -> put(change);
5454
case DELETE -> delete(change);
5555
}
5656
}
5757

5858
private void put(Change change) {
59-
var nodes = change.getEntities().stream()
59+
var nodes = change.entities().stream()
6060
.filter(type::isInstance)
6161
.map(type::cast)
6262
.toList();
@@ -70,7 +70,7 @@ private void put(Change change) {
7070
}
7171

7272
private void delete(Change change) {
73-
var nodes = change.getEntities().stream()
73+
var nodes = change.entities().stream()
7474
.filter(type::isInstance)
7575
.map(type::cast)
7676
.map(Element::getId)

‎baremaps-core/src/main/java/org/apache/baremaps/database/function/CopyChangeImporter.java

Lines changed: 28 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,11 @@
1818
package org.apache.baremaps.database.function;
1919

2020

21+
import java.util.List;
2122
import java.util.function.Consumer;
2223
import org.apache.baremaps.database.postgres.Repository;
2324
import org.apache.baremaps.database.postgres.RepositoryException;
24-
import org.apache.baremaps.openstreetmap.model.Change;
25-
import org.apache.baremaps.openstreetmap.model.Node;
26-
import org.apache.baremaps.openstreetmap.model.Relation;
27-
import org.apache.baremaps.openstreetmap.model.Way;
25+
import org.apache.baremaps.openstreetmap.model.*;
2826
import org.slf4j.Logger;
2927
import org.slf4j.LoggerFactory;
3028

@@ -56,57 +54,49 @@ public CopyChangeImporter(
5654
/** {@inheritDoc} */
5755
@Override
5856
public void accept(Change change) {
59-
var nodes = change.getEntities().stream()
57+
var nodes = change.entities().stream()
6058
.filter(entity -> entity instanceof Node)
6159
.map(entity -> (Node) entity)
6260
.toList();
63-
var nodeIds = nodes.stream().map(Node::getId).toList();
64-
var ways = change.getEntities().stream()
61+
var ways = change.entities().stream()
6562
.filter(entity -> entity instanceof Way)
6663
.map(entity -> (Way) entity)
6764
.toList();
68-
var wayIds = ways.stream().map(Way::getId).toList();
69-
var relations = change.getEntities().stream()
65+
var relations = change.entities().stream()
7066
.filter(entity -> entity instanceof Relation)
7167
.map(entity -> (Relation) entity)
7268
.toList();
73-
var relationIds = relations.stream().map(Relation::getId).toList();
7469
try {
75-
switch (change.getType()) {
70+
switch (change.type()) {
7671
case CREATE, MODIFY -> {
77-
if (!nodes.isEmpty()) {
78-
logger.trace("Creating {} nodes", nodes.size());
79-
nodeRepository.delete(nodeIds);
80-
nodeRepository.copy(nodes);
81-
}
82-
if (!ways.isEmpty()) {
83-
logger.trace("Creating {} ways", ways.size());
84-
wayRepository.delete(wayIds);
85-
wayRepository.copy(ways);
86-
}
87-
if (!relations.isEmpty()) {
88-
logger.trace("Creating {} relations", relations.size());
89-
relationRepository.delete(relationIds);
90-
relationRepository.copy(relations);
91-
}
72+
copy(nodeRepository, nodes);
73+
copy(wayRepository, ways);
74+
copy(relationRepository, relations);
9275
}
9376
case DELETE -> {
94-
if (!nodes.isEmpty()) {
95-
logger.trace("Deleting {} nodes", nodes.size());
96-
nodeRepository.delete(nodeIds);
97-
}
98-
if (!ways.isEmpty()) {
99-
logger.trace("Deleting {} ways", ways.size());
100-
wayRepository.delete(wayIds);
101-
}
102-
if (!relations.isEmpty()) {
103-
logger.trace("Deleting {} relations", relations.size());
104-
relationRepository.delete(relationIds);
105-
}
77+
delete(nodeRepository, nodes);
78+
delete(wayRepository, ways);
79+
delete(relationRepository, relations);
10680
}
10781
}
10882
} catch (RepositoryException e) {
10983
logger.error("Error while saving changes", e);
11084
}
11185
}
86+
87+
private <T extends Element> void copy(Repository<Long, T> repository, List<T> entities)
88+
throws RepositoryException {
89+
if (!entities.isEmpty()) {
90+
delete(repository, entities);
91+
repository.copy(entities);
92+
}
93+
}
94+
95+
private <T extends Element> void delete(Repository<Long, T> repository, List<T> entities)
96+
throws RepositoryException {
97+
List<Long> ids = entities.stream().map(Element::getId).toList();
98+
if (!ids.isEmpty()) {
99+
repository.delete(ids);
100+
}
101+
}
112102
}

‎baremaps-core/src/main/java/org/apache/baremaps/database/function/PutChangeImporter.java

Lines changed: 28 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,11 @@
1919

2020

2121

22+
import java.util.List;
2223
import java.util.function.Consumer;
2324
import org.apache.baremaps.database.postgres.Repository;
2425
import org.apache.baremaps.database.postgres.RepositoryException;
25-
import org.apache.baremaps.openstreetmap.model.Change;
26-
import org.apache.baremaps.openstreetmap.model.Node;
27-
import org.apache.baremaps.openstreetmap.model.Relation;
28-
import org.apache.baremaps.openstreetmap.model.Way;
26+
import org.apache.baremaps.openstreetmap.model.*;
2927
import org.slf4j.Logger;
3028
import org.slf4j.LoggerFactory;
3129

@@ -57,54 +55,49 @@ public PutChangeImporter(
5755
/** {@inheritDoc} */
5856
@Override
5957
public void accept(Change change) {
60-
var nodes = change.getEntities().stream()
58+
var nodes = change.entities().stream()
6159
.filter(entity -> entity instanceof Node)
6260
.map(entity -> (Node) entity)
6361
.toList();
64-
var nodeIds = nodes.stream().map(Node::getId).toList();
65-
var ways = change.getEntities().stream()
62+
var ways = change.entities().stream()
6663
.filter(entity -> entity instanceof Way)
6764
.map(entity -> (Way) entity)
6865
.toList();
69-
var wayIds = ways.stream().map(Way::getId).toList();
70-
var relations = change.getEntities().stream()
66+
var relations = change.entities().stream()
7167
.filter(entity -> entity instanceof Relation)
7268
.map(entity -> (Relation) entity)
7369
.toList();
74-
var relationIds = relations.stream().map(Relation::getId).toList();
7570
try {
76-
switch (change.getType()) {
71+
switch (change.type()) {
7772
case CREATE, MODIFY -> {
78-
if (!nodes.isEmpty()) {
79-
logger.trace("Creating {} nodes", nodes.size());
80-
nodeRepository.put(nodes);
81-
}
82-
if (!ways.isEmpty()) {
83-
logger.trace("Creating {} ways", ways.size());
84-
wayRepository.put(ways);
85-
}
86-
if (!relations.isEmpty()) {
87-
logger.trace("Creating {} relations", relations.size());
88-
relationRepository.put(relations);
89-
}
73+
put(nodeRepository, nodes);
74+
put(wayRepository, ways);
75+
put(relationRepository, relations);
9076
}
9177
case DELETE -> {
92-
if (!nodes.isEmpty()) {
93-
logger.trace("Deleting {} nodes", nodes.size());
94-
nodeRepository.delete(nodeIds);
95-
}
96-
if (!ways.isEmpty()) {
97-
logger.trace("Deleting {} ways", ways.size());
98-
wayRepository.delete(wayIds);
99-
}
100-
if (!relations.isEmpty()) {
101-
logger.trace("Deleting {} relations", relations.size());
102-
relationRepository.delete(relationIds);
103-
}
78+
delete(nodeRepository, nodes);
79+
delete(wayRepository, ways);
80+
delete(relationRepository, relations);
10481
}
10582
}
10683
} catch (RepositoryException e) {
10784
logger.error("Error while saving changes", e);
10885
}
10986
}
87+
88+
private <T extends Element> void put(Repository<Long, T> repository, List<T> entities)
89+
throws RepositoryException {
90+
if (!entities.isEmpty()) {
91+
repository.put(entities);
92+
}
93+
}
94+
95+
private <T extends Element> void delete(Repository<Long, T> repository, List<T> entities)
96+
throws RepositoryException {
97+
List<Long> ids = entities.stream().map(Element::getId).toList();
98+
if (!ids.isEmpty()) {
99+
repository.delete(ids);
100+
}
101+
}
102+
110103
}

‎baremaps-core/src/main/java/org/apache/baremaps/database/postgres/HeaderRepository.java

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,15 @@ public class HeaderRepository implements Repository<Long, Header> {
6464
* @param dataSource
6565
*/
6666
public HeaderRepository(DataSource dataSource) {
67-
this(dataSource, "public", "osm_headers", "replication_sequence_number",
67+
this(
68+
dataSource,
69+
"public",
70+
"osm_headers",
71+
"replication_sequence_number",
6872
"replication_timestamp",
69-
"replication_url", "source", "writing_program");
73+
"replication_url",
74+
"source",
75+
"writing_program");
7076
}
7177

7278
/**
@@ -81,9 +87,16 @@ public HeaderRepository(DataSource dataSource) {
8187
* @param sourceColumn
8288
* @param writingProgramColumn
8389
*/
84-
public HeaderRepository(DataSource dataSource, String schema, String table,
85-
String replicationSequenceNumberColumn, String replicationTimestampColumn,
86-
String replicationUrlColumn, String sourceColumn, String writingProgramColumn) {
90+
@SuppressWarnings("squid:S107")
91+
public HeaderRepository(
92+
DataSource dataSource,
93+
String schema,
94+
String table,
95+
String replicationSequenceNumberColumn,
96+
String replicationTimestampColumn,
97+
String replicationUrlColumn,
98+
String sourceColumn,
99+
String writingProgramColumn) {
87100
var fullTableName = String.format("%1$s.%2$s", schema, table);
88101
this.dataSource = dataSource;
89102
this.createTable = String.format("""
@@ -219,7 +232,7 @@ public List<Header> get(List<Long> keys) throws RepositoryException {
219232
Map<Long, Header> values = new HashMap<>();
220233
while (result.next()) {
221234
Header value = getValue(result);
222-
values.put(value.getReplicationSequenceNumber(), value);
235+
values.put(value.replicationSequenceNumber(), value);
223236
}
224237
return keys.stream().map(values::get).toList();
225238
}
@@ -302,11 +315,11 @@ public void copy(List<Header> values) throws RepositoryException {
302315
writer.writeHeader();
303316
for (Header value : values) {
304317
writer.startRow(5);
305-
writer.writeLong(value.getReplicationSequenceNumber());
306-
writer.writeLocalDateTime(value.getReplicationTimestamp());
307-
writer.write(value.getReplicationUrl());
308-
writer.write(value.getSource());
309-
writer.write(value.getWritingProgram());
318+
writer.writeLong(value.replicationSequenceNumber());
319+
writer.writeLocalDateTime(value.replicationTimestamp());
320+
writer.write(value.replicationUrl());
321+
writer.write(value.source());
322+
writer.write(value.writingProgram());
310323
}
311324
}
312325
} catch (IOException | SQLException e) {
@@ -325,10 +338,10 @@ private Header getValue(ResultSet resultSet) throws SQLException {
325338
}
326339

327340
private void setValue(PreparedStatement statement, Header value) throws SQLException {
328-
statement.setObject(1, value.getReplicationSequenceNumber());
329-
statement.setObject(2, value.getReplicationTimestamp());
330-
statement.setObject(3, value.getReplicationUrl());
331-
statement.setObject(4, value.getSource());
332-
statement.setObject(5, value.getWritingProgram());
341+
statement.setObject(1, value.replicationSequenceNumber());
342+
statement.setObject(2, value.replicationTimestamp());
343+
statement.setObject(3, value.replicationUrl());
344+
statement.setObject(4, value.source());
345+
statement.setObject(5, value.writingProgram());
333346
}
334347
}

‎baremaps-core/src/main/java/org/apache/baremaps/database/postgres/RelationRepository.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -315,11 +315,11 @@ public void copy(List<Relation> values) throws RepositoryException {
315315
writer.writeLong(value.getInfo().getChangeset());
316316
writer.writeJsonb(JsonbMapper.toJson(value.getTags()));
317317
writer.writeLongList(
318-
value.getMembers().stream().map(Member::getRef).toList());
319-
writer.writeIntegerList(value.getMembers().stream().map(Member::getType)
318+
value.getMembers().stream().map(Member::ref).toList());
319+
writer.writeIntegerList(value.getMembers().stream().map(Member::type)
320320
.map(MemberType::ordinal).toList());
321321
writer
322-
.write(value.getMembers().stream().map(Member::getRole).toList());
322+
.write(value.getMembers().stream().map(Member::role).toList());
323323
writer.writeGeometry(value.getGeometry());
324324
}
325325
}
@@ -355,12 +355,12 @@ private void setValue(PreparedStatement statement, Relation value)
355355
statement.setObject(4, value.getInfo().getTimestamp());
356356
statement.setObject(5, value.getInfo().getChangeset());
357357
statement.setObject(6, JsonbMapper.toJson(value.getTags()));
358-
Object[] refs = value.getMembers().stream().map(Member::getRef).toArray();
358+
Object[] refs = value.getMembers().stream().map(Member::ref).toArray();
359359
statement.setObject(7, statement.getConnection().createArrayOf("bigint", refs));
360360
Object[] types =
361-
value.getMembers().stream().map(Member::getType).map(MemberType::ordinal).toArray();
361+
value.getMembers().stream().map(Member::type).map(MemberType::ordinal).toArray();
362362
statement.setObject(8, statement.getConnection().createArrayOf("int", types));
363-
Object[] roles = value.getMembers().stream().map(Member::getRole).toArray();
363+
Object[] roles = value.getMembers().stream().map(Member::role).toArray();
364364
statement.setObject(9, statement.getConnection().createArrayOf("varchar", roles));
365365
statement.setBytes(10, GeometryUtils.serialize(value.getGeometry()));
366366
}

‎baremaps-core/src/main/java/org/apache/baremaps/storage/flatgeobuf/FlatGeoBufDataTable.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public class FlatGeoBufDataTable implements DataTable {
4646

4747
private final Path file;
4848

49-
private DataSchema schema;
49+
private final DataSchema schema;
5050

5151
/**
5252
* Constructs a table from a flatgeobuf file (used for reading).

0 commit comments

Comments
 (0)