refactor: update entity type references for consistency across importer formats#2245
refactor: update entity type references for consistency across importer formats#2245
Conversation
There was a problem hiding this comment.
Hello @robfrank, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
I, gemini-code-assist, have summarized this pull request. This PR focuses on improving code quality and consistency across the importer formats. Key changes include modernizing Java syntax by replacing if-else chains with switch expressions and traditional for loops with enhanced for-each loops, updating enum naming conventions (ENTITY_TYPE to EntityType, PHASE to Phase) for better adherence to Java standards, optimizing imports to remove unused ones and improve organization, and simplifying code logic, such as using computeIfAbsent in AnalyzedSchema.java.
Highlights
- Enum Renaming: The
ENTITY_TYPEenum inAnalyzedEntity.javahas been renamed toEntityType, and thePHASEenum inOrientDBImporter.javahas been renamed toPhaseto align with standard Java naming conventions. All references across affected files have been updated. - Syntax Modernization: Replaced
if-elsestatements with modernswitchexpressions inType.javafor handling different date/time types. Replaced a traditionalforloop with an enhancedfor-eachloop inAnalyzedEntity.java. - Import Optimization: Imports have been cleaned up and reorganized in several files (
JsonlExporterFormat.java,AnalyzedEntity.java,AnalyzedProperty.java,Neo4jImporter.java,OrientDBImporter.java,Source.java,SourceDiscovery.java,SourceSchema.java,CreateEdgeFromImportTask.java,LinkEdgeFromImportTask.java,RemoteDatabase.java,RemoteImmutableDocument.java) to remove unused imports and improve readability. - Code Simplification: The
getOrCreateEntitymethod inAnalyzedSchema.javawas simplified by usingcomputeIfAbsentinstead of a manual check and put operation. - Consistent Entity Type Usage: Updated method signatures and usages across various importer format files (
GraphMLImporterFormat.java,GraphSONImporterFormat.java,Importer.java,SourceDiscovery.java,CSVImporterFormat.java,FormatImporter.java,GloVeImporterFormat.java,JSONImporterFormat.java,JsonlImporterFormat.java,Neo4jImporterFormat.java,OrientDBImporterFormat.java,RDFImporterFormat.java,Word2VecImporterFormat.java,XMLImporterFormat.java) to use the newAnalyzedEntity.EntityTypeenum.
Changelog
Click here to see the changelog
- engine/src/main/java/com/arcadedb/schema/Type.java
- Replaced
if-else ifchain with aswitchexpression (lines 558-573) for handling type conversions toInstant.class.
- Replaced
- gremlin/src/main/java/com/arcadedb/gremlin/integration/importer/format/GraphMLImporterFormat.java
- Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinloadmethod signature (line 39). - Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinanalyzemethod signature (line 52).
- Updated parameter type from
- gremlin/src/main/java/com/arcadedb/gremlin/integration/importer/format/GraphSONImporterFormat.java
- Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinloadmethod signature (line 39). - Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinanalyzemethod signature (line 52).
- Updated parameter type from
- integration/src/main/java/com/arcadedb/integration/exporter/format/JsonlExporterFormat.java
- Reorganized and specified individual imports from
java.io.*,java.util.*,java.util.logging.*, andjava.util.zip.*(lines 40-48). - Reordered class members (lines 53-54).
- Reorganized and specified individual imports from
- integration/src/main/java/com/arcadedb/integration/importer/AnalyzedEntity.java
- Reorganized and specified individual imports from
java.util.*(lines 23-26). - Renamed
ENTITY_TYPEenum toEntityType(line 29). - Changed type of
typefield fromENTITY_TYPEtoEntityType(line 32). - Reordered class members (lines 34-36).
- Replaced traditional
forloop with enhancedfor-eachloop insetRowSizemethod (lines 68-70). - Changed return type of
getType()method fromENTITY_TYPEtoEntityType(line 83).
- Reorganized and specified individual imports from
- integration/src/main/java/com/arcadedb/integration/importer/AnalyzedProperty.java
- Reorganized and specified individual imports from
java.util.*(lines 23-24). - Reordered class members (lines 31-32).
- Reorganized and specified individual imports from
- integration/src/main/java/com/arcadedb/integration/importer/AnalyzedSchema.java
- Changed parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeingetOrCreateEntitymethod signature (line 31). - Replaced
if-elsecheck withcomputeIfAbsentlogic ingetOrCreateEntitymethod (line 32).
- Changed parameter type from
- integration/src/main/java/com/arcadedb/integration/importer/Importer.java
- Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypein calls toloadFromSource(lines 53-56). - Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinloadFromSourcemethod signature (line 80).
- Updated parameter type from
- integration/src/main/java/com/arcadedb/integration/importer/Neo4jImporter.java
- Reorganized and specified individual imports from
java.io.*,java.math.*,java.text.*,java.util.*,java.util.concurrent.atomic.*,java.util.stream.*, andjava.util.zip.*(lines 41-58). - Removed unused
PHASEenum declaration (lines 82-83).
- Reorganized and specified individual imports from
- integration/src/main/java/com/arcadedb/integration/importer/OrientDBImporter.java
- Reorganized and specified individual imports from
java.io.*,java.util.*,java.util.concurrent.atomic.*, andjava.util.zip.*(lines 47-62). - Changed type of
phasefield fromPHASEtoPhaseand updated initialization (line 102). - Renamed
PHASEenum toPhase(line 112). - Updated usage of
PHASE.ANALYZEtoPhase.ANALYZE(line 211). - Updated usage of
PHASE.CREATE_SCHEMAtoPhase.CREATE_SCHEMA(line 228). - Updated usage of
PHASE.CREATE_EDGEStoPhase.CREATE_EDGES(line 235). - Updated usage of
PHASE.CREATE_SCHEMA,PHASE.CREATE_RECORDS,PHASE.CREATE_EDGES, andPHASE.ANALYZEto usePhaseenum (lines 449-453). - Updated usage of
PHASE.CREATE_SCHEMAandPHASE.CREATE_RECORDSto usePhaseenum (lines 582-584).
- Reorganized and specified individual imports from
- integration/src/main/java/com/arcadedb/integration/importer/Source.java
- Reorganized and specified individual imports from
java.io.*,java.util.concurrent.*, andjava.util.logging.*(lines 23-26).
- Reorganized and specified individual imports from
- integration/src/main/java/com/arcadedb/integration/importer/SourceDiscovery.java
- Reorganized and specified individual imports from
java.io.*,java.lang.reflect.*,java.net.*,java.util.*,java.util.concurrent.*,java.util.concurrent.atomic.*,java.util.logging.*, andjava.util.zip.*(lines 34-52). - Improved formatting for
getSchemamethod signature (lines 66-69). - Removed unnecessary
if (formatImporter == null)check (lines 81-89). - Improved formatting for
getSourceFromURLmethod return statement (lines 116-140). - Improved formatting for
analyzeSourceContentmethod signature (lines 186-188). - Improved formatting for
Class.forNamecalls and catch blocks (lines 231-235, 242-246). - Improved formatting for log message in
analyzeSourceContent(lines 251-252). - Improved formatting for log message in
analyzeSourceContent(lines 326-327).
- Reorganized and specified individual imports from
- integration/src/main/java/com/arcadedb/integration/importer/SourceSchema.java
- Reorganized and specified individual imports from
java.util.*(lines 23-24). - Reordered class members (lines 27-28).
- Reorganized and specified individual imports from
- integration/src/main/java/com/arcadedb/integration/importer/format/CSVImporterFormat.java
- Improved formatting for
loadmethod signature (lines 61-65). - Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinanalyzemethod signature (line 434). - Updated usage of
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityType(lines 476-478).
- Improved formatting for
- integration/src/main/java/com/arcadedb/integration/importer/format/FormatImporter.java
- Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinloadmethod signature (line 34). - Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinanalyzemethod signature (line 37).
- Updated parameter type from
- integration/src/main/java/com/arcadedb/integration/importer/format/GloVeImporterFormat.java
- Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinloadmethod signature (line 44). - Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinanalyzemethod signature (line 59).
- Updated parameter type from
- integration/src/main/java/com/arcadedb/integration/importer/format/JSONImporterFormat.java
- Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinloadmethod signature (line 69). - Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinanalyzemethod signature (line 113).
- Updated parameter type from
- integration/src/main/java/com/arcadedb/integration/importer/format/JsonlImporterFormat.java
- Changed import from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityType(line 13). - Updated parameter type from
ENTITY_TYPEtoEntityTypeinloadmethod signature (line 44). - Updated parameter type from
ENTITY_TYPEtoEntityTypeinanalyzemethod signature (line 243).
- Changed import from
- integration/src/main/java/com/arcadedb/integration/importer/format/Neo4jImporterFormat.java
- Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinloadmethod signature (line 35). - Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinanalyzemethod signature (line 55).
- Updated parameter type from
- integration/src/main/java/com/arcadedb/integration/importer/format/OrientDBImporterFormat.java
- Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinloadmethod signature (line 36). - Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinanalyzemethod signature (line 56).
- Updated parameter type from
- integration/src/main/java/com/arcadedb/integration/importer/format/RDFImporterFormat.java
- Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinloadmethod signature (line 37).
- Updated parameter type from
- integration/src/main/java/com/arcadedb/integration/importer/format/Word2VecImporterFormat.java
- Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinloadmethod signature (line 44). - Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinanalyzemethod signature (line 60).
- Updated parameter type from
- integration/src/main/java/com/arcadedb/integration/importer/format/XMLImporterFormat.java
- Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinloadmethod signature (line 43). - Updated parameter type from
AnalyzedEntity.ENTITY_TYPEtoAnalyzedEntity.EntityTypeinanalyzemethod signature (line 140).
- Updated parameter type from
- integration/src/main/java/com/arcadedb/integration/importer/graph/CreateEdgeFromImportTask.java
- Reorganized and specified individual imports from
java.util.*andjava.util.logging.*(lines 41-43). - Improved formatting for constructor signature (lines 57-63).
- Added
@Overrideannotation toexecutemethod (line 73). - Improved formatting for log message in
execute(lines 108-113). - Improved formatting for
CompressedRID2RIDsIndexinstantiation (lines 120-122). - Improved formatting for
createEdgesInBatchmethod signature (lines 143-145). - Improved formatting for
createIncomingEdgesInBatchmethod signature (lines 163-164). - Improved formatting for log message in
createIncomingEdgesInBatch(lines 168-172). - Improved formatting for log message in
createIncomingEdgesInBatch(lines 205-207). - Improved formatting for log message in
createIncomingEdgesInBatch(lines 215-217).
- Reorganized and specified individual imports from
- integration/src/main/java/com/arcadedb/integration/importer/graph/GraphImporter.java
- Reorganized and specified individual imports from
java.util.*andjava.util.logging.*(lines 37-40). - Renamed
STATUSenum toStatus(line 47). - Changed type of
statusfield fromSTATUStoStatusand updated initialization (line 49). - Reordered class members in
GraphImporterThreadContext(lines 52-53). - Improved formatting for constructor signature (lines 64-65).
- Added check for active transaction before commit in
closemethod (lines 84-85). - Replaced traditional
forloop with enhancedfor-eachloop inclosemethod (lines 89-90). - Replaced traditional
forloop withArrays.fillinclosemethod (line 96). - Updated usage of
STATUS.CLOSEDtoStatus.CLOSED(line 98). - Replaced
final DatabaseInternal db = database;with direct usage ofdatabaseincreateVertex(lines 121-122). - Improved formatting for
createEdgemethod signature (lines 130-135). - Improved formatting for
async.scheduleTaskcall (lines 139-141). - Updated usage of
STATUS.IMPORTING_VERTEXtoStatus.IMPORTING_VERTEX(line 145). - Updated usage of
STATUS.IMPORTING_EDGEtoStatus.IMPORTING_EDGE(line 148). - Replaced traditional
forloop with enhancedfor-eachloop increateIncomingEdges(lines 177-178). - Improved formatting for log message in
createIncomingEdges(lines 195-197).
- Reorganized and specified individual imports from
- integration/src/main/java/com/arcadedb/integration/importer/graph/LinkEdgeFromImportTask.java
- Reorganized and specified individual imports from
java.util.*(line 31). - Improved formatting for constructor signature (lines 41-43).
- Added
@Overrideannotation toexecutemethod (line 49).
- Reorganized and specified individual imports from
- network/src/main/java/com/arcadedb/remote/RemoteDatabase.java
- Reorganized and specified individual imports from
java.io.*andjava.net.*(lines 45-49). - Added static import for
CAT_PROPERTY(line 51). - Used
CAT_PROPERTYconstant instead of literal string "@cat" (line 539). - Replaced
if-else ifchain with aswitchexpression (lines 541-545) for handling record categories.
- Reorganized and specified individual imports from
- network/src/main/java/com/arcadedb/remote/RemoteImmutableDocument.java
- Reorganized and specified individual imports from
java.util.*(lines 30-33). - Added static imports for
CAT_PROPERTYandTYPE_PROPERTY(lines 35, 37). - Added a blank line in the
getmethod (line 83). - Used
CAT_PROPERTYconstant instead of literal string "@cat" (line 96). - Used
TYPE_PROPERTYconstant instead of literal string "@type" (line 97). - Used
CAT_PROPERTYconstant instead of literal string "@cat" (line 108). - Used
TYPE_PROPERTYconstant instead of literal string "@type" (line 109).
- Reorganized and specified individual imports from
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Enums uppercase,
A style now past its grace.
Lowercase the name,
Consistency's the aim,
Refactor finds its place.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a good set of refactorings that enhance code readability, modernize syntax, and improve maintainability. The changes, particularly the consistent renaming of enums to follow Java conventions (e.g., EntityType, Phase) and the adoption of modern Java features like switch expressions and computeIfAbsent, are well-executed and beneficial.
The import optimizations and simplifications like using enhanced for-each loops also contribute positively to the codebase's clarity.
I've noted a few minor points for consideration, but overall, the changes are solid.
Summary of Findings
- Enum Naming Conventions: Successfully updated enums like
ENTITY_TYPEtoEntityTypeandPHASEtoPhase, adhering to Java naming standards. - Syntax Modernization: Effectively replaced
if-elsechains withswitchexpressions in several places, enhancing readability. - Code Simplification: Utilized modern Java features like
computeIfAbsentand enhancedfor-eachloops to simplify code and reduce boilerplate. - Import Optimization: Cleaned up imports across multiple files, improving maintainability.
- Use of Constants: Replaced magic strings like
"@cat"with defined constants (e.g.,Property.CAT_PROPERTY) in network communication classes, which is a good practice. - Potential NullPointerException: In
SourceDiscovery.java, a removed null check forformatImportermight lead to an NPE ifanalyzeSourceContentcan return null. This needs verification. - Switch Default Case Handling: In
Type.java, the newswitchexpression has an empty default case. Consider if explicit error handling for unexpected types would be more robust.
Merge Readiness
The pull request is in good shape and introduces valuable refactorings. The identified points are mostly minor or for clarification. Once these are considered or addressed, the PR should be ready for merging. As an AI, I cannot approve pull requests, so please ensure it's reviewed and approved by other team members.
| switch (value) { | ||
| case Instant instant -> { | ||
| if (property != null) | ||
| return instant.truncatedTo(DateUtils.getPrecisionFromType(property.getType())); | ||
| } else if (value instanceof Date date) | ||
| } | ||
| case Date date -> { | ||
| return DateUtils.dateTime(database, date.getTime(), ChronoUnit.MILLIS, LocalDateTime.class, | ||
| property != null ? DateUtils.getPrecisionFromType(property.getType()) : ChronoUnit.MILLIS); | ||
| else if (value instanceof Calendar calendar) | ||
| } | ||
| case Calendar calendar -> { | ||
| return DateUtils.dateTime(database, calendar.getTimeInMillis(), ChronoUnit.MILLIS, Instant.class, | ||
| property != null ? DateUtils.getPrecisionFromType(property.getType()) : ChronoUnit.MILLIS); | ||
| } | ||
| default -> { | ||
| } | ||
| } |
There was a problem hiding this comment.
The new switch expression is a good improvement for readability over the if-else if chain. However, the default case (lines 571-572) is currently empty. If the intention is that no action should be taken for other types, this is fine. But if this switch is expected to be exhaustive for Instant.class conversions or if other value types might lead to unexpected fall-through, consider if an explicit throw new IllegalArgumentException("Unsupported type for Instant conversion: " + value.getClass().getName()); or similar handling in the default case would be more robust. This would make unexpected types fail fast. What are your thoughts on this?
default -> {
throw new IllegalArgumentException("Unsupported type for Instant conversion: " + value.getClass().getName());
}| if (database.isTransactionActive()) | ||
| database.commit(); |
There was a problem hiding this comment.
The check if (database.isTransactionActive()) before committing is a good addition for robustness. It prevents attempting to commit if no transaction is active, which could lead to errors or unexpected behavior.
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
This pull request includes several changes aimed at improving code readability, modernizing syntax, and enhancing maintainability across multiple files. The most significant updates involve replacing legacy constructs with modern Java features, simplifying imports, and updating enum naming conventions.
Syntax Modernization
if-elsetype checks withswitchexpressions for better readability inType.java.Enum Naming and Usage
ENTITY_TYPEtoEntityTypeinAnalyzedEntity.javaand updated all references across the codebase to use the new naming convention. This improves adherence to Java naming standards. [1] [2] [3] [4] [5] [6] [7] [8]Import Optimization
Code Simplification
getOrCreateEntitymethod inAnalyzedSchema.javaby replacing manual checks withcomputeIfAbsent, reducing boilerplate code.forloop with an enhancedfor-eachloop inAnalyzedEntity.javafor better readability.Enum Refactoring
PHASEtoPhaseinOrientDBImporter.javaand updated all references to align with Java naming conventions. [1] [2] [3] [4] [5] [6]