Skip to content

RestHighLevelClient impossible mocking #40534

New issue
New issue
Closed
Closed
RestHighLevelClient impossible mocking#40534
Labels
Team:Data ManagementMeta label for data/management team
@MatCuk

Description

@MatCuk
MatCuk
opened on Mar 27, 2019 · edited by dadoonet

Hi,

I'm trying to unit test some code using RestHighLevelClient.

I have

private synchronized void createIndex(String indexName) throws IOException {
        if (!client.exists(new GetRequest(indexName), RequestOptions.DEFAULT)) {
            CreateIndexRequest request = new CreateIndexRequest(indexName);
            request.mapping(indexType, ElasticSearchConstants.MAPPING_SOURCE);
            request.alias(new Alias(alias));
            request.settings(ElasticSearchConstants.MAPPING_SETTINGS, XContentType.JSON);
            client.indices().create(request, RequestOptions.DEFAULT);
        }
    }

Now i want to verify that client.indices().create was called.

RestHighLevelClient is not final and I was able to mock it.

Mock for RestHighLevelClient.indices() returns null by default.
So of course we want to mock that

when(elasticSearchClient.indices()).thenReturn(mock(IndicesClient.class));

BAM problem. IndicesClient is final class. I can't mock it.

So I can't test if create gets called and with which parameters.

IndicesClient should not be final class or indices() should return an interface and your final class implementation is hidden away.

This is pretty much basic stuff for public APIs guys.

Activity

polyfractal
added
:Core/Features/Java High Level REST Client
on Mar 27, 2019
elasticmachine

elasticmachine commented on Mar 27, 2019

@elasticmachine
elasticmachine
on Mar 27, 2019
Collaborator

Pinging @elastic/es-core-features

jloisel

jloisel commented on Jun 10, 2019

@jloisel
jloisel
on Jun 10, 2019

Same boat here. Makes it impossible to properly unit test the code => I have to wrap each client to hide it behind an interface which is then mockable. Questionable design here.

jloisel

jloisel commented on Jun 12, 2019

@jloisel
jloisel
on Jun 12, 2019 · edited by jloisel

I ended up wrapping the client like this:

import org.elasticsearch.action.bulk.BulkItemResponse;
import org.elasticsearch.action.bulk.BulkRequest;
import org.elasticsearch.action.get.MultiGetItemResponse;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.search.SearchScrollRequest;

import java.util.List;
import java.util.Optional;

public interface DatabaseClient {

  boolean exists(String index, String id);

  BulkItemResponse[] bulk(BulkRequest request);

  Optional<String> get(String index, String id);

  MultiGetItemResponse[] multiGet(String index, List<String> ids);

  SearchResponse search(SearchRequest request);

  SearchResponse searchScroll(SearchScrollRequest request);

  void clearScroll(String scrollId);
}

Then, providing a Spring configuration which instantiates the wrapper:

import com.octoperf.database.client.api.DatabaseClient;
import org.elasticsearch.client.IndicesClient;
import org.elasticsearch.client.RestHighLevelClient;
import org.elasticsearch.client.SnapshotClient;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

@Configuration
class ElasticsearchClientConfig {

  @Bean
  DatabaseClient databaseClient(final RestHighLevelClient client) {
    return new ElasticsearchClient(
      client::exists,
      client::bulk,
      client::get,
      client::mget,
      client::search,
      client::scroll,
      client::clearScroll
    );
  }
}

And the wrapper itself:

import com.octoperf.database.client.api.DatabaseClient;
import io.vavr.CheckedFunction2;
import lombok.AllArgsConstructor;
import lombok.NonNull;
import lombok.experimental.FieldDefaults;
import lombok.extern.slf4j.Slf4j;
import org.elasticsearch.action.bulk.BulkItemResponse;
import org.elasticsearch.action.bulk.BulkRequest;
import org.elasticsearch.action.bulk.BulkResponse;
import org.elasticsearch.action.get.*;
import org.elasticsearch.action.search.*;
import org.elasticsearch.client.RequestOptions;

import java.util.List;
import java.util.Optional;
import java.util.function.Consumer;

import static io.vavr.control.Try.of;
import static io.vavr.control.Try.success;
import static lombok.AccessLevel.PACKAGE;
import static lombok.AccessLevel.PRIVATE;
import static org.elasticsearch.client.RequestOptions.DEFAULT;
import static org.elasticsearch.search.fetch.subphase.FetchSourceContext.DO_NOT_FETCH_SOURCE;
import static org.elasticsearch.search.fetch.subphase.FetchSourceContext.FETCH_SOURCE;

