Skip to content

feat(java,info): support multi-property in yaml #780

Merged
yangxk1 merged 6 commits intoapache:mainfrom
yangxk1:778-support-multi-porperices-in-yaml
Dec 8, 2025
Merged

feat(java,info): support multi-property in yaml #780
yangxk1 merged 6 commits intoapache:mainfrom
yangxk1:778-support-multi-porperices-in-yaml

Conversation

@yangxk1
Copy link
Contributor

@yangxk1 yangxk1 commented Sep 29, 2025

Reason for this PR

close #778

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@codecov-commenter
Copy link

codecov-commenter commented Sep 29, 2025

Codecov Report

❌ Patch coverage is 59.42029% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.60%. Comparing base (6f112c6) to head (b808b9b).
⚠️ Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
...rc/main/java/org/apache/graphar/info/EdgeInfo.java 0.00% 3 Missing and 3 partials ⚠️
...c/main/java/org/apache/graphar/info/GraphInfo.java 0.00% 3 Missing and 3 partials ⚠️
...java/org/apache/graphar/info/type/Cardinality.java 62.50% 3 Missing and 3 partials ⚠️
.../apache/graphar/info/saver/BaseGraphInfoSaver.java 28.57% 3 Missing and 2 partials ⚠️
.../java/org/apache/graphar/info/yaml/VertexYaml.java 66.66% 1 Missing and 1 partial ⚠️
...rc/main/java/org/apache/graphar/info/Property.java 90.00% 0 Missing and 1 partial ⚠️
.../main/java/org/apache/graphar/info/VertexInfo.java 66.66% 0 Missing and 1 partial ⚠️
...ava/org/apache/graphar/info/yaml/PropertyYaml.java 90.90% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##               main     #780    +/-   ##
==========================================
  Coverage     76.59%   76.60%            
- Complexity      560      585    +25     
==========================================
  Files            83       84     +1     
  Lines          8679     8788   +109     
  Branches       1006     1018    +12     
==========================================
+ Hits           6648     6732    +84     
- Misses         1813     1826    +13     
- Partials        218      230    +12     
Flag Coverage Δ
java-info 82.20% <59.42%> (-0.54%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yangxk1 yangxk1 requested a review from Thespica September 29, 2025 07:16
@yangxk1
Copy link
Contributor Author

yangxk1 commented Oct 9, 2025

Hey @Thespica, please help me review.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for multi-valued properties (LIST and SET cardinality) in GraphAr's Java implementation, enabling properties to hold multiple values instead of just single values. The implementation introduces a new Cardinality enum and integrates it throughout the info layer for YAML serialization/deserialization.

Key Changes

  • Introduces Cardinality enum (SINGLE, LIST, SET) to distinguish single-valued from multi-valued properties
  • Updates Property, PropertyYaml, and related info classes to support cardinality metadata
  • Adds validation rules: edges only support SINGLE cardinality, and CSV files don't support multi-cardinality properties
  • Enhances BaseGraphInfoSaver to auto-generate filenames when saving to directories

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 16 comments.

Show a summary per file
File Description
maven-projects/info/src/main/java/org/apache/graphar/info/type/Cardinality.java New enum defining SINGLE, LIST, and SET cardinality types with string conversion methods
maven-projects/info/src/main/java/org/apache/graphar/info/Property.java Adds cardinality field and constructors to support multi-valued properties
maven-projects/info/src/main/java/org/apache/graphar/info/yaml/PropertyYaml.java Adds cardinality field for YAML serialization, with special handling to omit SINGLE from output
maven-projects/info/src/main/java/org/apache/graphar/info/PropertyGroup.java Adds getCardinality() method and validation to reject multi-cardinality in CSV files
maven-projects/info/src/main/java/org/apache/graphar/info/VertexInfo.java Exposes getCardinality() method and adds validation calls before dumping
maven-projects/info/src/main/java/org/apache/graphar/info/EdgeInfo.java Adds validation to reject multi-cardinality properties in edges and validation calls before dumping
maven-projects/info/src/main/java/org/apache/graphar/info/GraphInfo.java Adds validation calls before dumping YAML
maven-projects/info/src/main/java/org/apache/graphar/info/yaml/VertexYaml.java Adds unused labels field (unrelated to multi-property feature)
maven-projects/info/src/main/java/org/apache/graphar/info/saver/BaseGraphInfoSaver.java Adds auto-filename generation when URIs are directories
maven-projects/info/src/test/java/org/apache/graphar/info/MultiPropertyTest.java Comprehensive test suite for cardinality functionality including YAML round-trip and validation
maven-projects/info/src/test/java/org/apache/graphar/info/TestDataFactory.java Adds factory method for creating properties with cardinality
maven-projects/info/src/test/java/org/apache/graphar/info/TestUtil.java Adds helper methods to access LDBC test data with multi-property support
maven-projects/info/src/test/java/org/apache/graphar/info/TestVerificationUtils.java Adds cardinality assertion to property comparison
maven-projects/info/src/test/java/org/apache/graphar/info/GraphInfoTest.java Adds cardinality assertions to existing property tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@Thespica Thespica left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yangxk1 yangxk1 merged commit 24dbe1b into apache:main Dec 8, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(java,info): support multi-property in yaml #768

4 participants