added enum values for update & delete

[jvh-iodigital]

This PR adds keys for update & delete operations, although these might not be sensible in the current context https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-bulk.html

The enum value lookup was implemented in a suboptimal way, which has been improved with a HashMap cache see https://github.com/jorimvanhove/operation-enum-micro-optimizations

Also added a warning when the operation input is invalid

11:08:38,785 |-WARN in com.agido.logback.elasticsearch.ElasticsearchAppender[elk-errors] - Invalid value provided for [operation] setting, assuming CREATE

[agido-jrieks]

@jvh-iodigital thank you for for contribution! the micro optimization makes the code quite complex (and thus error prone), what is your motivation for that? If I see it right, that code is only executed once during startup, so it shouldn't make any real difference?

[jvh-iodigital]

My motivation for optimising the enum value lookup logic was precisely because the lookup happens during application startup, and I wanted to minimise the impact the package has on overall start up time. Of course, as demonstrated here, the difference is mere nanoseconds and negligible in this use case. I can revert this logic to the initial implementation if preferred.

[agido-jrieks]

It would be good if you could do that, thank you. In my experience, such premature optimizations are causing more problems than they solve. It is also very unlikely that your code is faster now, because your benchmark (if i understand it correctly) does not include the required static initialization. BTW, enum.valueOf also uses a HashMap internally (see Class.enumConstantDirectory() in JDK) - so your optimization is now doing more during startup (building a whole map) compared to the single valueOf call previously.

[jvh-iodigital]

The initial implementation (below) for the enum value lookup did not use the native valueOf, which is case-sensitive.

public static Optional<Operation> of( String label ) {
        return EnumSet.allOf( Operation.class )
                      .stream()
                      .filter( op -> op.label.equalsIgnoreCase( label ) )
                      .findFirst();

Just to be sure, do I revert back to this custom implementation?

[agido-jrieks]

Okay, sorry, of course! I totally missed the case difference here.. maybe we could just use lowercase enum values instead, we then also do not have to pass it in as a string. But i'm also fine with just using the previous code here.

[jvh-iodigital]

I simplified the enum value lookup logic, using the native valueOf wrapped with a try/catch and returning an Optional. The lookup is still robust against invalid inputs, but no longer case-insensitive, which was never actually documented so should not be an issue.

[agido-jrieks]

@jvh-iodigital thank you very much!

[agido-jrieks]

v3.0.8 released

[jvh-iodigital]

thanks for the feedback and the new release!