@Slf4j
@AllArgsConstructor(access = PACKAGE)
@FieldDefaults(level = PRIVATE, makeFinal = true)
final class ElasticsearchClient implements DatabaseClient {
  public static final Consumer<? super Throwable> LOG_ERROR = e -> log.error("ElasticsearchClient", e);

  @NonNull
  CheckedFunction2<GetRequest, RequestOptions, Boolean> exists;
  @NonNull
  CheckedFunction2<BulkRequest, RequestOptions, BulkResponse> bulk;
  @NonNull
  CheckedFunction2<GetRequest, RequestOptions, GetResponse> get;
  @NonNull
  CheckedFunction2<MultiGetRequest, RequestOptions, MultiGetResponse> multiGet;
  @NonNull
  CheckedFunction2<SearchRequest, RequestOptions, SearchResponse> search;
  @NonNull
  CheckedFunction2<SearchScrollRequest, RequestOptions, SearchResponse> searchScroll;
  @NonNull
  CheckedFunction2<ClearScrollRequest, RequestOptions, ClearScrollResponse> clearScroll;

  @Override
  public boolean exists(final String index, final String id) {
    return success(new GetRequest(index, id))
      .map(r -> r.fetchSourceContext(DO_NOT_FETCH_SOURCE))
      .mapTry(r -> exists.apply(r, DEFAULT))
      .onFailure(LOG_ERROR)
      .get();
  }

  @Override
  public BulkItemResponse[] bulk(final BulkRequest request) {
    return of(() -> bulk.apply(request, DEFAULT))
      .map(BulkResponse::getItems)
      .onFailure(LOG_ERROR)
      .getOrElse(new BulkItemResponse[0]);
  }

  @Override
  public Optional<String> get(final String index, final String id) {
    return success(new GetRequest(index, id))
      .map(r -> r.fetchSourceContext(FETCH_SOURCE))
      .mapTry(r -> get.apply(r, DEFAULT))
      .filter(GetResponse::isExists)
      .map(GetResponse::getSourceAsString)
      .onFailure(LOG_ERROR)
      .toJavaOptional();
  }

  @Override
  public MultiGetItemResponse[] multiGet(final String index,
                                         final List<String> ids) {
    final MultiGetRequest multi = new MultiGetRequest();
    ids.forEach(id -> multi.add(index, id));

    return success(multi)
      .mapTry(r -> multiGet.apply(r, DEFAULT))
      .map(MultiGetResponse::getResponses)
      .getOrElse(new MultiGetItemResponse[0]);
  }

  @Override
  public SearchResponse search(final SearchRequest request) {
    return success(request).mapTry(r -> search.apply(r, DEFAULT)).get();
  }

  @Override
  public SearchResponse searchScroll(final SearchScrollRequest request) {
    return success(request).mapTry(r -> searchScroll.apply(r, DEFAULT)).get();
  }

  @Override
  public void clearScroll(final String scrollId) {
    final ClearScrollRequest request = new ClearScrollRequest();
    request.addScrollId(scrollId);

    of(() -> clearScroll.apply(request, DEFAULT));
  }
}

This way, I can fully mock every single function injected in the ElasticsearchClient. The RestHighLevelClient can be easily provided by instantiating it in JUnit.

davidkyle
mentioned this on Jun 12, 2019
dadoonet

dadoonet commented on Jun 12, 2019

@dadoonet
dadoonet
on Jun 12, 2019
Member

Thanks @jloisel for sharing your solution.

jloisel

jloisel commented on Jun 12, 2019

@jloisel
jloisel
on Jun 12, 2019

Sure no problem. Of course, it would be much nicer if the client would have exposed only interfaces. Even other classes like SearchRequest are causing mocking issue because its class is final. In that case, i'm instantiating the real object.

lxghtless

lxghtless commented on Sep 5, 2019

@lxghtless
lxghtless
on Sep 5, 2019

FWIW - I was able to use Mockito 2's mock-maker-inline to mock all the things. Here's a snippet of the body of a test that mocks every part used while scrolling.

	String testHitOneJson = "{...testjson}";
    String testScrollId = "unit-test-scroll-id";
    String testHitOneId = "hit-one-id";
    int testSize = 100;
    EsClientFactory clientFactory = mock(EsClientFactory.class);
    RestHighLevelClient restHighLevelClient = mock(RestHighLevelClient.class);
