Skip to content

Commit 48fc7cd

Browse files
mprinsmprins
committed
Apply code reviews
1 parent 545151e commit 48fc7cd

5 files changed

Lines changed: 30 additions & 19 deletions

File tree

‎.github/workflows/qa.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ jobs:
2626
- name: 'Dependency Review'
2727
uses: actions/dependency-review-action@v4
2828
with:
29+
allow-dependencies-licenses: pkg:maven/org.eclipse.angus/jakarta.mail@2.0.5?type=jar, pkg:maven/jakarta.ws.rs:jakarta.ws.rs-api@4.0.0?type=jar
2930
allow-licenses: MIT, Apache-2.0, BSD-2-Clause, BSD-3-Clause, ISC, MPL-2.0, LGPL-2.1-only, EPL-2.0, EPL-1.0
3031
fail-on-severity: low
3132
comment-summary-in-pr: always

‎pom.xml‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,12 @@ SPDX-License-Identifier: MIT
596596
<groupId>org.springframework.boot</groupId>
597597
<artifactId>spring-boot-starter-actuator-test</artifactId>
598598
<scope>test</scope>
599+
<exclusions>
600+
<exclusion>
601+
<groupId>org.assertj</groupId>
602+
<artifactId>assertj-core</artifactId>
603+
</exclusion>
604+
</exclusions>
599605
</dependency>
600606
<dependency>
601607
<groupId>org.springframework.boot</groupId>
@@ -1446,12 +1452,12 @@ SPDX-License-Identifier: MIT
14461452
<exclude>org.hamcrest:hamcrest-all</exclude>
14471453
<!-- slf4j should be used instead -->
14481454
<exclude>log4j:log4j</exclude>
1449-
<exclude>commons-logging:commons-logging.(,1.2]</exclude>
1450-
<exclude>commons-logging:commons-logging-api.(,1.2]</exclude>
1455+
<exclude>commons-logging:commons-logging:(,1.2]</exclude>
1456+
<exclude>commons-logging:commons-logging-api:(,1.2]</exclude>
14511457
<exclude>org.apache.logging.log4j:log4j-core</exclude>
14521458
<exclude>org.slf4j:slf4j-simple</exclude>
14531459
<exclude>joda-time:joda-time</exclude>
1454-
<exclude>org.assertj:assertj-core.(3.12,]</exclude>
1460+
<exclude>org.assertj:assertj-core:(3.12,]</exclude>
14551461
</excludes>
14561462
</bannedDependencies>
14571463
<RestrictImports>

