[HOPSWORKS-3209] id and meta expectationId field are not matching if …

…meta already had expectationId (logicalclocks#996)
smkniazi · Jun 23, 2022 · 2895aac · 2895aac
1 parent 166ec5f
commit 2895aac
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 6 deletions.
diff --git a/hopsworks-IT/README.md b/hopsworks-IT/README.md
@@ -9,12 +9,13 @@
 `git clone https://${GIT_TOKEN}@github.com/${GIT_USER}/hopsworks-ee hopsworks`
 4. Setup ruby env:
   * `cd hopsworks/hopsworks-IT/src/test/ruby`
-  * `cp .env.example .env
+  * `cp .env.example .env`
   * .env setup - PROJECT_DIR=/home/vagrant/hopsworks 
   * .env setup - OS=centos or ubuntu as necessary
 5. 
   * Setup as root
     * `sudo su`
+    * `apt install ruby-dev libmysqlclient-dev`
     * `bundle install`
   * If step above fails
     * ubuntu - `apt-get install bundler`

diff --git a/hopsworks-IT/src/test/ruby/spec/featurestore_validation_spec.rb b/hopsworks-IT/src/test/ruby/spec/featurestore_validation_spec.rb
@@ -943,6 +943,38 @@
           dto_parsed = JSON.parse(json_result)
           expect(dto_parsed["count"]).to eq(0)
         end
+
+        it "expectations should have matching id with expectationId in meta field" do
+          project = get_project
+          featurestore_id = get_featurestore_id(project.id)
+          json_result, featuregroup_name = create_cached_featuregroup(project.id, featurestore_id)
+          fg_json = JSON.parse(json_result)
+          expectation_suite = generate_template_expectation_suite()
+          persist_expectation_suite(project.id, featurestore_id, fg_json["id"], expectation_suite)
+          json_result = get_expectation_suite(project.id, featurestore_id, fg_json["id"])
+          expect_status_details(200)
+          stored_expectation_suite = JSON.parse(json_result)
+          meta = JSON.parse(stored_expectation_suite["expectations"][0]["meta"])
+          expect(stored_expectation_suite["expectations"][0]["id"]).to eq(meta["expectationId"])
+        end
+
+        # When writing a fetched modified expectation suite to DB, expectationId in meta field is not changed.
+        # It is therefore incorrect as recreating the expectation suite changes all ids. On fetching the modified se  
+        # suite incorrect expectationIds should be overwritten by the expectation suite builder.
+        it "updated expectation suite should have correct expectationId in meta fields" do
+          project = get_project
+          featurestore_id = get_featurestore_id(project.id)
+          json_result, featuregroup_name = create_cached_featuregroup(project.id, featurestore_id)
+          fg_json = JSON.parse(json_result)
+          expectation_suite = generate_template_expectation_suite()
+          expectation_suite[:expectations][0][:meta] = expectation_suite[:expectations][0][:meta][0..-2] + ", \"expectationsId\":1}"
+          persist_expectation_suite(project.id, featurestore_id, fg_json["id"], expectation_suite)
+          json_result = get_expectation_suite(project.id, featurestore_id, fg_json["id"])
+          expect_status_details(200)
+          updated_expectation_suite = JSON.parse(json_result)
+          meta = JSON.parse(updated_expectation_suite["expectations"][0]["meta"])
+          expect(updated_expectation_suite["expectations"][0]["id"]).to eq(meta["expectationId"])
+        end
       end
     end
 

diff --git a/...a/io/hops/hopsworks/api/featurestore/datavalidationv2/suites/ExpectationSuiteBuilder.java b/...a/io/hops/hopsworks/api/featurestore/datavalidationv2/suites/ExpectationSuiteBuilder.java
@@ -76,10 +76,8 @@ public ExpectationSuiteDTO build(UriInfo uriInfo, Project project,
       // Set expectationId in the meta field
       try {
         JSONObject meta = new JSONObject(expectation.getMeta());
-        if (!meta.has("expectationId")) {
-          meta.put("expectationId", expectation.getId());
-          expectationDTO.setMeta(meta.toString());
-        }
+        meta.put("expectationId", expectation.getId());
+        expectationDTO.setMeta(meta.toString());
       } catch (JSONException e) {
         // Argument can be made that we simply return it, rather than throwing an exception
         throw new FeaturestoreException(

diff --git a/...rks-common/src/main/java/io/hops/hopsworks/common/featurestore/FeaturestoreConstants.java b/...rks-common/src/main/java/io/hops/hopsworks/common/featurestore/FeaturestoreConstants.java
@@ -132,7 +132,7 @@ private FeaturestoreConstants() {
   public static final int MAX_CHARACTERS_IN_EXPECTATION_EXPECTATION_TYPE = 150;
   public static final int MAX_CHARACTERS_IN_VALIDATION_REPORT_EVALUATION_PARAMETERS = 1000;
   public static final int MAX_CHARACTERS_IN_VALIDATION_REPORT_STATISTICS = 1000;
-  public static final int MAX_CHARACTERS_IN_VALIDATION_REPORT_META = 1000;
+  public static final int MAX_CHARACTERS_IN_VALIDATION_REPORT_META = 2000;
   public static final int MAX_CHARACTERS_IN_VALIDATION_RESULT_META = 1000;
   public static final int MAX_CHARACTERS_IN_VALIDATION_RESULT_EXCEPTION_INFO = 1000;
   public static final int MAX_CHARACTERS_IN_VALIDATION_RESULT_EXPECTATION_CONFIG = 2150;