SearchResponse firstSearchResponse = mock(SearchResponse.class);
    SearchHits firstSearchHits = mock(SearchHits.class);
    SearchHit firstSearchHitsHitOne = mock(SearchHit.class);
    ClearScrollResponse clearScrollResponse = mock(ClearScrollResponse.class);

    when(firstSearchResponse.getScrollId()).thenReturn(testScrollId);
    when(firstSearchHitsHitOne.getId()).thenReturn(testHitOneId);
    when(firstSearchHitsHitOne.getSourceAsString()).thenReturn(testHitOneJson);
    when(firstSearchHits.getHits()).thenReturn(new SearchHit[] {firstSearchHitsHitOne});
    when(firstSearchResponse.getHits()).thenReturn(firstSearchHits);

    SearchResponse secondSearchResponse = mock(SearchResponse.class);
    SearchHits secondSearchHits = mock(SearchHits.class);

    when(secondSearchResponse.getScrollId()).thenReturn(testScrollId);
    when(secondSearchHits.getHits()).thenReturn(new SearchHit[] {});
    when(secondSearchResponse.getHits()).thenReturn(secondSearchHits);

    when(this.restHighLevelClient.search(any(SearchRequest.class), any(RequestOptions.class)))
        .thenReturn(firstSearchResponse);

    when(this.restHighLevelClient.scroll(any(SearchScrollRequest.class), any(RequestOptions.class)))
        .thenReturn(secondSearchResponse);

    when(this.clientFactory.buildClient()).thenReturn(this.restHighLevelClient);

    when(this.restHighLevelClient.clearScroll(
            any(ClearScrollRequest.class), any(RequestOptions.class)))
        .thenReturn(clearScrollResponse);

    doNothing().when(this.restHighLevelClient).close();

	// test things that use client

    // verify close and anything else needed
    verify(this.restHighLevelClient, times(1)).close();
hub-cap

hub-cap commented on Sep 9, 2019

@hub-cap
hub-cap
on Sep 9, 2019
Contributor

Just an fyi, I do not think we will be doing this for now, see #31065 (comment) for more information.

xeraa
mentioned this on Jan 24, 2020
rjernst
added
Team:Data ManagementMeta label for data/management team
on May 4, 2020
SaurabhRaoot

SaurabhRaoot commented on Feb 11, 2021

@SaurabhRaoot
SaurabhRaoot
on Feb 11, 2021

This article helped to resolve the issue https://www.baeldung.com/mockito-final

daidorian09

daidorian09 commented on Oct 14, 2021

@daidorian09
daidorian09
on Oct 14, 2021

We implemented this workaround in order to mock final class using Powermock as testing HighLevelRestClient

@RunWith(PowerMockRunner.class)
@PrepareForTest({RestHighLevelClient.class, SearchResponse.class})
public class HighLevelRestClientTest {

    @Captor
    private ArgumentCaptor<SearchRequest> searchRequestArgumentCaptor;

 @Test
    public void it_should_return_search_response() {
        //Given
        final String index = "index";
        PowerMockito.mockStatic(RestHighLevelClient.class);

        final SearchResponse searchResponseFromJson = PowerMockito.mock(SearchResponse.class);
        final RestHighLevelClient restHighLevelClient = PowerMockito.mock(RestHighLevelClient.class);
        final SearchRequest searchRequest = new SearchRequest(index);

        PowerMockito.when(restHighLevelClient.search(searchRequestArgumentCaptor.capture(), eq(RequestOptions.DEFAULT)))
                .thenReturn(searchResponseFromJson);

        //When
        SearchResponse searchResponse = elasticSearchContext.search(searchRequest);

        //Then
        assertThat(searchResponse).isEqualTo(searchResponseFromJson);
        verify(restHighLevelClient).search(searchRequestArgumentCaptor.capture(), eq(RequestOptions.DEFAULT));
    }
mshean

mshean commented on Dec 4, 2021

@mshean
mshean
on Dec 4, 2021

This is the most unfriendly developer library I've ever experienced in my career. WHY IS EVERYTHING FINAL????

jloisel

jloisel commented on Dec 7, 2021

@jloisel
jloisel
on Dec 7, 2021

That okay to have most classes and fields as final. Immutability is a good thing. The lack of interfaces to hive actual implementation is the real problem.

I refuse to use libraries such as PowerMock as it usually leads to a horrible, unmaintainable mess. It's a confession of failure when you have to resort to PowerMock.

btljames

btljames commented on Jul 3, 2023

@btljames
btljames
on Jul 3, 2023

This is even more of an issue in JDK 17 as PowerMock is no longer available. Can we please have any sort of traction on getting this resolved?

dakrone

dakrone commented on Mar 8, 2024

@dakrone
dakrone
on Mar 8, 2024
Member

Closing this as we've removed the high level rest client in favor of the Java client.

dakrone
closed this as completedon Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Team:Data ManagementMeta label for data/management team

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @dakrone@dadoonet@rjernst@hub-cap@lxghtless

        Issue actions
          RestHighLevelClient impossible mocking · Issue #40534 · elastic/elasticsearch