add a structured type to batchGet in OpenAPI V3 spec

[kevin1chun]

This will enable code-generation from the open api V3 spec to use a specific type rather than having to guess (and incorrectly) assign types

Checklist

• The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
• Links to related issues (if applicable)
• Tests for the changes have been added/updated (if applicable)
• Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
• For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

Summary by CodeRabbit

• New Features

  • Introduced a new BatchGetRequestBody schema for improved handling of batch requests in the API.
  • Added @Hidden annotation to the EntityController to exclude it from API documentation.

• Improvements

  • Updated namespaces in the Avro schema for MetadataChangeProposal and KafkaAuditHeader, enhancing clarity and organization.
  • Changed the default behavior of the createAspect method to automatically create aspects if they do not exist.

• Tests

  • Enhanced testing for the BatchGetContainerEntityRequest_v3 schema by adding validations to ensure properties reference the correct schema paths.

[coderabbitai]

Walkthrough

The recent changes enhance the organization and clarity of the Avro schema for MetadataChangeProposal and introduce a new BatchGetRequestBody schema in the OpenAPI generator. Namespace modifications streamline the structure, while updates to the controller methods improve functionality and usability. Enhanced tests increase reliability by validating schema properties and ensuring robust behavior, setting the stage for easier maintenance and future enhancements.

Changes

Files	Change Summary
metadata-integration/java/datahub-event/src/main/resources/MetadataChangeProposal.avsc	Updated namespaces for MetadataChangeProposal, KafkaAuditHeader, and ChangeType to improve clarity; simplified class references for URN handling.
metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/OpenAPIV3Generator.java	Added BatchGetRequestBody schema for batch requests, enhancing modularity by replacing inline headers with centralized references; improved method clarity.
metadata-service/openapi-servlet/src/test/java/io/datahubproject/openapi/v3/OpenAPIV3GeneratorTest.java	Enhanced testOpenApiSpecBuilder with additional assertions to validate schema properties, improving test robustness and coverage.
metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/controller/GenericEntitiesController.java	Changed the default value of the createIfNotExists parameter in the createAspect method from false to true, allowing automatic creation of aspects.
metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/controller/EntityController.java	Added @Hidden annotation to the EntityController class to exclude it from generated API documentation, enhancing control over API visibility.

Poem

🐰 In the land of code where changes play,
New schemas bloom, brightening the way.
With namespaces tidy, and tests that gleam,
We hop to the future, chasing the dream!
Let's dance through the bytes, make our code sing,
Celebrating enhancements that each change brings! 🎉

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[kevin1chun]

Hmm this seems unrelated but was applied when i ran ./gradlew :metadata-service:openapi-servlet:spotlessApply

[coderabbitai]

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 44cdb04 and df460bc.

Files selected for processing (3)

• metadata-integration/java/datahub-event/src/main/resources/MetadataChangeProposal.avsc (3 hunks)
• metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/OpenAPIV3Generator.java (2 hunks)
• metadata-service/openapi-servlet/src/test/java/io/datahubproject/openapi/v3/OpenAPIV3GeneratorTest.java (1 hunks)

Additional comments not posted (6)

metadata-service/openapi-servlet/src/test/java/io/datahubproject/openapi/v3/OpenAPIV3GeneratorTest.java (1)

91-103: LGTM! The new assertions enhance test robustness.

The added assertions validate the properties of the BatchGetContainerEntityRequest_v3 schema, ensuring they reference the correct schema path.

metadata-integration/java/datahub-event/src/main/resources/MetadataChangeProposal.avsc (3)

4-4: Namespace change approved.

The namespace for MetadataChangeProposal has been updated to com.linkedin.mxe for better organization.

---

11-11: Namespace change approved.

The namespace for KafkaAuditHeader has been updated to com.linkedin.avro2pegasus.events for better alignment with the Avro to Pegasus conversion process.

---

102-102: Namespace change approved.

The namespace for ChangeType has been updated to com.linkedin.events.metadata for better clarity and organization.

metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/OpenAPIV3Generator.java (2)

85-98: Addition of BatchGetRequestBody schema approved.

The new schema encapsulates the request body for batch get aspects, enhancing the API's capability to handle batch requests with a clear structure.

