Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
@@ -1,7 +1,7 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionSha256Sum=443c9c8ee2ac1ee0e11881a40f2376d79c66386264a44b24a9f8ca67e633375f
distributionUrl=https\://services.gradle.org/distributions/gradle-8.14.2-all.zip
distributionSha256Sum=f759b8dd5204e2e3fa4ca3e73f452f087153cf81bac9561eeb854229cc2c5365
distributionUrl=https\://services.gradle.org/distributions/gradle-9.0.0-all.zip
networkTimeout=10000
validateDistributionUrl=true
zipStoreBase=GRADLE_USER_HOME
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ abstract class AbstractRestResourcesFuncTest extends AbstractGradleFuncTest {
}
"""

subProject(":distribution:archives:integ-test-zip") << "configurations.create('extracted')\n"
subProject(":distribution:archives:integ-test-zip") << "configurations.create('default')\n"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

manual creating default configuration is deprecated now

subProject(":distribution:archives:integ-test-zip") << """
apply plugin: 'base'
configurations.create('extracted')
"""
}

void setupRestResources(List<String> apis, List<String> tests = [], List<String> xpackTests = []) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,8 @@ class InternalDistributionArchiveSetupPluginFuncTest extends AbstractGradleFuncT
def "registered distribution provides archives and directory variant"() {
given:
file('someFile.txt') << "some content"

settingsFile << """
include ':consumer'
include ':producer-tar'
"""
subProject("consumer")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

gradle throws error if the project folder does not exist yet. therefore tweaks to our tests in that area.

subProject("producer-tar")

buildFile << """
import org.gradle.api.artifacts.type.ArtifactTypeDefinition;
Expand Down Expand Up @@ -154,9 +151,7 @@ class InternalDistributionArchiveSetupPluginFuncTest extends AbstractGradleFuncT
def "builds extracted distribution via extractedAssemble"() {
given:
file('someFile.txt') << "some content"
settingsFile << """
include ':producer-tar'
"""
subProject("producer-tar")

buildFile << """
import org.gradle.api.artifacts.type.ArtifactTypeDefinition;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,5 +117,4 @@ class InternalDistributionBwcSetupPluginFuncTest extends AbstractGitAwareGradleF
result.output.contains("nested folder /distribution/bwc/minor/build/bwc/checkout-8.x/" +
"distribution/archives/darwin-tar/build/install/elasticsearch-8.4.0-SNAPSHOT")
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@

package org.elasticsearch.gradle.internal

import spock.lang.Ignore

import spock.lang.Unroll

import org.apache.commons.compress.archivers.tar.TarArchiveEntry
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream
import org.apache.commons.compress.compressors.bzip2.BZip2CompressorInputStream
import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream
import org.elasticsearch.gradle.fixtures.AbstractGradleFuncTest
import org.gradle.api.GradleException
import spock.lang.Unroll

import java.nio.file.Files
import java.nio.file.Path
Expand Down Expand Up @@ -66,8 +66,12 @@ tasks.register("buildBZip2Tar", SymbolicLinkPreservingTar) { SymbolicLinkPreserv
from fileTree("archiveRoot")

into('config') {
dirMode 0750
Copy link
Contributor Author

Choose a reason for hiding this comment

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

that APi has been removed

fileMode 0660
dirPermissions {
unix(0750)
}
filePermissions {
unix(0660)
}
from "real-folder2"
}
}
Expand Down Expand Up @@ -118,8 +122,10 @@ tasks.register("buildTar", SymbolicLinkPreservingTar) { SymbolicLinkPreservingTa
preserverTimestamp << [true, false]
}

private boolean assertTar(final File archive, final Function<? super FileInputStream, ? extends InputStream> wrapper, boolean preserveFileTimestamps)
throws IOException {
private boolean assertTar(final File archive,
final Function<? super FileInputStream, ? extends InputStream> wrapper,
boolean preserveFileTimestamps)
throws IOException {
try (TarArchiveInputStream tar = new TarArchiveInputStream(wrapper.apply(new FileInputStream(archive)))) {
TarArchiveEntry entry = tar.getNextTarEntry();
boolean realFolderEntry = false;
Expand All @@ -132,7 +138,7 @@ tasks.register("buildTar", SymbolicLinkPreservingTar) { SymbolicLinkPreservingTa
if (entry.getName().equals("real-folder/")) {
assert entry.isDirectory()
realFolderEntry = true
} else if (entry.getName().equals("real-folder/file")) {
} else if (entry.getName().equals("real-folder/file")) {
assert entry.isFile()
fileEntry = true
} else if (entry.getName().equals("real-folder/link-to-file")) {
Expand All @@ -145,7 +151,7 @@ tasks.register("buildTar", SymbolicLinkPreservingTar) { SymbolicLinkPreservingTa
} else if (entry.getName().equals("config/sub/")) {
assert entry.isDirectory()
assert entry.getMode() == 16872
}else if (entry.getName().equals("link-in-folder/")) {
} else if (entry.getName().equals("link-in-folder/")) {
assert entry.isDirectory()
linkInFolderEntry = true
} else if (entry.getName().equals("link-in-folder/link-to-file")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,18 @@

package org.elasticsearch.gradle.internal

import groovy.ant.AntBuilder
Copy link
Contributor Author

Choose a reason for hiding this comment

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

packages for some groovy util classes have moved


import org.apache.tools.ant.BuildListener
import org.apache.tools.ant.BuildLogger
import org.apache.tools.ant.DefaultLogger
import org.apache.tools.ant.Project
import org.gradle.api.DefaultTask
import org.gradle.api.GradleException
import org.gradle.api.file.FileSystemOperations
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.TaskAction

import javax.inject.Inject
import java.nio.charset.Charset
import javax.inject.Inject

/**
* A task which will run ant commands.
Expand Down Expand Up @@ -83,7 +83,8 @@ public abstract class AntTask extends DefaultTask {
return new DefaultLogger(
errorPrintStream: stream,
outputPrintStream: stream,
messageOutputLevel: outputLevel)
messageOutputLevel: outputLevel
)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

package org.elasticsearch.gradle.internal.test

import groovy.ant.AntBuilder

import org.elasticsearch.gradle.OS

import org.elasticsearch.gradle.internal.AntFixtureStop
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
*
* This task is necessary because the built-in task {@link org.gradle.api.tasks.bundling.Tar} does not preserve symbolic links.
*/
public class SymbolicLinkPreservingTar extends Tar {
public abstract class SymbolicLinkPreservingTar extends Tar {

@Override
protected CopyAction createCopyAction() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
package org.elasticsearch.gradle.internal.idea;

import groovy.util.Node;
import groovy.util.XmlParser;
import groovy.xml.XmlNodePrinter;
import groovy.xml.XmlParser;

import org.gradle.api.Action;
import org.xml.sax.SAXException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.elasticsearch.gradle.VersionProperties;
import org.elasticsearch.gradle.internal.test.rest.transform.RestTestTransform;
import org.elasticsearch.gradle.internal.test.rest.transform.RestTestTransformer;
import org.elasticsearch.gradle.internal.test.rest.transform.SerializableJsonNode;
import org.elasticsearch.gradle.internal.test.rest.transform.close_to.ReplaceValueInCloseTo;
import org.elasticsearch.gradle.internal.test.rest.transform.do_.ReplaceKeyInDo;
import org.elasticsearch.gradle.internal.test.rest.transform.headers.InjectHeaders;
Expand Down Expand Up @@ -169,7 +170,7 @@ public void skipTestsByFilePattern(String filePattern, String reason) {
* @param value the value used in the replacement. For example "bar"
*/
public void replaceValueInMatch(String subKey, Object value) {
getTransformations().add(new ReplaceValueInMatch(subKey, MAPPER.convertValue(value, JsonNode.class)));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

JsonNode cannot be serialized anymore due to restrictions to the serializer. (details in commit messages). therefore we wrap this in a serializable class

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it simply because JsonNode doesn't implement java.io.Serializable?
Could you use BaseJsonNode? As far as I can see this is the only subclass of the JsonNode and implement Serializable. This way we could potentially avoid having SerializableJsonNode class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No its not that. its the fact that NodeSerialisation relies on ObjectInputStream#.readFully which is unsupported by the gradle serialization. Usually serializing any more complex class often results in a hassle in Gradle therefore should be avoided as it causes this kind of unexpected behaviour

Copy link
Contributor

Choose a reason for hiding this comment

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

I misinterpreted the change. I thought SerializableJsonNode is just a wrapper on JsonNode, but it actually stores the original value and only change it to JsonNode when toJsonNode() is called.
Is there any risk related to the value not being Serializable? Would it make sense to changed if from Object to Serializable?

getTransformations().add(new ReplaceValueInMatch(subKey, SerializableJsonNode.of(value, JsonNode.class)));
}

/**
Expand All @@ -180,7 +181,7 @@ public void replaceValueInMatch(String subKey, Object value) {
* @param testName the testName to apply replacement
*/
public void replaceValueInMatch(String subKey, Object value, String testName) {
getTransformations().add(new ReplaceValueInMatch(subKey, MAPPER.convertValue(value, JsonNode.class), testName));
getTransformations().add(new ReplaceValueInMatch(subKey, SerializableJsonNode.of(value, JsonNode.class), testName));
}

/**
Expand Down Expand Up @@ -225,7 +226,7 @@ public void replaceKeyInLength(String oldKeyName, String newKeyName) {
* @param value the value used in the replacement. For example 99
*/
public void replaceValueInLength(String subKey, int value) {
getTransformations().add(new ReplaceValueInLength(subKey, MAPPER.convertValue(value, NumericNode.class)));
getTransformations().add(new ReplaceValueInLength(subKey, SerializableJsonNode.of(value, NumericNode.class)));
}

/**
Expand All @@ -237,7 +238,7 @@ public void replaceValueInLength(String subKey, int value) {
* @param testName the testName to apply replacement
*/
public void replaceValueInLength(String subKey, int value, String testName) {
getTransformations().add(new ReplaceValueInLength(subKey, MAPPER.convertValue(value, NumericNode.class), testName));
getTransformations().add(new ReplaceValueInLength(subKey, SerializableJsonNode.of(value, NumericNode.class), testName));
}

/**
Expand All @@ -260,7 +261,7 @@ public void replaceKeyInMatch(String oldKeyName, String newKeyName) {
* @param testName the testName to apply replacement
*/
public void replaceValueInCloseTo(String subKey, double newValue, String testName) {
getTransformations().add(new ReplaceValueInCloseTo(subKey, MAPPER.convertValue(newValue, NumericNode.class), testName));
getTransformations().add(new ReplaceValueInCloseTo(subKey, SerializableJsonNode.of(newValue, NumericNode.class), testName));
}

/**
Expand All @@ -271,7 +272,7 @@ public void replaceValueInCloseTo(String subKey, double newValue, String testNam
* @param newValue the value used in the replacement. For example 9.5
*/
public void replaceValueInCloseTo(String subKey, double newValue) {
getTransformations().add(new ReplaceValueInCloseTo(subKey, MAPPER.convertValue(newValue, NumericNode.class)));
getTransformations().add(new ReplaceValueInCloseTo(subKey, SerializableJsonNode.of(newValue, NumericNode.class)));
}

/**
Expand All @@ -282,7 +283,7 @@ public void replaceValueInCloseTo(String subKey, double newValue) {
* @param newValue the value used in the replacement
*/
public void replaceIsTrue(String oldValue, Object newValue) {
getTransformations().add(new ReplaceIsTrue(oldValue, MAPPER.convertValue(newValue, TextNode.class)));
getTransformations().add(new ReplaceIsTrue(oldValue, SerializableJsonNode.of(newValue, TextNode.class)));
}

/**
Expand All @@ -294,7 +295,7 @@ public void replaceIsTrue(String oldValue, Object newValue) {
* @param testName the testName to apply replacement
*/
public void replaceIsTrue(String oldValue, Object newValue, String testName) {
getTransformations().add(new ReplaceIsTrue(oldValue, MAPPER.convertValue(newValue, TextNode.class), testName));
getTransformations().add(new ReplaceIsTrue(oldValue, SerializableJsonNode.of(newValue, TextNode.class), testName));
}

/**
Expand All @@ -305,7 +306,7 @@ public void replaceIsTrue(String oldValue, Object newValue, String testName) {
* @param newValue the value used in the replacement
*/
public void replaceIsFalse(String oldValue, Object newValue) {
getTransformations().add(new ReplaceIsFalse(oldValue, MAPPER.convertValue(newValue, TextNode.class)));
getTransformations().add(new ReplaceIsFalse(oldValue, SerializableJsonNode.of(newValue, TextNode.class)));
}

/**
Expand All @@ -317,7 +318,7 @@ public void replaceIsFalse(String oldValue, Object newValue) {
* @param testName the testName to apply replacement
*/
public void replaceIsFalse(String oldValue, Object newValue, String testName) {
getTransformations().add(new ReplaceIsFalse(oldValue, MAPPER.convertValue(newValue, TextNode.class), testName));
getTransformations().add(new ReplaceIsFalse(oldValue, SerializableJsonNode.of(newValue, TextNode.class), testName));
}

/**
Expand All @@ -329,7 +330,7 @@ public void replaceIsFalse(String oldValue, Object newValue, String testName) {
* @param newValue the value used in the replacement
*/
public void replaceValueTextByKeyValue(String key, String oldValue, Object newValue) {
getTransformations().add(new ReplaceTextual(key, oldValue, MAPPER.convertValue(newValue, TextNode.class)));
getTransformations().add(new ReplaceTextual(key, oldValue, SerializableJsonNode.of(newValue, TextNode.class)));
}

/**
Expand All @@ -342,7 +343,7 @@ public void replaceValueTextByKeyValue(String key, String oldValue, Object newVa
* @param testName the testName to apply replacement
*/
public void replaceValueTextByKeyValue(String key, String oldValue, Object newValue, String testName) {
getTransformations().add(new ReplaceTextual(key, oldValue, MAPPER.convertValue(newValue, TextNode.class), testName));
getTransformations().add(new ReplaceTextual(key, oldValue, SerializableJsonNode.of(newValue, TextNode.class), testName));
}

/**
Expand Down Expand Up @@ -376,7 +377,7 @@ public void removeMatch(String subKey, String testName) {
* @param testName the testName to apply addition
*/
public void addMatch(String subKey, Object value, String testName) {
getTransformations().add(new AddMatch(subKey, MAPPER.convertValue(value, JsonNode.class), testName));
getTransformations().add(new AddMatch(subKey, SerializableJsonNode.of(value, JsonNode.class), testName));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
package org.elasticsearch.gradle.internal.test.rest.transform;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ObjectNode;

import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.Optional;
Expand All @@ -25,18 +26,23 @@
public abstract class ReplaceByKey implements RestTestTransformByParentObject {
private final String requiredChildKey;
private final String newChildKey;
private final JsonNode replacementNode;
private final SerializableJsonNode replacementNode;
private final String testName;

public ReplaceByKey(String requiredChildKey, JsonNode replacementNode) {
public ReplaceByKey(String requiredChildKey, SerializableJsonNode<JsonNode> replacementNode) {
this(requiredChildKey, replacementNode, null);
}

public ReplaceByKey(String requiredChildKey, JsonNode replacementNode, String testName) {
public ReplaceByKey(String requiredChildKey, SerializableJsonNode<JsonNode> replacementNode, String testName) {
this(requiredChildKey, requiredChildKey, replacementNode, testName);
}

public ReplaceByKey(String requiredChildKey, String newChildKey, JsonNode replacementNode, String testName) {
public ReplaceByKey(
String requiredChildKey,
String newChildKey,
SerializableJsonNode<? extends JsonNode> replacementNode,
String testName
) {
this.requiredChildKey = requiredChildKey;
this.newChildKey = newChildKey;
this.replacementNode = replacementNode;
Expand All @@ -60,7 +66,7 @@ public boolean shouldApply(RestTestContext testContext) {

@Input
@Optional
public JsonNode getReplacementNode() {
public SerializableJsonNode<? extends JsonNode> getReplacementNode() {
return replacementNode;
}

Expand All @@ -69,4 +75,10 @@ public JsonNode getReplacementNode() {
public String getTestName() {
return testName;
}

protected void updateReplacement(ObjectNode matchNode) {
matchNode.remove(requiredChildKey());
matchNode.set(getNewChildKey(), replacementNode.toJsonNode());
}

}
Loading