[CDAP-21172] Implement Search functionality for Metadata Tables #15990

123-komal · 2025-06-27T10:02:37Z

Description:

This PR is the follow-up for #15986 and here we are temporarily building this module separately as a workaround for a dependency issue that breaks the unified build. Its key changes are listed below:

SpannerMetadataStorage: Implemented Full-Text Search functionality for metadata searches in metadata tables.

Testing:

Local Testing:Tested these changes using a Distributed docker image.
Unit Test: Test cases to test different scenarios. Test cases for MetadataStorage classes are defined in the MetadataStorageTest class.

bhardwaj-priyanshu · 2025-06-30T16:48:56Z

...metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/SpannerMetadataStorage.java

+    return performSpannerSearch(request, cursor);
+  }
+
+  private io.cdap.cdap.spi.metadata.SearchResponse doSearch(SearchRequest request) throws IOException {


can we import this SearchResponse class?

bhardwaj-priyanshu · 2025-06-30T16:49:36Z

...metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/SpannerMetadataStorage.java

+
+      LOG.info(sql);
+      LOG.info(statement.toString());
+      LOG.info(resultSet.toString());


log statements should be informative

bhardwaj-priyanshu · 2025-06-30T16:50:41Z

...metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/SpannerMetadataStorage.java

+
+      if (results.isEmpty()) {
+        nextCursor = null;
+      }


this can be simplified I think can we use stream?

bhardwaj-priyanshu · 2025-06-30T16:51:58Z

...metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/SpannerMetadataStorage.java

+      return createSpannerSearchResponse(request, results, nextCursor);
+    } catch (SpannerException e) {
+      throw new IOException("Spanner search failed", e);
+    }


this is the internal function is it okay to let the function throw original exception and the caller should wrap it as needed?

bhardwaj-priyanshu · 2025-06-30T16:52:21Z

...metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/SpannerMetadataStorage.java

+      throw new IOException("Spanner search failed", e);
+    }
+  }
+  private MetadataRecord mapSpannerResult(ResultSet resultSet) {


bhardwaj-priyanshu · 2025-06-30T16:52:47Z

...metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/SpannerMetadataStorage.java

+      throw new IOException("Spanner search failed", e);
+    }
+  }
+  private MetadataRecord mapSpannerResult(ResultSet resultSet) {


even though its privat function please add the comment about what this function is supposed to do

added javadocs

bhardwaj-priyanshu · 2025-06-30T16:53:10Z

...metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/SpannerMetadataStorage.java

+    Metadata metadata = parseMetadataFromJson(metadataString);
+    MetadataEntity entity = toMetadataEntity(documentId);
+
+


remove extra space

bhardwaj-priyanshu · 2025-06-30T16:53:29Z

...metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/SpannerMetadataStorage.java

+    String documentId = resultSet.getString("metadata_id");
+    Struct row = resultSet.getCurrentRowAsStruct();
+    LOG.info(row.toString());
+    String metadataString = row.getJson(9);


removed the harcoded part

bhardwaj-priyanshu · 2025-06-30T16:53:41Z

...metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/SpannerMetadataStorage.java

+    }
+  }
+  private MetadataRecord mapSpannerResult(ResultSet resultSet) {
+    String documentId = resultSet.getString("metadata_id");


can we have const for this

bhardwaj-priyanshu · 2025-07-07T08:21:42Z

...metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/SpannerMetadataStorage.java

  private DatabaseClient dbClient;
  private DatabaseAdminClient adminClient;

+  // Define the wildcard characters you support for user input


rephrase the comment

bhardwaj-priyanshu · 2025-07-07T08:22:07Z

...metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/SpannerMetadataStorage.java

-    throw new IOException("NOT IMPLEMENTED");
+    return request.getCursor() != null && !request.getCursor().isEmpty()
+      ? doScroll(request) : doSearch(request);
+  }


bhardwaj-priyanshu · 2025-07-07T08:25:24Z

...metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/SpannerMetadataStorage.java

+      LOG.info("Found {} results.", results.size());
+
+      return createSearchResponse(request, results, nextActualCursor);
+


remove extra space

updated updated updated updated updated updated updated [CDAP-21172] Implement CRUD Operations for Metadata Tables updated updated updated updated updated updated updated formatted updated updated resolved checkstyle warnings updated updated updated updated updated updated updated formatted formatted updated updated updated formatted formatted updated updated [CDAP-21172] Implement the Search functionality updated updated updated updated updated updated updated updated updated updated updated updated updated updated updated updated updated updated

vsethi09 · 2025-07-11T06:59:24Z

