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 @@ -125,6 +125,9 @@ class AbstractTransportVersionFuncTest extends AbstractGradleFuncTest {
tasks.named('generateTransportVersion') {
currentUpperBoundName = '9.2'
}
tasks.named('validateTransportVersionResources') {
currentUpperBoundName = '9.2'
}
"""
referableAndReferencedTransportVersion("existing_91", "8012000")
referableAndReferencedTransportVersion("older_92", "8122000")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,4 +277,21 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes
"[myserver/src/main/resources/transport/definitions/referable/existing_92.csv]"
)
}

def "primary id checks skipped on release branch"() {
given:
file("myserver/build.gradle") << """
tasks.named('validateTransportVersionResources') {
currentUpperBoundName = '9.1'
}
"""
referableAndReferencedTransportVersion("some_tv", "8125000")
transportVersionUpperBound("9.2", "some_tv", "8125000")

when:
def result = gradleRunner("validateTransportVersionResources").build()

then:
result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public void apply(Project project) {
t.getReferencesFiles().setFrom(tvReferencesConfig);
t.getShouldValidateDensity().convention(true);
t.getShouldValidatePrimaryIdNotPatch().convention(true);
t.getCurrentUpperBoundName().convention(currentVersion.getMajor() + "." + currentVersion.getMinor());
});
project.getTasks().named(PrecommitPlugin.PRECOMMIT_TASK_NAME).configure(t -> t.dependsOn(validateTask));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@ public Path getResourcesDir() {
@Input
public abstract Property<Boolean> getShouldValidatePrimaryIdNotPatch();

/**
* The name of the upper bounds file which will be used at runtime on the current branch. Normally
* this equates to VersionProperties.getElasticsearchVersion().
*/
@Input
public abstract Property<String> getCurrentUpperBoundName();

private record IdAndDefinition(TransportVersionId id, TransportVersionDefinition definition) {}

private static final Pattern NAME_FORMAT = Pattern.compile("[a-z0-9_]+");
Expand All @@ -76,6 +83,7 @@ public void validateTransportVersions() throws IOException {
Map<String, TransportVersionDefinition> allDefinitions = collectAllDefinitions(referableDefinitions, unreferableDefinitions);
Map<Integer, List<IdAndDefinition>> idsByBase = collectIdsByBase(allDefinitions.values());
Map<String, TransportVersionUpperBound> upperBounds = resources.getUpperBounds();
boolean onReleaseBranch = checkIfDefinitelyOnReleaseBranch(upperBounds);

for (var definition : referableDefinitions.values()) {
validateNamedDefinition(definition, referencedNames);
Expand All @@ -93,7 +101,9 @@ public void validateTransportVersions() throws IOException {
validateUpperBound(upperBound, allDefinitions, idsByBase);
}

validatePrimaryIds(resources, upperBounds, allDefinitions);
if (onReleaseBranch == false) {
validatePrimaryIds(resources, upperBounds, allDefinitions);
}
}

private Map<String, TransportVersionDefinition> collectAllDefinitions(
Expand Down Expand Up @@ -318,6 +328,22 @@ private void validatePrimaryIds(
);
}

private boolean checkIfDefinitelyOnReleaseBranch(Map<String, TransportVersionUpperBound> upperBounds) {
// only want to look at definitions <= the current upper bound.
// TODO: we should filter all of the upper bounds/definitions that are validated by this, not just in this method
String currentUpperBoundName = getCurrentUpperBoundName().get();
TransportVersionUpperBound currentUpperBound = upperBounds.get(currentUpperBoundName);

// non-main branches
for (TransportVersionUpperBound upperBound : upperBounds.values()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: FWIW, I like using streams here for stuff like this. For example:

return upperBounds.values().stream()
    .anyMatch(u -> u.definitionId().complete() > currentUpperBound.definitionId().complete())

if (upperBound.definitionId().complete() > currentUpperBound.definitionId().complete()) {
return true;
}
}

return false;
}

private void throwDefinitionFailure(TransportVersionDefinition definition, String message) {
Path relativePath = getResources().get().getDefinitionPath(definition);
throw new VerificationException("Transport version definition file [" + relativePath + "] " + message);
Expand Down