---

667-674: Modification to buildEntityBatchGetRequestSchema approved.

The method now utilizes the BatchGetRequestBody schema, promoting better modularity and reusability.

[coderabbitai]

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between df460bc and 5e51f2a.

Files selected for processing (2)

• metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/OpenAPIV3Generator.java (2 hunks)
• metadata-service/openapi-servlet/src/test/java/io/datahubproject/openapi/v3/OpenAPIV3GeneratorTest.java (1 hunks)

Files skipped from review as they are similar to previous changes (2)

• metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/OpenAPIV3Generator.java
• metadata-service/openapi-servlet/src/test/java/io/datahubproject/openapi/v3/OpenAPIV3GeneratorTest.java

[coderabbitai]

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 5e51f2a and 97d5b28.

Files selected for processing (1)

• metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/OpenAPIV3Generator.java (3 hunks)

Files skipped from review as they are similar to previous changes (1)

• metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/OpenAPIV3Generator.java

[kevin1chun]

See GenericEntitiesController.createAspect.

The jsonAspect is directly deserialized to the MCP

  protected ChangeMCP toUpsertItem(
      @Nonnull AspectRetriever aspectRetriever,
      Urn entityUrn,
      AspectSpec aspectSpec,
      Boolean createIfNotExists,
      String jsonAspect,
      Actor actor)
      throws URISyntaxException {
    return ChangeItemImpl.builder()
        .urn(entityUrn)
        .aspectName(aspectSpec.getName())
        .changeType(Boolean.TRUE.equals(createIfNotExists) ? ChangeType.CREATE : ChangeType.UPSERT)
        .auditStamp(AuditStampUtils.createAuditStamp(actor.toUrnStr()))
        .recordTemplate(
            GenericRecordUtils.deserializeAspect(
                ByteString.copyString(jsonAspect, StandardCharsets.UTF_8),
                GenericRecordUtils.JSON,
                aspectSpec))
        .build(aspectRetriever);
  }

I believe @david-leifker intention was to pass the wrapped aspect, but this will unblock us until he's back and can inspect further

[coderabbitai]

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 97d5b28 and 1d2c7e7.

Files selected for processing (3)

• metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/controller/GenericEntitiesController.java (1 hunks)
• metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/OpenAPIV3Generator.java (3 hunks)
• metadata-service/openapi-servlet/src/test/java/io/datahubproject/openapi/v3/OpenAPIV3GeneratorTest.java (1 hunks)

Files skipped from review as they are similar to previous changes (2)

• metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/OpenAPIV3Generator.java
• metadata-service/openapi-servlet/src/test/java/io/datahubproject/openapi/v3/OpenAPIV3GeneratorTest.java

Additional comments not posted (1)

metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/controller/GenericEntitiesController.java (1)

511-511: Change in Default Behavior: createIfNotExists

The default value of the createIfNotExists parameter has been changed from false to true. This change means that if the createIfNotExists parameter is not provided in the request, the system will now default to creating the aspect if it does not already exist. This enhances the method's functionality by allowing for a more permissive creation policy.

Ensure that this change is communicated to all relevant stakeholders and that any dependent systems or documentation are updated accordingly.

[coderabbitai]

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1d2c7e7 and 95ecfd3.

Files selected for processing (1)

• metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/controller/EntityController.java (2 hunks)

Additional comments not posted (1)

metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/controller/EntityController.java (1)

71-71: Hiding the EntityController from API documentation.

The addition of the @Hidden annotation will hide the EntityController and its endpoints from the generated API documentation. Ensure this aligns with the overall API design and documentation strategy.

Verify that hiding this controller does not impact any public-facing documentation or intended API consumers.

[RyanHolstien]

Please note this default behavior change in updating-datahub.md as this may change workflows built around this endpoint, it does make more sense to me as a default though.

[kevin1chun]

This was recently added in #10654 for us specifically and no one is using this afaik, but i will note it just in case

[kevin1chun]

Actually @RyanHolstien is that necessary if there was no release published between the changes?
This change (#10654) was implemented after v0.13.3 was released

[RyanHolstien]

Nope! That's fine if it hasn't been released yet :)

[RyanHolstien]