cdap-metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/FormattedMetadata.java

  private FormattedMetadata(MetadataEntity entity, Metadata metadata) throws IOException {
-    this.namespace = entity.getValue("namespace");
+    this.namespace = Optional.ofNullable(entity.getValue("namespace"))
+      .orElse("default");


Is namespace is null, is it ok to use default value default or could it be system as well?

namespace default value is default.

vsethi09 · 2025-07-11T07:01:23Z

...metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/SpannerMetadataStorage.java

+  @VisibleForTesting
+  static final boolean KEEP = true;
+  @VisibleForTesting
+  static final boolean DISCARD = false;


No need to define const for these as they have single usage in source code. Use the boolean values in place.

For test code also use boolean values.

vsethi09 · 2025-07-11T07:07:28Z

...metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/SpannerMetadataStorage.java

-  public SearchResponse search(SearchRequest request) throws IOException {
-    throw new IOException("NOT IMPLEMENTED");
+  public SearchResponse search(SearchRequest request)  {
+    Cursor cursor = Optional.ofNullable(request.getCursor())


Can be simplified as:

Cursor cursor = Strings.isNullOrEmpty(request.getCursor()) ? null : Cursor.fromString(request.getCursor()); return doSearch(request, cursor);

OR

Cursor cursor; if !Strings.isNullOrEmpty(request.getCursor()) { cursor = Cursor.fromString(request.getCursor()); } return doSearch(request, cursor);

vsethi09 · 2025-07-11T07:10:35Z

...metadata-ext-spanner/src/main/java/io/cdap/cdap/metadata/spanner/SpannerMetadataStorage.java

+      params.forEach((key, value) -> statementBuilder.bind(key).to(value));
+      Statement statement = statementBuilder.build();
+
+      LOG.info("Executing Spanner SQL Template: {}", statement.getSql());


Please combine the log statements.

updated

sonarqubecloud · 2025-07-11T12:51:21Z

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

bhardwaj-priyanshu reviewed Jun 30, 2025

View reviewed changes

123-komal force-pushed the search_branch branch from 0542201 to 0468b14 Compare July 4, 2025 10:30

123-komal changed the base branch from develop to execute_mutation_branch-2 July 4, 2025 10:32

123-komal force-pushed the search_branch branch from 0468b14 to f5bcfc3 Compare July 4, 2025 13:15

123-komal force-pushed the execute_mutation_branch-2 branch from f4e1fa5 to 79d7f0b Compare July 4, 2025 14:25

123-komal force-pushed the search_branch branch from 8a5e67e to 07afd08 Compare July 7, 2025 07:08

bhardwaj-priyanshu reviewed Jul 7, 2025

View reviewed changes

123-komal force-pushed the search_branch branch 3 times, most recently from a4409ff to 7f50de8 Compare July 7, 2025 15:43

123-komal force-pushed the execute_mutation_branch-2 branch 6 times, most recently from f0ff86a to 7f9ee57 Compare July 9, 2025 04:30

123-komal force-pushed the search_branch branch 2 times, most recently from 449aea3 to 30020b0 Compare July 9, 2025 04:45

123-komal force-pushed the execute_mutation_branch-2 branch 2 times, most recently from ef0dd76 to aa6d333 Compare July 10, 2025 02:45

123-komal force-pushed the execute_mutation_branch-2 branch 3 times, most recently from d6884fe to de60c7c Compare July 10, 2025 05:42

Base automatically changed from execute_mutation_branch-2 to develop July 10, 2025 08:40

123-komal force-pushed the search_branch branch 4 times, most recently from f71c5d1 to ca54c13 Compare July 11, 2025 04:13

123-komal force-pushed the search_branch branch from ca54c13 to ec4c161 Compare July 11, 2025 06:44

vsethi09 reviewed Jul 11, 2025

View reviewed changes

123-komal added the build Triggers github actions build label Jul 11, 2025

updated

1a11376

updated

123-komal force-pushed the search_branch branch from 26806ee to 1a11376 Compare July 11, 2025 11:33

		Metadata metadata = parseMetadataFromJson(metadataString);
		MetadataEntity entity = toMetadataEntity(documentId);

		LOG.info("Found {} results.", results.size());

		return createSearchResponse(request, results, nextActualCursor);

[CDAP-21172] Implement Search functionality for Metadata Tables #15990

Are you sure you want to change the base?

[CDAP-21172] Implement Search functionality for Metadata Tables #15990

Uh oh!

Conversation

123-komal commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Testing:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jul 11, 2025

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

123-komal commented Jun 27, 2025 •

edited

Loading