Skip to content

Commit

Permalink
Add back SetValues() as it is used internally (#579)
Browse files Browse the repository at this point in the history
* addSetValues

* fix test

* fix test

---------

Co-authored-by: Yingjian Wu <[email protected]>
  • Loading branch information
stevie9868 and Yingjian Wu authored Mar 11, 2024
1 parent 2bf2a0b commit 0845991
Show file tree
Hide file tree
Showing 5 changed files with 223 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,29 +77,34 @@ default Set<String> getValues(final Long lookupId) {
*
* @param name lookup name
* @param values multiple values
* @param includeValues whether to populate the values field in the Lookup Object
* @param includeValues whether to include values in the final Lookup Object
* @return updated lookup
*/
@Nullable
default Lookup addValues(final String name, final Set<String> values, boolean includeValues) {
return null;
}

/**
* Saves the lookup value.
*
* @param name lookup name
* @param values multiple values
* @param includeValues whether to include values in the final Lookup Object
* @param name lookup name
* @param values lookup value
* @return updated lookup
*/
@Nullable
default Lookup addValues(final String name, final Set<String> values, boolean includeValues) {
default Lookup setValues(final String name, final Set<String> values) {
return null;
}

/**
* Saves the lookup value.
*
* Save single lookup Value.
* @param name lookup name
* @param value lookup value
* @param includeValues whether to return lookup value in the Lookup Object
* @return updated lookup
* @return returns the lookup with the given name.
*/
@Nullable
default Lookup setValue(final String name, final String value) {
return null;
}
}
2 changes: 1 addition & 1 deletion metacat-functional-tests/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ configurations {


dependencies {
testImplementation project(path: ':metacat-metadata-mysql')
warproject(project(path: ':metacat-war', configuration: 'archives'))

/*******************************
Expand All @@ -64,7 +65,6 @@ dependencies {
testImplementation(project(":metacat-common-server"))
testImplementation(project(":metacat-connector-hive"))
testImplementation(project(":metacat-testdata-provider"))

functionalTestImplementation(project(":metacat-client"))
functionalTestImplementation("org.apache.hadoop:hadoop-core")
functionalTestImplementation("org.apache.hive:hive-exec:${hive_version}") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ services:
- MYSQL_PASSWORD=metacat_user_password
- MYSQL_DATABASE=metacat
ports:
- '3306'
- '3306:3306'
labels:
- "com.netflix.metacat.oss.test"
postgresql:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
/*
* Copyright 2016 Netflix, Inc.
*
* 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.netflix.metacat

import com.netflix.metacat.common.server.model.Lookup
import com.netflix.metacat.common.server.properties.DefaultConfigImpl
import com.netflix.metacat.common.server.properties.MetacatProperties
import com.netflix.metacat.metadata.mysql.MySqlLookupService
import org.springframework.jdbc.core.JdbcTemplate
import org.springframework.jdbc.datasource.DriverManagerDataSource
import spock.lang.Shared
import spock.lang.Specification

class MySqlLookupServiceSpec extends Specification{
private MySqlLookupService mySqlLookupService;
private JdbcTemplate jdbcTemplate;

@Shared
MySqlLookupService mySqlLookupService

@Shared
JdbcTemplate jdbcTemplate

def setupSpec() {
String jdbcUrl = "jdbc:mysql://localhost:3306/metacat"
String username = "metacat_user"
String password = "metacat_user_password"

DriverManagerDataSource dataSource = new DriverManagerDataSource()
dataSource.setDriverClassName("com.mysql.cj.jdbc.Driver")
dataSource.setUrl(jdbcUrl)
dataSource.setUsername(username)
dataSource.setPassword(password)

jdbcTemplate = new JdbcTemplate(dataSource)
mySqlLookupService = new MySqlLookupService(new DefaultConfigImpl(new MetacatProperties()), jdbcTemplate)
}

boolean areLookupsEqual(Lookup l1, Lookup l2) {
l1.id == l2.id &&
l1.name == l2.name &&
l1.type == l2.type &&
l1.values == l2.values &&
l1.createdBy == l2.createdBy &&
l1.lastUpdatedBy == l2.lastUpdatedBy
}

def "test setValues with getValue/getValues iterative"() {
setup:
def values = valuesList as Set<String>
def lookup = mySqlLookupService.setValues("mock", values)

expect:
lookup.values.size() == expectedSize
lookup.values == mySqlLookupService.getValues("mock")
lookup.values == mySqlLookupService.getValues(lookup.id)
lookup.values.contains(mySqlLookupService.getValue("mock"))
areLookupsEqual(lookup, mySqlLookupService.get("mock", true))

where:
valuesList | expectedSize
["1", "2", "3"] | 3
["1", "2", "3", "4"] | 4
["1", "2", "3", "3", "4"] | 4
["3", "4"] | 2
["6"] | 1
["1", "6"] | 2
}

def "test setValues for different id"(){
when:
def mock1LookUp = mySqlLookupService.setValues("mock1", ["1", "2", "3"] as Set<String>)
def mock2LookUp = mySqlLookupService.setValues("mock2", ["4", "5", "6"] as Set<String>)
then:
mock1LookUp.values == ["1", "2", "3"] as Set<String>
mock1LookUp.values == mySqlLookupService.getValues("mock1")
areLookupsEqual(mock1LookUp, mySqlLookupService.get("mock1", true))
mock2LookUp.values == ["4", "5", "6"] as Set<String>
mock2LookUp.values == mySqlLookupService.getValues("mock2")
areLookupsEqual(mock2LookUp, mySqlLookupService.get("mock2", true))
}

def "test addValues iterative"() {
setup:
def values = valuesList as Set<String>
def lookup = mySqlLookupService.addValues("mockAdd", values, includeValues)

expect:
if (includeValues) {
lookup.values == mySqlLookupService.getValues(lookup.id)
lookup.values.contains(mySqlLookupService.getValue("mockAdd"))
lookup.values == mySqlLookupService.getValues("mockAdd")
}
lookup.values.size() == expectedSize
areLookupsEqual(lookup, mySqlLookupService.get("mockAdd", includeValues))
where:
valuesList | expectedSize | includeValues
["1", "2", "3"] | 3 | true
["1", "2", "3", "4"] | 4 | true
["1", "2", "3", "3", "4"] | 4 | true
["3", "4"] | 4 | true
["6"] | 5 | true
["1", "6"] | 5 | true
["1", "2", "3"] | 0 | false
["1", "2", "3", "4"] | 0 | false
["1", "2", "3", "3", "4"] | 0 | false
["3", "4"] | 0 | false
["6"] | 0 | false
["1", "6"] | 0 | false
}

def "test addValues for different id"() {
setup:
def mock1LookUp = mySqlLookupService.addValues("addValues_mock1", ["1", "2", "3"] as Set<String>, false)
def mock2LookUp = mySqlLookupService.addValues("addValues_mock2", ["4", "5", "6"] as Set<String>, true)

expect:
mock1LookUp.values.isEmpty()
areLookupsEqual(mock1LookUp, mySqlLookupService.get("addValues_mock1", false))
mock2LookUp.values == ["4", "5", "6"] as Set<String>
mock2LookUp.values == mySqlLookupService.getValues("addValues_mock2")
areLookupsEqual(mock2LookUp, mySqlLookupService.get("addValues_mock2", true))
}

def "test setValue for different id"(){
when:
def mock1LookUp = mySqlLookupService.setValues("mock1", ["1", "2", "3"] as Set<String>)
def mock2LookUp = mySqlLookupService.setValues("mock2", ["4", "5", "6"] as Set<String>)
then:
mock1LookUp.values == ["1", "2", "3"] as Set<String>
mock1LookUp.values == mySqlLookupService.getValues("mock1")
areLookupsEqual(mock1LookUp, mySqlLookupService.get("mock1", true))
mock2LookUp.values == ["4", "5", "6"] as Set<String>
mock2LookUp.values == mySqlLookupService.getValues("mock2")
areLookupsEqual(mock2LookUp, mySqlLookupService.get("mock2", true))
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

package com.netflix.metacat.metadata.mysql;

import com.google.common.base.Joiner;
import com.google.common.collect.Sets;
import com.netflix.metacat.common.server.model.Lookup;
import com.netflix.metacat.common.server.properties.Config;
Expand All @@ -22,6 +23,7 @@
import lombok.extern.slf4j.Slf4j;
import org.springframework.dao.EmptyResultDataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.SqlParameterValue;
import org.springframework.jdbc.support.GeneratedKeyHolder;
import org.springframework.jdbc.support.KeyHolder;
import org.springframework.transaction.annotation.Transactional;
Expand Down Expand Up @@ -52,6 +54,9 @@ public class MySqlLookupService implements LookupService {
private static final String SQL_INSERT_LOOKUP_VALUE_IF_NOT_EXIST =
"INSERT IGNORE INTO lookup_values (lookup_id, values_string) VALUES (?, ?)";

private static final String SQL_DELETE_LOOKUP_VALUES_IF_NOT_IN =
"delete from lookup_values where lookup_id=? and values_string not in (%s)";

private static final String SQL_GET_LOOKUP_VALUES =
"select values_string value from lookup_values where lookup_id=?";
private static final String SQL_GET_LOOKUP_VALUES_BY_NAME =
Expand Down Expand Up @@ -168,6 +173,13 @@ private void insertLookupValuesIfNotExist(final Long id, final Set<String> inser
.collect(Collectors.toList()), new int[]{Types.BIGINT, Types.VARCHAR});
}

private void deleteLookupValuesIfNotIn(final Long id, final Set<String> values) {
jdbcTemplate.update(
String.format(SQL_DELETE_LOOKUP_VALUES_IF_NOT_IN,
"'" + Joiner.on("','").skipNulls().join(values) + "'"),
new SqlParameterValue(Types.BIGINT, id));
}

/**
* findOrCreateLookupByName.
*
Expand All @@ -193,6 +205,7 @@ private Lookup findOrCreateLookupByName(final String name, final boolean include
lookup = new Lookup();
lookup.setName(name);
lookup.setId(lookupId);
lookup.setValues(Collections.emptySet());
}
return lookup;
}
Expand All @@ -212,13 +225,54 @@ public Lookup addValues(final String name, final Set<String> values, final boole
insertLookupValuesIfNotExist(lookup.getId(), values);
}
if (includeValues) {
lookup.getValues().addAll(values);
if (lookup.getValues() == null || lookup.getValues().isEmpty()) {
lookup.setValues(values);
} else {
lookup.getValues().addAll(values);
}
}
return lookup;
} catch (Exception e) {
final String message = String.format("Failed to set the lookup values for name %s", name);
log.error(message, e);
throw new UserMetadataServiceException(message, e);
}
}

/**
* Saves the lookup value.
*
* @param name lookup name
* @param values multiple values
* @return returns the lookup with the given name.
*/
@Override
public Lookup setValues(final String name, final Set<String> values) {
try {
// For set values, no need to include values from querying rds
// since the values will be the same as the input values
final Lookup lookup = findOrCreateLookupByName(name, false);
if (!values.isEmpty()) {
insertLookupValuesIfNotExist(lookup.getId(), values);
deleteLookupValuesIfNotIn(lookup.getId(), values);
}
lookup.setValues(values);
return lookup;
} catch (Exception e) {
final String message = String.format("Failed to set the lookup values for name %s", name);
log.error(message, e);
throw new UserMetadataServiceException(message, e);
}
}
/**
* Saves the lookup value.
*
* @param name lookup name
* @param value lookup value
* @return returns the lookup with the given name.
*/
@Override
public Lookup setValue(final String name, final String value) {
return setValues(name, Sets.newHashSet(value));
}
}

0 comments on commit 0845991

Please sign in to comment.