Support for log templates #202
Conversation
| @@ -185,7 +241,7 @@ void elasticIndexValidation(ElasticsearchClient client) { | |||
| // Olog Logbook Index | |||
| try (InputStream is = ElasticConfig.class.getResourceAsStream("/logbook_mapping.json")) { | |||
| BooleanResponse exits = client.indices().exists(ExistsRequest.of(e -> e.index(ES_LOGBOOK_INDEX))); | |||
| if(!exits.value()) { | |||
| if (!exits.value()) { | |||
There was a problem hiding this comment.
Should this variable be called exists? Same question for several instances below.
There was a problem hiding this comment.
It's local scope so I do not think it matters that much.
There was a problem hiding this comment.
I thought for increased readability.
|
|
||
| @Override | ||
| public long count() { | ||
| return 0; |
There was a problem hiding this comment.
Should this be the constant 0?
There was a problem hiding this comment.
Unused override, so it does not matter that much in my opinion. If we need to expose the count in the REST API or internally, we can implement the actual count.
There was a problem hiding this comment.
If it's unused, should the function count() be removed from the interface?
There was a problem hiding this comment.
Third party interface, mandates implementation
|
|
||
| @Override | ||
| public void deleteAll(Iterable<? extends LogTemplate> entities) { | ||
| throw new ResponseStatusException(HttpStatus.NOT_FOUND, TextUtil.LOG_TEMPLATE_DELETE_ALL_NOT_SUPPORTED); |
There was a problem hiding this comment.
Why is this not impemented when delete(LogTemplate logTemplate) and deleteById() are implemented? Same question for deleteAll() and deleteAllById(Iterable ids) below.
There was a problem hiding this comment.
Since there is currently no use case for that in clients.
There was a problem hiding this comment.
As above, should the functions then be removed from the interface if they are unused?
There was a problem hiding this comment.
Third party interface, mandates implementation
| d.index(ElasticConfig.ES_LOG_TEMPLATE_INDEX).id(id).refresh(Refresh.True)); | ||
| DeleteResponse deleteResponse = client.delete(deleteRequest); | ||
| if (deleteResponse.result().equals(Result.Deleted)) { | ||
| logger.log(Level.WARNING, MessageFormat.format(TextUtil.LOG_TEMPLATE_DELETED, id)); |
There was a problem hiding this comment.
Why is a warning logged when the template is deleted (which is the purpose of calling the function)?
There was a problem hiding this comment.
Agree, will change to INFO
| * @return The updated {@link LogTemplate} record, or HTTP status 404 if the log template does not exist. If the path | ||
| * variable does not match the id in the log record, HTTP status 400 (bad request) is returned. | ||
| */ | ||
| /* |
There was a problem hiding this comment.
Sorry, commented out for the time being, once client UI is in place (and authorization) this will be uncommented.
See also #201