Skip to content

Commit

Permalink
Adding Version Checks for Session Properties
Browse files Browse the repository at this point in the history
Summary: We need to compare Presto Versions for Session property version checks to allow only the valid session properties based on the current running Presto version. To do that, I added the Presto Version class in `presto-common` repo so that it can be used by other classes if needed.

Test Plan: - Updated vll1_verifier1
	   - Canaried configerator configs and tested the changes.

Reviewers:

Subscribers:

Tasks: T115110477

Tags:
  • Loading branch information
vaishnavibatni authored and tdcmeehan committed Jun 9, 2022
1 parent ef7d408 commit b7d3d4c
Show file tree
Hide file tree
Showing 7 changed files with 334 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.facebook.presto.common;

import java.util.Objects;
import java.util.Optional;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static java.util.Objects.requireNonNull;

public class PrestoVersion
implements Comparable<PrestoVersion>
{
private static final Pattern VERSION_REGEX = Pattern.compile("^(?<major>0|[1-9]\\d*)\\.(?<minor>0|[1-9]\\d*)(\\.(?<patch>0|[1-9]\\d*))?(?:-(?<prerelease>(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+(?<buildmetadata>[0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$");

private int majorVersion;
private int minorVersion;
private int patchVersion;
private String prereleaseVersion = "";
private String version;

public PrestoVersion(String version)
{
this.version = requireNonNull(version, "version string is null");
Matcher matcher = VERSION_REGEX.matcher(version);
if (matcher.find()) {
this.majorVersion = Integer.parseInt(Optional.ofNullable(matcher.group("major")).orElse("0"));
this.minorVersion = Integer.parseInt(Optional.ofNullable(matcher.group("minor")).orElse("0"));
this.patchVersion = Integer.parseInt(Optional.ofNullable(matcher.group("patch")).orElse("0"));
this.prereleaseVersion = Optional.ofNullable(matcher.group("prerelease")).orElse("");
}
}

public String getVersion()
{
return version;
}

@Override
public int hashCode()
{
return Objects.hash(this.majorVersion, this.minorVersion, this.patchVersion, this.prereleaseVersion);
}

@Override
public boolean equals(Object version)
{
if (this == version) {
return true;
}
if (version == null || !(version instanceof PrestoVersion)) {
return false;
}
PrestoVersion prestoVersion = (PrestoVersion) version;
return (Objects.equals(this.majorVersion, prestoVersion.majorVersion)
&& Objects.equals(this.minorVersion, prestoVersion.minorVersion)
&& Objects.equals(this.patchVersion, prestoVersion.patchVersion)
&& Objects.equals(this.prereleaseVersion, prestoVersion.prereleaseVersion));
}

@Override
public int compareTo(PrestoVersion prestoVersion)
{
if (this.majorVersion != prestoVersion.majorVersion) {
return Integer.compare(this.majorVersion, prestoVersion.majorVersion);
}
if (this.minorVersion != prestoVersion.minorVersion) {
return Integer.compare(this.minorVersion, prestoVersion.minorVersion);
}
if (this.patchVersion != prestoVersion.patchVersion) {
return Integer.compare(this.patchVersion, prestoVersion.patchVersion);
}
if (!(this.prereleaseVersion.equals(prestoVersion.prereleaseVersion))) {
if (prestoVersion.prereleaseVersion.isEmpty()) {
return -1;
}
if (this.prereleaseVersion.isEmpty()) {
return 1;
}
return this.prereleaseVersion.compareTo(prestoVersion.prereleaseVersion);
}
return 0;
}

public boolean equalTo(PrestoVersion prestoVersion)
{
return this.compareTo(prestoVersion) == 0;
}

public boolean greaterThan(PrestoVersion prestoVersion)
{
return this.compareTo(prestoVersion) > 0;
}

public boolean lessThan(PrestoVersion prestoVersion)
{
return this.compareTo(prestoVersion) < 0;
}

public boolean greaterThanOrEqualTo(PrestoVersion prestoVersion)
{
return this.compareTo(prestoVersion) >= 0;
}

public boolean lessThanOrEqualTo(PrestoVersion prestoVersion)
{
return this.compareTo(prestoVersion) <= 0;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.facebook.presto.common;

import org.testng.annotations.Test;

import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertTrue;

public class TestPrestoVersion
{
private static final PrestoVersion VERSION_274_20220517_171619_22 = new PrestoVersion("0.274-20220517.171619-22");
private static final PrestoVersion VERSION_274_20220518_025123_29 = new PrestoVersion("0.274-20220518.025123-29");
private static final PrestoVersion VERSION_274_SNAPSHOT_5d0ba93 = new PrestoVersion("0.274-SNAPSHOT-5d0ba93");
private static final PrestoVersion VERSION_273 = new PrestoVersion("0.273");
private static final PrestoVersion VERSION_274 = new PrestoVersion("0.274");
private static final PrestoVersion VERSION_274_1 = new PrestoVersion("0.274.1");
private static final PrestoVersion VERSION_274_2 = new PrestoVersion("0.274.2");

@Test
public void testPrestoEquals()
{
assertTrue(VERSION_274_20220517_171619_22.equals(VERSION_274_20220517_171619_22));
assertTrue(VERSION_274_1.equals(new PrestoVersion("0.274.1")));
assertFalse(VERSION_274_1.equals(VERSION_274));
assertFalse(VERSION_274_20220517_171619_22.equals(null));
}

@Test
public void testLessThan()
{
//Comparing major versions
assertFalse(VERSION_274_1.lessThan(VERSION_273));

// Comparing Snapshot and non-Snapshot versions
assertTrue(VERSION_274_20220517_171619_22.lessThan(VERSION_274));
assertTrue(VERSION_274_SNAPSHOT_5d0ba93.lessThan(VERSION_274));
assertTrue(VERSION_274_SNAPSHOT_5d0ba93.lessThan(VERSION_274_1));

//Comparing snapshot versions
assertFalse(VERSION_274_20220518_025123_29.lessThan(VERSION_274_20220517_171619_22));

// Comparing equal versions
assertFalse(VERSION_274_SNAPSHOT_5d0ba93.lessThan(VERSION_274_SNAPSHOT_5d0ba93));

// Comparing major and minor versions
assertTrue(VERSION_274.lessThan(VERSION_274_1));
assertFalse(VERSION_274_1.lessThan(VERSION_274));

//comparing minor versions
assertFalse(VERSION_274_2.lessThan(VERSION_274_1));
}

@Test
public void testLessThanOrEqualTo()
{
assertTrue(VERSION_274_20220517_171619_22.lessThanOrEqualTo(VERSION_274));
assertTrue(VERSION_274_SNAPSHOT_5d0ba93.lessThanOrEqualTo(VERSION_274));
assertTrue(VERSION_274.lessThanOrEqualTo(VERSION_274_1));
assertTrue(VERSION_274_SNAPSHOT_5d0ba93.lessThanOrEqualTo(VERSION_274_1));

//The following values should be equal
assertTrue(VERSION_274_SNAPSHOT_5d0ba93.lessThanOrEqualTo(VERSION_274_SNAPSHOT_5d0ba93));
assertTrue(VERSION_274.lessThanOrEqualTo(VERSION_274));
}

@Test
public void testGreaterThanOrEqualTo()
{
assertTrue(VERSION_274.greaterThanOrEqualTo(VERSION_274_20220517_171619_22));
assertTrue(VERSION_274.greaterThanOrEqualTo(VERSION_274_SNAPSHOT_5d0ba93));
assertTrue(VERSION_274_1.greaterThanOrEqualTo(VERSION_274));
assertTrue(VERSION_274_1.greaterThanOrEqualTo(VERSION_274_SNAPSHOT_5d0ba93));

//The following values should be equal
assertTrue(VERSION_274_SNAPSHOT_5d0ba93.greaterThanOrEqualTo(VERSION_274_SNAPSHOT_5d0ba93));
assertTrue(VERSION_274.greaterThanOrEqualTo(VERSION_274));
}

@Test
public void testGreaterThan()
{
//Comparing major versions
assertTrue(VERSION_274_1.greaterThan(VERSION_273));

// Comparing Snapshot and non-Snapshot versions
assertTrue(VERSION_274.greaterThan(VERSION_274_20220517_171619_22));
assertFalse(VERSION_274_20220517_171619_22.greaterThan(VERSION_274));
assertFalse(VERSION_274_SNAPSHOT_5d0ba93.greaterThan(VERSION_274));
assertFalse(VERSION_274_SNAPSHOT_5d0ba93.greaterThan(VERSION_274_1));

//Comparing snapshot versions
assertFalse(VERSION_274_20220517_171619_22.greaterThan(VERSION_274_20220518_025123_29));
assertTrue(VERSION_274_20220518_025123_29.greaterThan(VERSION_274_20220517_171619_22));

// Comparing equal versions
assertFalse(VERSION_274_SNAPSHOT_5d0ba93.greaterThan(VERSION_274_SNAPSHOT_5d0ba93));

// Comparing major and minor versions
assertFalse(VERSION_274.greaterThan(VERSION_274_1));
assertTrue(VERSION_274_1.greaterThan(VERSION_274));
}

@Test
public void testVersionValidity()
{
PrestoVersion minVersion = new PrestoVersion("0.282");
PrestoVersion maxVersion = new PrestoVersion("0.292");
assertFalse(minVersion.lessThanOrEqualTo(VERSION_274_20220517_171619_22) && maxVersion.greaterThanOrEqualTo(VERSION_274_20220517_171619_22));
}
}
27 changes: 27 additions & 0 deletions presto-session-property-managers/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,41 @@
<scope>provided</scope>
</dependency>

<dependency>
<groupId>com.facebook.presto</groupId>
<artifactId>presto-client</artifactId>
</dependency>

<dependency>
<groupId>com.facebook.drift</groupId>
<artifactId>drift-api</artifactId>
<scope>provided</scope>
</dependency>

<dependency>
<groupId>io.airlift</groupId>
<artifactId>units</artifactId>
<scope>provided</scope>
</dependency>

<dependency>
<groupId>io.airlift</groupId>
<artifactId>slice</artifactId>
<scope>provided</scope>
</dependency>

<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-annotations</artifactId>
<scope>provided</scope>
</dependency>

<dependency>
<groupId>org.openjdk.jol</groupId>
<artifactId>jol-core</artifactId>
<scope>provided</scope>
</dependency>

<!-- for testing -->
<dependency>
<groupId>com.facebook.presto</groupId>
Expand All @@ -105,5 +128,9 @@
<artifactId>testing</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.facebook.presto</groupId>
<artifactId>presto-main</artifactId>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import com.facebook.airlift.json.JsonCodec;
import com.facebook.airlift.json.JsonCodecFactory;
import com.facebook.airlift.json.JsonObjectMapperProvider;
import com.facebook.presto.client.NodeVersion;
import com.facebook.presto.common.PrestoVersion;
import com.facebook.presto.spi.session.SessionConfigurationContext;
import com.facebook.presto.spi.session.SessionPropertyConfigurationManager;
import com.fasterxml.jackson.databind.JsonMappingException;
Expand Down Expand Up @@ -46,11 +48,13 @@ public class FileSessionPropertyManager
.listJsonCodec(SessionMatchSpec.class);

private final List<SessionMatchSpec> sessionMatchSpecs;
private final PrestoVersion prestoVersion;

@Inject
public FileSessionPropertyManager(FileSessionPropertyManagerConfig config)
public FileSessionPropertyManager(FileSessionPropertyManagerConfig config, NodeVersion nodeVersion)
{
requireNonNull(config, "config is null");
this.prestoVersion = new PrestoVersion(nodeVersion.getVersion());

Path configurationFile = config.getConfigFile().toPath();
try {
Expand Down Expand Up @@ -87,7 +91,7 @@ public SystemSessionPropertyConfiguration getSystemSessionProperties(SessionConf
Map<String, String> defaultProperties = new HashMap<>();
Set<String> overridePropertyNames = new HashSet<>();
for (SessionMatchSpec sessionMatchSpec : sessionMatchSpecs) {
Map<String, String> newProperties = sessionMatchSpec.match(context);
Map<String, String> newProperties = sessionMatchSpec.match(context, this.prestoVersion);
defaultProperties.putAll(newProperties);
if (sessionMatchSpec.getOverrideSessionProperties().orElse(false)) {
overridePropertyNames.addAll(newProperties.keySet());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,25 @@
*/
package com.facebook.presto.session;

import com.facebook.airlift.configuration.AbstractConfigurationAwareModule;
import com.facebook.presto.client.NodeVersion;
import com.facebook.presto.server.ServerConfig;
import com.google.inject.Binder;
import com.google.inject.Module;
import com.google.inject.Scopes;

import static com.facebook.airlift.configuration.ConfigBinder.configBinder;

public class FileSessionPropertyManagerModule
implements Module
extends AbstractConfigurationAwareModule
{
@Override
public void configure(Binder binder)
public void setup(Binder binder)
{
configBinder(binder).bindConfig(FileSessionPropertyManagerConfig.class);
binder.bind(FileSessionPropertyManager.class).in(Scopes.SINGLETON);

ServerConfig serverConfig = buildConfigObject(ServerConfig.class);
NodeVersion nodeVersion = new NodeVersion(serverConfig.getPrestoVersion());
binder.bind(NodeVersion.class).toInstance(nodeVersion);
}
}
Loading

0 comments on commit b7d3d4c

Please sign in to comment.