Skip to content

feat: add support for mapping transient fields with entity class hierarchy introspection #168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 24, 2019

Conversation

igdianov
Copy link
Collaborator

@igdianov igdianov commented Aug 24, 2019

This PR adds support for mapping entity fields that use Java transient modifier JPA transient fields mapping specification, i.e.

@MappedSuperclass
public class ParentCalculatedEntity {

 private transient String parentTransientModifier; // transient property

}
@Data
@EqualsAndHashCode(callSuper = true)
@Entity
public class CalculatedEntity extends ParentCalculatedEntity {

  private transient Integer transientModifier; // transient property

}

The transient Java fields can be excluded from schema using @GraphQLIgnore annotation.

…tion

fix(IntrospectoinUtils): throw RuntimeException for non existing fields


fix: Jacoco RTE


fix: add test coverage for transient fields with class hierarchy support


fix: test typo
@igdianov igdianov self-assigned this Aug 24, 2019
@codecov
Copy link

codecov bot commented Aug 24, 2019

Codecov Report

Merging #168 into master will increase coverage by 0.23%.
The diff coverage is 96.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #168      +/-   ##
============================================
+ Coverage     68.12%   68.36%   +0.23%     
- Complexity      502      513      +11     
============================================
  Files            33       33              
  Lines          2576     2598      +22     
  Branches        431      431              
============================================
+ Hits           1755     1776      +21     
- Misses          644      645       +1     
  Partials        177      177
Impacted Files Coverage Δ Complexity Δ
...jpa/query/schema/impl/GraphQLJpaSchemaBuilder.java 89.04% <100%> (ø) 129 <1> (+3) ⬆️
...phql/jpa/query/schema/impl/IntrospectionUtils.java 88.88% <96.55%> (+4.51%) 14 <11> (+8) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8028d12...fbf54b8. Read the comment docs.

@igdianov igdianov merged commit 81a6a19 into master Aug 24, 2019
@igdianov igdianov deleted the igidianov-transient-modifiers-support branch August 24, 2019 14:03
}

private static boolean isAnnotationPresent(Class<?> entity, String propertyName, Class<? extends Annotation> annotation){
private static Optional<Boolean> isAnnotationPresent(Class<?> entity, String propertyName, Class<? extends Annotation> annotation){
Copy link
Contributor

@anotender anotender Aug 26, 2019

Choose a reason for hiding this comment

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

Returning Optional<Boolean> is a little bit misleading in this case. In my opinion calling isXXX methods should always return pure boolean value by convention. Using API with Optional<Boolean> leads us to the three-state logic we need to deal with. What do you think?

public static boolean isIgnored(Class<?> entity, String propertyName) {
return isAnnotationPresent(entity, propertyName, GraphQLIgnore.class);
return isAnnotationPresent(entity, propertyName, GraphQLIgnore.class)
.orElseThrow(() -> new RuntimeException(new NoSuchFieldException(propertyName)));
Copy link
Contributor

Choose a reason for hiding this comment

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

The orElse(false) was added before to make sure that the application can start even if the field is not found. There may be a case that we have a getter that is not directly connected with any of the class' properties but is still mapped to the column in the database e.g.

public boolean isValuePresent(){
    return value != null;
}

But you are right that we should probably throw this error anyway and try to deal with such failure on the higher level. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants