Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix npe if money is null #467

Closed
wants to merge 1 commit into from
Closed

Fix npe if money is null #467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions ...nate-types-52/src/main/java/com/vladmihalcea/hibernate/type/money/MonetaryAmountType.java
View file
Open in desktop
Expand Up @@ -75,14 +75,14 @@ public int hashCode(Object x) throws HibernateException {

@Override
public Object nullSafeGet(ResultSet rs, String[] names, SharedSessionContractImplementor session, Object owner) throws HibernateException, SQLException {
if (rs.wasNull()) {
return null;
}

String amountColumnName = names[0];
String currencyColumnName = names[1];

BigDecimal amount = rs.getBigDecimal(amountColumnName);
if(amount == null) {
Copy link
Contributor

@piotrooo piotrooo Aug 3, 2022 •
edited

Choose a reason for hiding this comment

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

Removing this null-check in favor of rs.wasNull() (see prev comment).

Copy link
Owner

Choose a reason for hiding this comment

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

@piotrooo The wasNull method call is useful only when you have primitive values on the Java side, but this is not the case here. We can use == null checks.

Copy link
Contributor

Choose a reason for hiding this comment

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

null seems to be enough.

return null;
}

String currency = rs.getString(currencyColumnName);

Copy link
Contributor

Choose a reason for hiding this comment

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

Null-check should be here. Because creating monetary type without currency make no sense and could rise an exception Currency Code may not be null.

I propose to use here:

if (rs.wasNull()) {
    return null;
}

Copy link
Contributor Author

@ugrave ugrave Aug 3, 2022 •
edited

Choose a reason for hiding this comment

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

Question is should the system thow an exception if only one of them is null or still ignored it an return also null in that case. I`m was not sure about this case and decide to let the exception be thrown.

Is there a diffferend if i use rs.wasNull() or check the value of the last read column on none primitive types?
To get the correct result of rs.wasNull() i need to read the column before. It also only checks if the last read column is null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the Java doc of the rs.getBigDicimal(x) says: if the value is SQL NULL, the value returned is null

Copy link
Contributor

Choose a reason for hiding this comment

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

null seems to be enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

But we should reconsider strategy. If any of a column has null - that should be null or exception? Maybe if any value is null - null should be returned.

return Money.of(amount, currency);
Expand Down
36 changes: 36 additions & 0 deletions ...s-52/src/test/java/com/vladmihalcea/hibernate/type/money/MySQLMonetaryAmountTypeTest.java
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@
import java.math.BigDecimal;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

/**
* @author Piotr Olaszewski
Expand Down Expand Up @@ -64,6 +65,31 @@ public void testSearchByMoney() {
});
}

@Test
public void testReturnNullMoney() {
long _id = doInJPA(entityManager -> {
Salary salary = new Salary();
entityManager.persist(salary);
return salary.getId();
});

doInJPA(entityManager -> {
Salary salary = entityManager.createQuery("select s from Salary s where s.id = :id", Salary.class)
.setParameter("id", _id)
.getSingleResult();

assertNull(salary.getSalary());
});

doInJPA(entityManager -> {
MonetaryAmount money = entityManager.createQuery("select s.salary from Salary s where s.id = :id", MonetaryAmount.class)
.setParameter("id", _id)
.getSingleResult();

assertNull(money);
});
}

@Entity(name = "Salary")
@Table(name = "salary")
@TypeDef(name = "monetary-amount-currency", typeClass = MonetaryAmountType.class, defaultForType = MonetaryAmount.class)
Expand All @@ -72,6 +98,8 @@ public static class Salary {
@GeneratedValue
private Long id;

private String other;

@Columns(columns = {
@Column(name = "salary_amount"),
@Column(name = "salary_currency")
Expand All @@ -94,5 +122,13 @@ public MonetaryAmount getSalary() {
public void setSalary(MonetaryAmount salary) {
this.salary = salary;
}

public String getOther() {
return other;
}

public void setOther(String other) {
this.other = other;
}
}
}
36 changes: 36 additions & 0 deletions ...src/test/java/com/vladmihalcea/hibernate/type/money/PostgreSQLMonetaryAmountTypeTest.java
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@
import java.math.BigDecimal;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

/**
* @author Piotr Olaszewski
Expand Down Expand Up @@ -64,6 +65,31 @@ public void testSearchByMoney() {
});
}

@Test
public void testReturnNullMoney() {
long _id = doInJPA(entityManager -> {
Salary salary = new Salary();
entityManager.persist(salary);
return salary.getId();
});

doInJPA(entityManager -> {
Salary salary = entityManager.createQuery("select s from Salary s where s.id = :id", Salary.class)
.setParameter("id", _id)
.getSingleResult();

assertNull(salary.getSalary());
});

doInJPA(entityManager -> {
MonetaryAmount money = entityManager.createQuery("select s.salary from Salary s where s.id = :id", MonetaryAmount.class)
.setParameter("id", _id)
.getSingleResult();

assertNull(money);
});
}

@Entity(name = "Salary")
@Table(name = "salary")
@TypeDef(name = "monetary-amount-currency", typeClass = MonetaryAmountType.class, defaultForType = MonetaryAmount.class)
Expand All @@ -72,6 +98,8 @@ public static class Salary {
@GeneratedValue
private Long id;

private String other;

@Columns(columns = {
@Column(name = "salary_amount"),
@Column(name = "salary_currency")
Expand All @@ -94,5 +122,13 @@ public MonetaryAmount getSalary() {
public void setSalary(MonetaryAmount salary) {
this.salary = salary;
}

public String getOther() {
return other;
}

public void setOther(String other) {
this.other = other;
}
}
}
8 changes: 4 additions & 4 deletions ...nate-types-55/src/main/java/com/vladmihalcea/hibernate/type/money/MonetaryAmountType.java
View file
Open in desktop
Expand Up @@ -75,14 +75,14 @@ public int hashCode(Object x) throws HibernateException {

@Override
public Object nullSafeGet(ResultSet rs, String[] names, SharedSessionContractImplementor session, Object owner) throws HibernateException, SQLException {
if (rs.wasNull()) {
return null;
}

String amountColumnName = names[0];
String currencyColumnName = names[1];

BigDecimal amount = rs.getBigDecimal(amountColumnName);
if(amount == null) {
return null;
}

String currency = rs.getString(currencyColumnName);

return Money.of(amount, currency);
Expand Down
36 changes: 36 additions & 0 deletions ...s-55/src/test/java/com/vladmihalcea/hibernate/type/money/MySQLMonetaryAmountTypeTest.java
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@
import java.math.BigDecimal;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

/**
* @author Piotr Olaszewski
Expand Down Expand Up @@ -64,6 +65,31 @@ public void testSearchByMoney() {
});
}

@Test
public void testReturnNullMoney() {
long _id = doInJPA(entityManager -> {
Salary salary = new Salary();
entityManager.persist(salary);
return salary.getId();
});

doInJPA(entityManager -> {
Salary salary = entityManager.createQuery("select s from Salary s where s.id = :id", Salary.class)
.setParameter("id", _id)
.getSingleResult();

assertNull(salary.getSalary());
});

doInJPA(entityManager -> {
MonetaryAmount money = entityManager.createQuery("select s.salary from Salary s where s.id = :id", MonetaryAmount.class)
.setParameter("id", _id)
.getSingleResult();

assertNull(money);
});
}

@Entity(name = "Salary")
@Table(name = "salary")
@TypeDef(name = "monetary-amount-currency", typeClass = MonetaryAmountType.class, defaultForType = MonetaryAmount.class)
Expand All @@ -72,6 +98,8 @@ public static class Salary {
@GeneratedValue
private Long id;

private String other;

@Columns(columns = {
@Column(name = "salary_amount"),
@Column(name = "salary_currency")
Expand All @@ -94,5 +122,13 @@ public MonetaryAmount getSalary() {
public void setSalary(MonetaryAmount salary) {
this.salary = salary;
}

public String getOther() {
return other;
}

public void setOther(String other) {
this.other = other;
}
}
}
36 changes: 36 additions & 0 deletions ...src/test/java/com/vladmihalcea/hibernate/type/money/PostgreSQLMonetaryAmountTypeTest.java
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@
import java.math.BigDecimal;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

/**
* @author Piotr Olaszewski
Expand Down Expand Up @@ -64,6 +65,31 @@ public void testSearchByMoney() {
});
}

@Test
public void testReturnNullMoney() {
long _id = doInJPA(entityManager -> {
Salary salary = new Salary();
entityManager.persist(salary);
return salary.getId();
});

doInJPA(entityManager -> {
Salary salary = entityManager.createQuery("select s from Salary s where s.id = :id", Salary.class)
.setParameter("id", _id)
.getSingleResult();

assertNull(salary.getSalary());
});

doInJPA(entityManager -> {
MonetaryAmount money = entityManager.createQuery("select s.salary from Salary s where s.id = :id", MonetaryAmount.class)
.setParameter("id", _id)
.getSingleResult();

assertNull(money);
});
}

@Entity(name = "Salary")
@Table(name = "salary")
@TypeDef(name = "monetary-amount-currency", typeClass = MonetaryAmountType.class, defaultForType = MonetaryAmount.class)
Expand All @@ -72,6 +98,8 @@ public static class Salary {
@GeneratedValue
private Long id;

private String other;

@Columns(columns = {
@Column(name = "salary_amount"),
@Column(name = "salary_currency")
Expand All @@ -94,5 +122,13 @@ public MonetaryAmount getSalary() {
public void setSalary(MonetaryAmount salary) {
this.salary = salary;
}

public String getOther() {
return other;
}

public void setOther(String other) {
this.other = other;
}
}
}
36 changes: 36 additions & 0 deletions ...s-60/src/test/java/com/vladmihalcea/hibernate/type/money/MySQLMonetaryAmountTypeTest.java
View file
Open in desktop
Expand Up @@ -10,6 +10,7 @@
import java.math.BigDecimal;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;

/**
* @author Piotr Olaszewski
Expand Down Expand Up @@ -62,13 +63,40 @@ public void testSearchByMoney() {
});
}

@Test
public void testReturnNullMoney() {
long _id = doInJPA(entityManager -> {
Salary salary = new Salary();
entityManager.persist(salary);
return salary.getId();
});

doInJPA(entityManager -> {
Salary salary = entityManager.createQuery("select s from Salary s where s.id = :id", Salary.class)
.setParameter("id", _id)
.getSingleResult();

assertNull(salary.getSalary());
});

doInJPA(entityManager -> {
MonetaryAmount money = entityManager.createQuery("select s.salary from Salary s where s.id = :id", MonetaryAmount.class)
.setParameter("id", _id)
.getSingleResult();

assertNull(money);
});
}

@Entity(name = "Salary")
@Table(name = "salary")
public static class Salary {
@Id
@GeneratedValue
private Long id;

private String other;

@AttributeOverride(name = "amount", column = @Column(name = "salary_amount"))
@AttributeOverride(name = "currency", column = @Column(name = "salary_currency"))
@CompositeType(MonetaryAmountType.class)
Expand All @@ -89,5 +117,13 @@ public MonetaryAmount getSalary() {
public void setSalary(MonetaryAmount salary) {
this.salary = salary;
}

public String getOther() {
return other;
}

public void setOther(String other) {
this.other = other;
}
}
}