‎src/main/java/org/tailormap/api/controller/PageController.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ private PageResponse getPageResponse(Page page) {
9999
.filter(JsonNode::isArray)
100100
.map(jsonNode -> {
101101
try {
102-
return Arrays.asList(new JsonMapper().treeToValue(jsonNode, MenuItem[].class));
102+
return Arrays.asList(JsonMapper.shared().treeToValue(jsonNode, MenuItem[].class));
103103
} catch (JacksonException e) {
104104
throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, null, e);
105105
}
@@ -129,8 +129,7 @@ private PageResponse getPageResponse(Page page) {
129129
* @param tile The page tile configuration
130130
* @return A page tile for the viewer with a boolean set to whether the tile should not be shown (filtered).
131131
*/
132-
// TODO check: private
133-
ViewerPageTileResult convert(PageTile tile) {
132+
private ViewerPageTileResult convert(PageTile tile) {
134133
ViewerPageTile viewerPageTile = new ViewerPageTile();
135134
ViewerPageTileResult result = new ViewerPageTileResult();
136135
result.viewerPageTile = viewerPageTile;

‎src/main/java/org/tailormap/api/controller/admin/SolrAdminController.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@
5555
import org.tailormap.api.solr.SolrHelper;
5656
import org.tailormap.api.solr.SolrService;
5757
import org.tailormap.api.viewer.model.ErrorResponse;
58-
import tools.jackson.core.JacksonException;
5958
import tools.jackson.databind.json.JsonMapper;
6059

6160
/** Admin controller for Solr. */
@@ -69,6 +68,7 @@ public class SolrAdminController implements InitializingBean {
6968
private final SolrService solrService;
7069
private final Scheduler scheduler;
7170
private final TaskManagerService taskManagerService;
71+
private final JsonMapper jsonMapper;
7272

7373
@Value("${tailormap-api.solr-query-timeout-seconds:7}")
7474
private int solrQueryTimeout;
@@ -81,12 +81,14 @@ public SolrAdminController(
8181
@Autowired SearchIndexRepository searchIndexRepository,
8282
@Autowired SolrService solrService,
8383
@Autowired Scheduler scheduler,
84-
@Autowired TaskManagerService taskManagerService) {
84+
@Autowired TaskManagerService taskManagerService,
85+
JsonMapper jsonMapper) {
8586
this.featureTypeRepository = featureTypeRepository;
8687
this.searchIndexRepository = searchIndexRepository;
8788
this.solrService = solrService;
8889
this.scheduler = scheduler;
8990
this.taskManagerService = taskManagerService;
91+
this.jsonMapper = jsonMapper;
9092
}
9193

9294
@ExceptionHandler({ResponseStatusException.class})
@@ -128,11 +130,11 @@ public ResponseEntity<?> pingSolr() {
128130
final SolrPingResponse ping = solrClient.ping();
129131
logger.info("Solr ping status {}", ping.getResponse().get("status"));
130132
Metrics.timer("tailormap_solr_ping").record(ping.getElapsedTime(), TimeUnit.MILLISECONDS);
131-
return ResponseEntity.ok(new JsonMapper()
133+
return ResponseEntity.ok(this.jsonMapper
132134
.createObjectNode()
133135
.put("status", ping.getResponse().get("status").toString())
134136
.put("timeElapsed", ping.getElapsedTime()));
135-
} catch (JacksonException | IOException | SolrServerException e) {
137+
} catch (IOException | SolrServerException e) {
136138
logger.error("Error pinging solr", e);
137139
throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, e.getMessage());
138140
}

‎src/main/java/org/tailormap/api/controller/admin/TaskAdminController.java‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,17 @@ public class TaskAdminController {
7272
private final Scheduler scheduler;
7373
private final TaskManagerService taskManagerService;
7474
private final SearchIndexRepository searchIndexRepository;
75+
private final JsonMapper jsonMapper;
7576

7677
public TaskAdminController(
7778
@Autowired Scheduler scheduler,
7879
@Autowired TaskManagerService taskManagerService,
79-
@Autowired SearchIndexRepository searchIndexRepository) {
80+
@Autowired SearchIndexRepository searchIndexRepository,
81+
JsonMapper jsonMapper) {
8082
this.scheduler = scheduler;
8183
this.taskManagerService = taskManagerService;
8284
this.searchIndexRepository = searchIndexRepository;
85+
this.jsonMapper = jsonMapper;
8386
}
8487

8588
@Operation(summary = "List all tasks, optionally filtered by type", description = """
@@ -127,7 +130,7 @@ public ResponseEntity<Object> list(@RequestParam(required = false) String type)
127130
logger.error("Error getting task state", e);
128131
state = null;
129132
}
130-
tasks.add(new JsonMapper()
133+
tasks.add(this.jsonMapper
131134
.createObjectNode()
132135
.put(Task.TYPE_KEY, jobDetail.getKey().getGroup())
133136
.put(Task.UUID_KEY, jobDetail.getKey().getName())
@@ -146,9 +149,9 @@ public ResponseEntity<Object> list(@RequestParam(required = false) String type)
146149
throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Error getting tasks", e);
147150
}
148151

149-
return ResponseEntity.ok(new JsonMapper()
152+
return ResponseEntity.ok(this.jsonMapper
150153
.createObjectNode()
151-
.set("tasks", new JsonMapper().createArrayNode().addAll(tasks)));
154+
.set("tasks", this.jsonMapper.createArrayNode().addAll(tasks)));
152155
}
153156

154157
@Operation(summary = "List all details for a given task", description = """
@@ -217,7 +220,7 @@ public ResponseEntity<Object> details(@PathVariable TaskType type, @PathVariable
217220
result[0] = jobExecutionContext.getResult();
218221
});
219222

220-
return ResponseEntity.ok(new JsonMapper()
223+
return ResponseEntity.ok(this.jsonMapper
221224
.createObjectNode()
222225
// immutable uuid, type and description
223226
.put(Task.TYPE_KEY, jobDetail.getKey().getGroup())
@@ -277,7 +280,7 @@ public ResponseEntity<Object> startTask(@PathVariable TaskType type, @PathVariab
277280
}
278281
scheduler.triggerJob(jobKey);
279282
return ResponseEntity.status(HttpStatusCode.valueOf(HTTP_ACCEPTED))
280-
.body(new JsonMapper().createObjectNode().put("message", "Task starting accepted"));
283+
.body(this.jsonMapper.createObjectNode().put("message", "Task starting accepted"));
281284

282285
} catch (SchedulerException e) {
283286
throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Error getting task", e);
@@ -329,13 +332,13 @@ public ResponseEntity<Object> stopTask(@PathVariable TaskType type, @PathVariabl
329332
scheduler.getJobDetail(jobKey).getJobClass())) {
330333
boolean interrupted = scheduler.interrupt(jobKey);
331334
return ResponseEntity.status(HttpStatusCode.valueOf(HTTP_ACCEPTED))
332-
.body(new JsonMapper()
335+
.body(this.jsonMapper
333336
.createObjectNode()
334337
.put("message", "Task stopping accepted")
335338
.put("succes", interrupted));
336339
} else {
337340
return ResponseEntity.status(HttpStatusCode.valueOf(HTTP_BAD_REQUEST))
338-
.body(new JsonMapper()
341+
.body(this.jsonMapper
339342
.createObjectNode()
340343
.put("message", "Task cannot be stopped")
341344
.put("succes", false));
@@ -397,7 +400,7 @@ private void deleteScheduleFromSearchIndex(UUID uuid) {
397400
private ResponseEntity<Object> handleTaskNotFound() {
398401
return ResponseEntity.status(HttpStatusCode.valueOf(HTTP_NOT_FOUND))
399402
.contentType(MediaType.APPLICATION_JSON)
400-
.body(new JsonMapper()
403+
.body(this.jsonMapper
401404
.createObjectNode()
402405
.put("message", "Task not found")
403406
.put("code", HTTP_NOT_FOUND));

0 commit comments

Comments
 (0)