Probably makes more sense to do this as a anyMatch() rather than filtering specific entries, no? Otherwise this test will likely need to be updated every time a new component gets added.

[kevin1chun]

So I specifically wanted to test all attributes in this batchGet request body except urn.
Reasoning is that all attributes should be mapped to a specific format, but urn is a standalone string that is used as a key

[RyanHolstien]

This does not feel like the correct change, it's not inline with how the other APIs are structured and significantly changes the contract. I don't think I'm comfortable signing off on this for it to just be flipped back once David comes back. I believe the right fix here would be to modify the toUpsertItem method to pull the top level field that contains the actual aspect so:

  protected ChangeMCP toUpsertItem(
      @Nonnull AspectRetriever aspectRetriever,
      Urn entityUrn,
      AspectSpec aspectSpec,
      Boolean createIfNotExists,
      String jsonAspect,
      Actor actor) throws URISyntaxException, JsonProcessingException {
    JsonNode jsonNode = objectMapper.readTree(jsonAspect);
    String aspectJson = jsonNode.get("value").toString();
    return ChangeItemImpl.builder()
        .urn(entityUrn)
        .aspectName(aspectSpec.getName())
        .changeType(Boolean.TRUE.equals(createIfNotExists) ? ChangeType.CREATE : ChangeType.UPSERT)
        .auditStamp(AuditStampUtils.createAuditStamp(actor.toUrnStr()))
        .recordTemplate(
            GenericRecordUtils.deserializeAspect(
                ByteString.copyString(aspectJson, StandardCharsets.UTF_8),
                GenericRecordUtils.JSON,
                aspectSpec))
        .build(aspectRetriever);
  }

[kevin1chun]

That works for me too 👍
I wanted to minimize the change while David was out but this will fix it as well
I'll go ahead and make this change instead

[coderabbitai]

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (1)

metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/controller/GenericEntitiesController.java (1)

512-512: Document the default behavior change.

The default value of createIfNotExists has been changed to true. Note this change in the documentation as it may affect workflows built around this endpoint.

• File: metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/controller/GenericEntitiesController.java
• Line: 512

Analysis chain

Document the default behavior change.

The default value of createIfNotExists has been changed to true. Note this change in the documentation as it may affect workflows built around this endpoint.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify if the default behavior change is documented.

# Test: Search for documentation updates. Expect: Documentation should mention the default value change.
rg --type md 'createIfNotExists'

Length of output: 32

---

Script:

#!/bin/bash
# Description: Verify if the createAspect method is documented in the codebase.

# Test: Search for any documentation related to the createAspect method.
rg 'createAspect'

Length of output: 4811

---

Script:

#!/bin/bash
# Description: Verify if the createIfNotExists parameter is documented within the GenericEntitiesController.java file.

# Test: Search for any comments or JavaDoc annotations mentioning createIfNotExists in the GenericEntitiesController.java file.
rg 'createIfNotExists' metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/controller/GenericEntitiesController.java

Length of output: 428

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 95ecfd3 and e377b43.

Files selected for processing (4)

• metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/controller/GenericEntitiesController.java (4 hunks)
• metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/OpenAPIV3Generator.java (2 hunks)
• metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/controller/EntityController.java (2 hunks)
• metadata-service/openapi-servlet/src/test/java/io/datahubproject/openapi/v3/OpenAPIV3GeneratorTest.java (1 hunks)

Files skipped from review as they are similar to previous changes (3)

• metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/OpenAPIV3Generator.java
• metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/v3/controller/EntityController.java
• metadata-service/openapi-servlet/src/test/java/io/datahubproject/openapi/v3/OpenAPIV3GeneratorTest.java

Additional comments not posted (2)

metadata-service/openapi-servlet/src/main/java/io/datahubproject/openapi/controller/GenericEntitiesController.java (2)

515-515: LGTM! The addition of JsonProcessingException improves robustness.

The inclusion of JsonProcessingException in the list of thrown exceptions enhances error handling for JSON parsing errors.

---

736-738: LGTM! Ensure correctness of JSON parsing.

The changes involve reading the JSON aspect and constructing the ChangeMCP object. Ensure the JSON parsing logic is correct.