Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,8 @@ public void testMatchAllQuery() throws IOException {
final SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder().query(QueryBuilders.matchAllQuery())
.size(numberOfDocuments);

final MatchResult matchResult = Matcher.mappings(getContenderMappings(), getBaselineMappings())
final MatchResult matchResult = Matcher.matchSource()
.mappings(getContenderMappings(), getBaselineMappings())
.settings(getContenderSettings(), getBaselineSettings())
.expected(getQueryHits(queryBaseline(searchSourceBuilder)))
.ignoringSort(true)
Expand All @@ -208,7 +209,8 @@ public void testTermsQuery() throws IOException {
final SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder().query(QueryBuilders.termQuery("method", "put"))
.size(numberOfDocuments);

final MatchResult matchResult = Matcher.mappings(getContenderMappings(), getBaselineMappings())
final MatchResult matchResult = Matcher.matchSource()
.mappings(getContenderMappings(), getBaselineMappings())
.settings(getContenderSettings(), getBaselineSettings())
.expected(getQueryHits(queryBaseline(searchSourceBuilder)))
.ignoringSort(true)
Expand Down Expand Up @@ -324,5 +326,4 @@ private void assertDocumentIndexing(List<XContentBuilder> documents) throws IOEx
var contenderResponseBody = entityAsMap(tuple.v2());
assertThat("errors in contender bulk response:\n " + contenderResponseBody, contenderResponseBody.get("errors"), equalTo(false));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.io.IOException;
import java.time.Instant;
import java.util.List;
import java.util.function.Function;

/**
* Challenge test (see {@link StandardVersusLogsIndexModeChallengeRestIT}) that uses randomly generated
Expand All @@ -42,26 +41,7 @@ public StandardVersusLogsIndexModeRandomDataChallengeRestIT() {
DataGeneratorSpecification.builder()
// Nested fields don't work with subobjects: false.
.withNestedFieldsLimit(0)
// TODO increase depth of objects
// Currently matching fails because in synthetic source all fields are flat (given that we have subobjects: false)
// but stored source is identical to original document which has nested structure.
.withMaxObjectDepth(0)
.withDataSourceHandlers(List.of(new DataSourceHandler() {
// TODO enable null values
// Matcher does not handle nulls currently
@Override
public DataSourceResponse.NullWrapper handle(DataSourceRequest.NullWrapper request) {
return new DataSourceResponse.NullWrapper(Function.identity());
}

// TODO enable arrays
// List matcher currently does not apply matching logic recursively
// and equality check fails because arrays are sorted in synthetic source.
@Override
public DataSourceResponse.ArrayWrapper handle(DataSourceRequest.ArrayWrapper request) {
return new DataSourceResponse.ArrayWrapper(Function.identity());
}

// TODO enable scaled_float fields
// There a difference in synthetic source (precision loss)
// specific to this fields which matcher can't handle.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

import java.util.List;

class EqualMatcher<T> extends Matcher {
public class EqualMatcher<T> extends Matcher {
protected final XContentBuilder actualMappings;
protected final Settings.Builder actualSettings;
protected final XContentBuilder expectedMappings;
Expand All @@ -22,7 +22,7 @@ class EqualMatcher<T> extends Matcher {
protected final T expected;
protected final boolean ignoringSort;

EqualMatcher(
protected EqualMatcher(
XContentBuilder actualMappings,
Settings.Builder actualSettings,
XContentBuilder expectedMappings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@

import java.util.List;

class ListEqualMatcher extends EqualMatcher<List<?>> {
ListEqualMatcher(
public class ListEqualMatcher extends EqualMatcher<List<?>> {
public ListEqualMatcher(
final XContentBuilder actualMappings,
final Settings.Builder actualSettings,
final XContentBuilder expectedMappings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@

import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.datastreams.logsdb.qa.matchers.source.SourceMatcher;
import org.elasticsearch.xcontent.XContentBuilder;

import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

/**
Expand All @@ -25,6 +27,14 @@ public static <T> SettingsStep<T> mappings(final XContentBuilder actualMappings,
return new Builder<>(expectedMappings, actualMappings);
}

public static MappingsStep<List<Map<String, Object>>> matchSource() {
return new SourceMatcherBuilder();
}

public interface MappingsStep<T> {
SettingsStep<T> mappings(XContentBuilder actualMappings, XContentBuilder expectedMappings);
}

public interface SettingsStep<T> {
ExpectedStep<T> settings(Settings.Builder actualSettings, Settings.Builder expectedSettings);
}
Expand All @@ -40,7 +50,6 @@ public interface CompareStep<T> {
}

private static class Builder<T> implements SettingsStep<T>, CompareStep<T>, ExpectedStep<T> {

private final XContentBuilder expectedMappings;
private final XContentBuilder actualMappings;
private Settings.Builder expectedSettings;
Expand Down Expand Up @@ -84,6 +93,57 @@ public CompareStep<T> expected(T expected) {
}
}

private static class SourceMatcherBuilder
implements
MappingsStep<List<Map<String, Object>>>,
SettingsStep<List<Map<String, Object>>>,
CompareStep<List<Map<String, Object>>>,
ExpectedStep<List<Map<String, Object>>> {
private XContentBuilder expectedMappings;
private XContentBuilder actualMappings;
private Settings.Builder expectedSettings;
private Settings.Builder actualSettings;
private List<Map<String, Object>> expected;
private boolean ignoringSort;

@Override
public ExpectedStep<List<Map<String, Object>>> settings(Settings.Builder actualSettings, Settings.Builder expectedSettings) {
this.actualSettings = actualSettings;
this.expectedSettings = expectedSettings;
return this;
}

private SourceMatcherBuilder() {}

public SettingsStep<List<Map<String, Object>>> mappings(
final XContentBuilder actualMappings,
final XContentBuilder expectedMappings
) {
this.actualMappings = actualMappings;
this.expectedMappings = expectedMappings;

return this;
}

@Override
public MatchResult isEqualTo(List<Map<String, Object>> actual) {
return new SourceMatcher(actualMappings, actualSettings, expectedMappings, expectedSettings, actual, expected, ignoringSort)
.match();
}

@Override
public CompareStep<List<Map<String, Object>>> ignoringSort(boolean ignoringSort) {
this.ignoringSort = ignoringSort;
return this;
}

@Override
public CompareStep<List<Map<String, Object>>> expected(List<Map<String, Object>> expected) {
this.expected = expected;
return this;
}
}

protected static String formatErrorMessage(
final XContentBuilder actualMappings,
final Settings.Builder actualSettings,
Expand Down Expand Up @@ -112,11 +172,11 @@ protected static String prettyPrintArrays(final Object[] actualArray, final Obje
return "actual: " + prettyPrintList(Arrays.asList(actualArray)) + ", expected: " + prettyPrintList(Arrays.asList(expectedArray));
}

protected static String prettyPrintLists(final List<Object> actualList, final List<Object> expectedList) {
protected static <T> String prettyPrintLists(final List<T> actualList, final List<T> expectedList) {
return "actual: " + prettyPrintList(actualList) + ", expected: " + prettyPrintList(expectedList);
}

private static String prettyPrintList(final List<Object> list) {
private static <T> String prettyPrintList(final List<T> list) {
return "[" + list.stream().map(Object::toString).collect(Collectors.joining(", ")) + "]";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.datastreams.logsdb.qa.matchers.source;

import org.apache.lucene.sandbox.document.HalfFloatPoint;

import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;

public interface FieldSpecificMatcher {
boolean match(List<Object> actual, List<Object> expected);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this method return a MatchResult instead of a boolean?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to do that but to produce full message in case there is a mismatch you'll need to pass mapping and settings here which ends up being a mouthful.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it's not a big deal.


class HalfFloatMatcher implements FieldSpecificMatcher {
public boolean match(List<Object> actual, List<Object> expected) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Override?

var actualHalfFloatBytes = normalize(actual);
var expectedHalfFloatBytes = normalize(expected);

return actualHalfFloatBytes.equals(expectedHalfFloatBytes);
}

private static Set<Short> normalize(List<Object> values) {
if (values == null) {
return Set.of();
}

Function<Object, Float> toFloat = (o) -> o instanceof Number n ? n.floatValue() : Float.parseFloat((String) o);
return values.stream()
.filter(Objects::nonNull)
.map(toFloat)
// Based on logic in NumberFieldMapper
.map(HalfFloatPoint::halfFloatToSortableShort)
.collect(Collectors.toSet());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.datastreams.logsdb.qa.matchers.source;

import java.util.HashMap;
import java.util.Map;

class MappingTransforms {
/**
* Normalize mapping to have the same structure as normalized source and enable field mapping lookup.
* Similar to {@link SourceTransforms#normalize(Map)} but needs to get rid of intermediate nodes
* and collect results into a different data structure.
*
* @param map
* @return
*/
public static Map<String, Map<String, Object>> normalizeMapping(Map<String, Object> map) {
var flattened = new HashMap<String, Map<String, Object>>();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm somewhat puzzled here, why not just use HashMap<String, Object> to track just the leaf fields?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, you do that for source checking while you also want to compare the mappings. Need to think a bit about it..

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a map from normalized field name (a.b.c) to a map of mapping parameters (like type) since there can be multiple.


descend(null, map, flattened);

return flattened;
}

@SuppressWarnings("unchecked")
private static void descend(String pathFromRoot, Map<String, Object> currentLevel, Map<String, Map<String, Object>> flattened) {
for (var entry : currentLevel.entrySet()) {
if (entry.getKey().equals("_doc") || entry.getKey().equals("properties")) {
descend(pathFromRoot, (Map<String, Object>) entry.getValue(), flattened);
} else {
if (entry.getValue() instanceof Map<?, ?> map) {
var pathToField = pathFromRoot == null ? entry.getKey() : pathFromRoot + "." + entry.getKey();
descend(pathToField, (Map<String, Object>) map, flattened);
} else {
if (pathFromRoot == null) {
// Ignore top level mapping parameters for now
continue;
}

flattened.putIfAbsent(pathFromRoot, new HashMap<>());

@kkrik-es kkrik-es Aug 5, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: combine the two lines:

 flattened.computeIfAbsent(pathFromRoot, new HashMap<>()).put(entry.getKey(), entry.getValue());

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat, didn't expect this from java.

flattened.get(pathFromRoot).put(entry.getKey(), entry.getValue());
}
}
}
}
}
Loading