From 3b00de3b261f4b0dd28b8da7500c7b81dad2d132 Mon Sep 17 00:00:00 2001 From: Pinaki Poddar Date: Tue, 5 Aug 2008 07:23:41 +0000 Subject: [PATCH] OPENJPA-665: Check for null constraint on insert and update git-svn-id: https://svn.apache.org/repos/asf/openjpa/trunk@682610 13f79535-47bb-0310-9956-ffa450edef68 --- .../meta/strats/HandlerFieldStrategy.java | 25 ++++-- .../jdbc/meta/strats/HandlerStrategies.java | 26 ++++-- .../jdbc/meta/strats/localizer.properties | 1 + .../persistence/nullity/BlobValue.java | 33 ++++++++ .../persistence/nullity/NullValues.java | 56 +++++++++++++ .../nullity/TestBasicFieldNullity.java | 82 ++++++++++++++----- 6 files changed, 187 insertions(+), 36 deletions(-) create mode 100644 openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/nullity/BlobValue.java diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/HandlerFieldStrategy.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/HandlerFieldStrategy.java index a36c1ed2d..1b8246390 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/HandlerFieldStrategy.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/HandlerFieldStrategy.java @@ -42,6 +42,7 @@ import org.apache.openjpa.meta.JavaTypes; import org.apache.openjpa.meta.ValueStrategies; import org.apache.openjpa.util.InternalException; import org.apache.openjpa.util.MetaDataException; +import org.apache.openjpa.util.UserException; /** * Mapping for a single-valued field that delegates to a {@link ValueHandler}. @@ -122,19 +123,27 @@ public class HandlerFieldStrategy public void insert(OpenJPAStateManager sm, JDBCStore store, RowManager rm) throws SQLException { Row row = field.getRow(sm, store, rm, Row.ACTION_INSERT); - if (row != null) - HandlerStrategies.set(field, sm.fetch(field.getIndex()), store, - row, _cols, _io, field.getNullValue() == - FieldMapping.NULL_NONE); + if (row != null) { + Object value = sm.fetch(field.getIndex()); + if (!HandlerStrategies.set(field, value, store, row, _cols, _io, + field.getNullValue() == FieldMapping.NULL_NONE)) + if (field.getValueStrategy() != ValueStrategies.AUTOASSIGN) + throw new UserException(_loc.get("cant-set-value", + row.getFailedObject(), field, value)); + } } public void update(OpenJPAStateManager sm, JDBCStore store, RowManager rm) throws SQLException { Row row = field.getRow(sm, store, rm, Row.ACTION_UPDATE); - if (row != null) - HandlerStrategies.set(field, sm.fetch(field.getIndex()), store, - row, _cols, _io, field.getNullValue() == - FieldMapping.NULL_NONE); + if (row != null){ + Object value = sm.fetch(field.getIndex()); + if (!HandlerStrategies.set(field, value, store, row, _cols, _io, + field.getNullValue() == FieldMapping.NULL_NONE)) + if (field.getValueStrategy() != ValueStrategies.AUTOASSIGN) + throw new UserException(_loc.get("cant-set-value", + row.getFailedObject(), field, value)); + } } public void delete(OpenJPAStateManager sm, JDBCStore store, RowManager rm) diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/HandlerStrategies.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/HandlerStrategies.java index 04b6c4d8a..12f04c375 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/HandlerStrategies.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/strats/HandlerStrategies.java @@ -23,6 +23,7 @@ import java.sql.SQLException; import org.apache.openjpa.jdbc.kernel.JDBCFetchConfiguration; import org.apache.openjpa.jdbc.kernel.JDBCStore; import org.apache.openjpa.jdbc.meta.ClassMapping; +import org.apache.openjpa.jdbc.meta.FieldMapping; import org.apache.openjpa.jdbc.meta.RelationId; import org.apache.openjpa.jdbc.meta.ValueHandler; import org.apache.openjpa.jdbc.meta.ValueMapping; @@ -35,6 +36,7 @@ import org.apache.openjpa.jdbc.sql.Row; import org.apache.openjpa.kernel.OpenJPAStateManager; import org.apache.openjpa.lib.util.Localizer; import org.apache.openjpa.util.InvalidStateException; +import org.apache.openjpa.util.UserException; /** * Utility methods for strategies using value handlers. @@ -78,28 +80,38 @@ public class HandlerStrategies { /** * Set the given value into the given row. + * Return false if the given value can not be set, for example, due to + * null constraints on the columns. */ - public static void set(ValueMapping vm, Object val, JDBCStore store, + public static boolean set(ValueMapping vm, Object val, JDBCStore store, Row row, Column[] cols, ColumnIO io, boolean nullNone) throws SQLException { if (!canSetAny(row, io, cols)) - return; + return false; ValueHandler handler = vm.getHandler(); val = handler.toDataStoreValue(vm, val, store); + boolean isSet = false; if (val == null) { for (int i = 0; i < cols.length; i++) - if (canSet(row, io, i, true)) + if (canSet(row, io, i, true)) { + isSet = true; set(row, cols[i], null, handler, nullNone); + } } else if (cols.length == 1) { - if (canSet(row, io, 0, val == null)) + if (canSet(row, io, 0, val == null)) { + isSet = true; set(row, cols[0], val, handler, nullNone); + } } else { Object[] vals = (Object[]) val; for (int i = 0; i < vals.length; i++) - if (canSet(row, io, i, vals[i] == null)) + if (canSet(row, io, i, vals[i] == null)) { + isSet = true; set(row, cols[i], vals[i], handler, nullNone); + } } + return isSet; } /** @@ -108,9 +120,9 @@ public class HandlerStrategies { private static boolean canSet(Row row, ColumnIO io, int i, boolean nullValue) { if (row.getAction() == Row.ACTION_INSERT) - return io.isInsertable(i, nullValue); + return io.isInsertable(i, nullValue); if (row.getAction() == Row.ACTION_UPDATE) - return io.isUpdatable(i, nullValue); + return io.isUpdatable(i, nullValue); return true; } diff --git a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/meta/strats/localizer.properties b/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/meta/strats/localizer.properties index 6f7e16ebb..f441b19ff 100644 --- a/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/meta/strats/localizer.properties +++ b/openjpa-jdbc/src/main/resources/org/apache/openjpa/jdbc/meta/strats/localizer.properties @@ -136,3 +136,4 @@ unmapped-datastore-value: Instances of type "{0}" are not valid query \ parameters because the type is not mapped. cache-hit: SQL Cache hit with key: {0} in {1} cache-missed: SQL Cache missed with key: {0} in {1} +cant-set-value: Field "{1}" of "{0}" can not be set to "{2}" value. diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/nullity/BlobValue.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/nullity/BlobValue.java new file mode 100644 index 000000000..cc9b2dc75 --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/nullity/BlobValue.java @@ -0,0 +1,33 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.openjpa.persistence.nullity; + +import java.io.Serializable; + +/** + * Simple serializable entity for testing null constraint on field values. + * + * @author Pinaki Poddar + * + */ +public class BlobValue implements Serializable { + private String strVal; + private int intVal; + private byte[] bytes; +} diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/nullity/NullValues.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/nullity/NullValues.java index 02736a76b..0c3979915 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/nullity/NullValues.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/nullity/NullValues.java @@ -21,7 +21,9 @@ package org.apache.openjpa.persistence.nullity; import javax.persistence.Basic; import javax.persistence.Column; import javax.persistence.Entity; +import javax.persistence.GeneratedValue; import javax.persistence.Id; +import javax.persistence.Version; /** * Persistent entity used to test behavior of null constraint on basic fields. @@ -32,6 +34,7 @@ import javax.persistence.Id; @Entity public class NullValues { @Id + @GeneratedValue private long id; @Column(nullable=true) @@ -46,6 +49,22 @@ public class NullValues { @Basic(optional=false) private Integer notOptional; + @Column(nullable=true) + private BlobValue nullableBlob; + + @Column(nullable=false) + private BlobValue notNullableBlob; + + @Basic(optional=true) + private BlobValue optionalBlob; + + @Basic(optional=false) + private BlobValue notOptionalBlob; + + @Version + private int version; + + /** * Construct with all fields set to non-null values. */ @@ -54,6 +73,11 @@ public class NullValues { setNotOptional(42); setNotNullable(42); setNullable(42); + + setNullableBlob(new BlobValue()); + setNotNullableBlob(new BlobValue()); + setOptionalBlob(new BlobValue()); + setNotOptionalBlob(new BlobValue()); } public long getId() { @@ -91,4 +115,36 @@ public class NullValues { public void setNotOptional(Integer notOptional) { this.notOptional = notOptional; } + + public BlobValue getNullableBlob() { + return nullableBlob; + } + + public void setNullableBlob(BlobValue nullableBlob) { + this.nullableBlob = nullableBlob; + } + + public BlobValue getNotNullableBlob() { + return notNullableBlob; + } + + public void setNotNullableBlob(BlobValue notNullableBlob) { + this.notNullableBlob = notNullableBlob; + } + + public BlobValue getOptionalBlob() { + return optionalBlob; + } + + public void setOptionalBlob(BlobValue optionalBlob) { + this.optionalBlob = optionalBlob; + } + + public BlobValue getNotOptionalBlob() { + return notOptionalBlob; + } + + public void setNotOptionalBlob(BlobValue notOptionalBlob) { + this.notOptionalBlob = notOptionalBlob; + } } diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/nullity/TestBasicFieldNullity.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/nullity/TestBasicFieldNullity.java index ece0cf5b0..d8bf5cbbd 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/nullity/TestBasicFieldNullity.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/nullity/TestBasicFieldNullity.java @@ -22,6 +22,7 @@ import javax.persistence.EntityManager; import javax.persistence.RollbackException; import org.apache.openjpa.persistence.InvalidStateException; +import org.apache.openjpa.persistence.OpenJPAPersistence; import org.apache.openjpa.persistence.test.SingleEMFTestCase; @@ -34,86 +35,125 @@ import org.apache.openjpa.persistence.test.SingleEMFTestCase; * @author Pinaki Poddar */ public class TestBasicFieldNullity extends SingleEMFTestCase { - + private static boolean NEW = true; public void setUp() { - setUp(NullValues.class); + setUp(CLEAR_TABLES, NullValues.class); } public void testNullOnOptionalFieldIsAllowed() { NullValues pc = new NullValues(); - pc.setOptional(null); - assertCommitSucceeds(pc); + pc.setOptional(null); + assertCommitSucceeds(pc, NEW); } public void testNullOnNonOptionalFieldIsDisallowed() { NullValues pc = new NullValues(); pc.setNotOptional(null); - assertCommitFails(pc, InvalidStateException.class); + assertCommitFails(pc, NEW, InvalidStateException.class); } public void testNotNullOnOptionalFieldIsAllowed() { NullValues pc = new NullValues(); - assertCommitSucceeds(pc); + assertCommitSucceeds(pc, NEW); } public void testNotNullOnNonOptionalFieldIsAllowed() { NullValues pc = new NullValues(); - assertCommitSucceeds(pc); + assertCommitSucceeds(pc, NEW); } public void testNullOnNullableColumnAllowed() { NullValues pc = new NullValues(); pc.setNullable(null); - assertCommitSucceeds(pc); + assertCommitSucceeds(pc, NEW); } public void testNullOnNonNullableColumnIsDisallowed() { NullValues pc = new NullValues(); pc.setNotNullable(null); - assertCommitFails(pc, RollbackException.class); + assertCommitFails(pc, NEW, RollbackException.class); } public void testNotNullOnNullableColumnIsAllowed() { NullValues pc = new NullValues(); - assertCommitSucceeds(pc); + assertCommitSucceeds(pc, NEW); } public void testNotNullOnNonNullableColumnIsAllowed() { NullValues pc = new NullValues(); - assertCommitSucceeds(pc); + assertCommitSucceeds(pc, NEW); + } + + public void testNullOnOptionalBlobFieldIsAllowed() { + NullValues pc = new NullValues(); + pc.setOptionalBlob(null); + assertCommitSucceeds(pc, NEW); + } + + public void testNullOnNonOptionalBlobFieldIsDisallowed() { + NullValues pc = new NullValues(); + pc.setNotOptionalBlob(null); + assertCommitFails(pc, NEW, InvalidStateException.class); + } + + public void testNullOnNullableBlobColumnAllowed() { + NullValues pc = new NullValues(); + pc.setNullableBlob(null); + assertCommitSucceeds(pc, NEW); + } + + public void testNullOnNonNullableBlobColumnIsDisallowed() { + NullValues pc = new NullValues(); + pc.setNotNullableBlob(null); + assertCommitFails(pc, NEW, RollbackException.class); + } + + public void testX() { + NullValues pc = new NullValues(); + assertCommitSucceeds(pc, NEW); + OpenJPAPersistence.getEntityManager(pc).close(); + + pc.setNotNullableBlob(null); + assertCommitFails(pc, !NEW, RollbackException.class); + } /** * Asserts that the given instance can not be committed. */ - void assertCommitFails(Object pc, Class expected) { + void assertCommitFails(Object pc, boolean isNew, Class expected) { EntityManager em = emf.createEntityManager(); em.getTransaction().begin(); - em.persist(pc); + if (isNew) + em.persist(pc); + else { + Object merged = em.merge(pc); + } try { em.getTransaction().commit(); fail(); - } catch (RuntimeException e) { + } catch (Exception e) { if (!expected.isAssignableFrom(e.getClass())) { - fail("Expected " + expected.getName()); e.printStackTrace(); - } + fail("Expected " + expected.getName()); + } } } - void assertCommitSucceeds(Object pc) { + void assertCommitSucceeds(Object pc, boolean isNew) { EntityManager em = emf.createEntityManager(); em.getTransaction().begin(); - em.persist(pc); + if (isNew) + em.persist(pc); + else + em.merge(pc); try { em.getTransaction().commit(); } catch (RuntimeException e) { - fail(); e.printStackTrace(); + fail(); } } - - }