Add back SetValues() as it is used internally

metacat-common-server/src/main/java/com/netflix/metacat/common/server/usermetadata/LookupService.java

@@ -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;
+    }
 }

metacat-functional-tests/build.gradle

@@ -42,6 +42,7 @@ configurations {
 
 
 dependencies {
+    testImplementation project(path: ':metacat-metadata-mysql')
     warproject(project(path: ':metacat-war', configuration: 'archives'))
 
     /*******************************
@@ -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}") {

metacat-functional-tests/metacat-test-cluster/docker-compose.yml

@@ -89,7 +89,7 @@ services:
             - MYSQL_PASSWORD=metacat_user_password
             - MYSQL_DATABASE=metacat
         ports:
-            - '3306'
+            - '3306:3306'
         labels:
           - "com.netflix.metacat.oss.test"
     postgresql:

metacat-functional-tests/src/functionalTest/groovy/com/netflix/metacat/MySqlLookupServiceSpec.groovy

@@ -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))
+    }
+}
+

metacat-metadata-mysql/src/main/java/com/netflix/metacat/metadata/mysql/MySqlLookupService.java

@@ -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;
@@ -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;
@@ -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 =
@@ -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.
      *
@@ -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;
     }
@@ -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));
+    }
 }