diff --git a/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/SchemaSpec.groovy b/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/SchemaSpec.groovy index a637233f7a6..76eb909980f 100644 --- a/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/SchemaSpec.groovy +++ b/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/SchemaSpec.groovy @@ -191,9 +191,9 @@ class SchemaSpec extends Specification implements GraphQLSchemaSpec { GraphQLInputObjectType type = schema.getType('ToOneCreate') expect: - unwrap(null, type.getFieldDefinition('circularOne').type) == schema.getType('CircularOneCreateNested') - unwrap(null, type.getFieldDefinition('one').type) == schema.getType('OneCreateNested') - unwrap(null, type.getFieldDefinition('anEnum').type) == schema.getType('Enum') + type.getFieldDefinition('circularOne').type == schema.getType('CircularOneCreateNested') + type.getFieldDefinition('one').type == schema.getType('OneCreateNested') + type.getFieldDefinition('anEnum').type == schema.getType('Enum') //everything else is a scalar.. not worth testing every property } diff --git a/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/entity/property/impl/HibernatePersistentGraphQLPropertySpec.groovy b/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/entity/property/impl/HibernatePersistentGraphQLPropertySpec.groovy index d987426a136..15617d3452f 100644 --- a/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/entity/property/impl/HibernatePersistentGraphQLPropertySpec.groovy +++ b/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/entity/property/impl/HibernatePersistentGraphQLPropertySpec.groovy @@ -467,8 +467,18 @@ class Book { static hasMany = [authors: Author, tags: Tag, basics: String, otherBookTypes: BookType] static constraints = { - description nullable: true - nullBookType nullable: true + title nullable: false + metadata nullable: false + bookType nullable: false + + otherMetadata nullable: false + otherMetadata2 nullable: false + someOtherMetadata nullable: false + + authors nullable: false + tags nullable: false + basics nullable: false + otherBookTypes nullable: false } static embedded = ['otherMetadata2', 'someOtherMetadata'] @@ -485,6 +495,10 @@ class Book2 implements Serializable { static mapping = { id composite: ['title', 'description'] + title nullable: false + description nullable: false + metadata nullable: false + bookType nullable: false } int hashCode() { diff --git a/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/types/input/EmbeddedInputObjectTypeBuilderSpec.groovy b/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/types/input/EmbeddedInputObjectTypeBuilderSpec.groovy index 03d0ca48d85..73ee7750ec4 100644 --- a/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/types/input/EmbeddedInputObjectTypeBuilderSpec.groovy +++ b/grails-data-graphql/core/src/test/groovy/org/grails/gorm/graphql/types/input/EmbeddedInputObjectTypeBuilderSpec.groovy @@ -84,6 +84,6 @@ class EmbeddedInputObjectTypeBuilderSpec extends HibernateSpec { then: 'one is included because it is the owning side, many is included because it is not bidirectional' props*.name == ['one', 'many'] - props.any { !it.nullable } //some are not nullable + !props.any { !it.nullable } //all are nullable by default } } diff --git a/grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/cfg/HibernateMappingBuilder.groovy b/grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/cfg/HibernateMappingBuilder.groovy index 4e9ac40c952..a2613eaea43 100644 --- a/grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/cfg/HibernateMappingBuilder.groovy +++ b/grails-data-hibernate5/core/src/main/groovy/org/grails/orm/hibernate/cfg/HibernateMappingBuilder.groovy @@ -34,7 +34,6 @@ import org.grails.datastore.mapping.reflect.ClassPropertyFetcher * Implements the ORM mapping DSL constructing a model that can be evaluated by the * GrailsDomainBinder class which maps GORM classes onto the database. * - * @author Graeme Rocher * @since 1.0 */ @@ -468,7 +467,9 @@ class HibernateMappingBuilder implements MappingConfigurationBuilder { + String name +} + +@Entity +class MappingContextConstrainedEntity implements HibernateEntity { + String name + + static constraints = { + name maxSize: 100 + } +} + +@Entity +class MappingContextWildcardMappedEntity implements HibernateEntity { + String name + + static mapping = { + '*' cache: true + } +} + +@Entity +class MappingContextCompositeIdEntity implements HibernateEntity { + String tenantId + String code + + static mapping = { + id composite: ['tenantId', 'code'] + } +} + // --- helpers for unit tests --- @Entity diff --git a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/EnumTypeBinderSpec.groovy b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/EnumTypeBinderSpec.groovy index 990260ba00e..147d5310fda 100644 --- a/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/EnumTypeBinderSpec.groovy +++ b/grails-data-hibernate7/core/src/test/groovy/org/grails/orm/hibernate/cfg/domainbinding/EnumTypeBinderSpec.groovy @@ -18,35 +18,33 @@ */ package org.grails.orm.hibernate.cfg.domainbinding -import org.hibernate.type.descriptor.WrapperOptions +import java.sql.PreparedStatement +import java.sql.ResultSet +import java.sql.SQLException -import grails.gorm.tests.HibernateGormDatastoreSpec -import grails.persistence.Entity -import org.grails.datastore.mapping.model.PersistentProperty -import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateBasicProperty -import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateEnumProperty -import org.grails.orm.hibernate.cfg.IdentityEnumType import jakarta.persistence.EnumType -import org.grails.orm.hibernate.cfg.domainbinding.binder.GrailsDomainBinder -import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateToManyProperty -import org.grails.orm.hibernate.cfg.domainbinding.util.BackticksRemover -import org.grails.orm.hibernate.cfg.domainbinding.util.ColumnNameForPropertyAndPathFetcher -import org.grails.orm.hibernate.cfg.domainbinding.util.DefaultColumnNameFetcher -import org.hibernate.engine.spi.SharedSessionContractImplementor -import org.hibernate.mapping.Table + import org.hibernate.mapping.RootClass +import org.hibernate.mapping.Table +import org.hibernate.type.descriptor.WrapperOptions import org.hibernate.usertype.UserType import spock.lang.Subject import spock.lang.Unroll -import java.sql.PreparedStatement -import java.sql.ResultSet -import java.sql.SQLException - +import grails.gorm.tests.HibernateGormDatastoreSpec +import grails.persistence.Entity +import org.grails.datastore.mapping.model.PersistentProperty +import org.grails.orm.hibernate.cfg.IdentityEnumType import org.grails.orm.hibernate.cfg.domainbinding.binder.ColumnConfigToColumnBinder import org.grails.orm.hibernate.cfg.domainbinding.binder.EnumTypeBinder +import org.grails.orm.hibernate.cfg.domainbinding.binder.GrailsDomainBinder import org.grails.orm.hibernate.cfg.domainbinding.binder.IndexBinder import org.grails.orm.hibernate.cfg.domainbinding.hibernate.GrailsHibernatePersistentEntity +import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateBasicProperty +import org.grails.orm.hibernate.cfg.domainbinding.hibernate.HibernateEnumProperty +import org.grails.orm.hibernate.cfg.domainbinding.util.BackticksRemover +import org.grails.orm.hibernate.cfg.domainbinding.util.ColumnNameForPropertyAndPathFetcher +import org.grails.orm.hibernate.cfg.domainbinding.util.DefaultColumnNameFetcher class EnumTypeBinderSpec extends HibernateGormDatastoreSpec { @@ -166,22 +164,25 @@ class EnumTypeBinderSpec extends HibernateGormDatastoreSpec { enum Status01 { AVAILABLE, OUT_OF_STOCK } -@Entity class Person01 { Long id; Status01 status } +@Entity class Person01 { + Long id; Status01 status + static constraints = { status nullable: false } +} @Entity class Person02 { Long id; Status01 status - static mapping = { status enumType: "string", nullable: true } + static mapping = { status enumType: "string" } } @Entity class Person03 { Long id; Status01 status - static mapping = { status enumType: "ordinal", nullable: true } + static mapping = { status enumType: "ordinal" } } @Entity class Person04 { Long id; Status01 status - static mapping = { status enumType: "identity" } + static mapping = { status enumType: "identity", nullable: false } } @Entity class Person05 { Long id; Status01 status - static mapping = { status type: UserTypeEnumType } + static mapping = { status type: UserTypeEnumType, nullable: false } } @Entity class PersonWithCollection { Long id diff --git a/grails-data-hibernate7/dbmigration/src/test-cli/groovy/org/apache/grails/data/hibernate7/dbmigration/cli/ApplicationContextDatabaseMigrationCommandSpec.groovy b/grails-data-hibernate7/dbmigration/src/test-cli/groovy/org/apache/grails/data/hibernate7/dbmigration/cli/ApplicationContextDatabaseMigrationCommandSpec.groovy index f8e2755e786..ea279b71ebb 100644 --- a/grails-data-hibernate7/dbmigration/src/test-cli/groovy/org/apache/grails/data/hibernate7/dbmigration/cli/ApplicationContextDatabaseMigrationCommandSpec.groovy +++ b/grails-data-hibernate7/dbmigration/src/test-cli/groovy/org/apache/grails/data/hibernate7/dbmigration/cli/ApplicationContextDatabaseMigrationCommandSpec.groovy @@ -121,6 +121,7 @@ class Book { Author author static belongsTo = [author: Author] static constraints = { + title nullable: false author nullable: false } } @@ -129,4 +130,7 @@ class Book { class Author { String name static hasMany = [books: Book] + static constraints = { + name nullable: false + } } diff --git a/grails-data-mongodb/core/src/test/groovy/org/grails/datastore/mapping/mongo/config/MongoMappingContextSpec.groovy b/grails-data-mongodb/core/src/test/groovy/org/grails/datastore/mapping/mongo/config/MongoMappingContextSpec.groovy new file mode 100644 index 00000000000..a701635a4fb --- /dev/null +++ b/grails-data-mongodb/core/src/test/groovy/org/grails/datastore/mapping/mongo/config/MongoMappingContextSpec.groovy @@ -0,0 +1,89 @@ +/* + * 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 + * + * https://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.grails.datastore.mapping.mongo.config + +import grails.gorm.annotation.Entity +import org.grails.datastore.mapping.mongo.connections.MongoConnectionSourceSettings +import spock.lang.Specification + +class MongoMappingContextSpec extends Specification { + + void "properties are nullable by default"() { + when: + def entity = new MongoMappingContext(new MongoConnectionSourceSettings()) + .addPersistentEntity(MongoEntityWithName) + + then: + entity.getPropertyByName('name').mapping.mappedForm.nullable + } + + void "explicit constraints preserve the default nullable mapping"() { + when: + def entity = new MongoMappingContext(new MongoConnectionSourceSettings()) + .addPersistentEntity(ConstrainedMongoEntity) + + then: + entity.getPropertyByName('name').mapping.mappedForm.nullable + } + + void "wildcard mappings preserve the default nullable mapping"() { + when: + def entity = new MongoMappingContext(new MongoConnectionSourceSettings()) + .addPersistentEntity(WildcardMongoEntity) + + then: + entity.getPropertyByName('name').mapping.mappedForm.nullable + } + + void "default nullable can be disabled"() { + given: + def settings = new MongoConnectionSourceSettings() + settings.default.nullable = false + + when: + def entity = new MongoMappingContext(settings) + .addPersistentEntity(MongoEntityWithName) + + then: + !entity.getPropertyByName('name').mapping.mappedForm.nullable + } +} + +@Entity +class MongoEntityWithName { + String name +} + +@Entity +class ConstrainedMongoEntity { + String name + + static constraints = { + name maxSize: 100 + } +} + +@Entity +class WildcardMongoEntity { + String name + + static mapping = { + '*' cache: true + } +} diff --git a/grails-data-neo4j/grails-datastore-gorm-neo4j/src/test/groovy/org/grails/datastore/gorm/neo4j/Neo4jMappingContextSpec.groovy b/grails-data-neo4j/grails-datastore-gorm-neo4j/src/test/groovy/org/grails/datastore/gorm/neo4j/Neo4jMappingContextSpec.groovy new file mode 100644 index 00000000000..435cb048a75 --- /dev/null +++ b/grails-data-neo4j/grails-datastore-gorm-neo4j/src/test/groovy/org/grails/datastore/gorm/neo4j/Neo4jMappingContextSpec.groovy @@ -0,0 +1,71 @@ +/* + * 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 + * + * https://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.grails.datastore.gorm.neo4j + +import grails.gorm.annotation.Entity +import org.grails.datastore.gorm.neo4j.connections.Neo4jConnectionSourceSettings +import spock.lang.Specification + +class Neo4jMappingContextSpec extends Specification { + + void "properties are nullable by default"() { + when: + def entity = new Neo4jMappingContext(new Neo4jConnectionSourceSettings()) + .addPersistentEntity(Neo4jEntityWithName) + + then: + entity.getPropertyByName('name').mapping.mappedForm.nullable + } + + void "explicit constraints preserve the default nullable mapping"() { + when: + def entity = new Neo4jMappingContext(new Neo4jConnectionSourceSettings()) + .addPersistentEntity(ConstrainedNeo4jEntity) + + then: + entity.getPropertyByName('name').mapping.mappedForm.nullable + } + + void "default nullable can be disabled"() { + given: + def settings = new Neo4jConnectionSourceSettings() + settings.default.nullable = false + + when: + def entity = new Neo4jMappingContext(settings) + .addPersistentEntity(Neo4jEntityWithName) + + then: + !entity.getPropertyByName('name').mapping.mappedForm.nullable + } +} + +@Entity +class Neo4jEntityWithName { + String name +} + +@Entity +class ConstrainedNeo4jEntity { + String name + + static constraints = { + name maxSize: 100 + } +} diff --git a/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/AbstractGormMappingFactory.java b/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/AbstractGormMappingFactory.java index d4029eefa60..b34e4dac3a7 100644 --- a/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/AbstractGormMappingFactory.java +++ b/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/AbstractGormMappingFactory.java @@ -42,8 +42,6 @@ /** * Abstract GORM implementation that uses the GORM MappingConfigurationBuilder to configure entity mappings. - * - * @author Graeme Rocher */ @SuppressWarnings({"rawtypes", "unchecked"}) public abstract class AbstractGormMappingFactory extends MappingFactory { @@ -54,6 +52,7 @@ public abstract class AbstractGormMappingFactory properties = entityToPropertyMap.get(mpp.getOwner()); if (properties != null && properties.containsKey(mpp.getName())) { - return properties.get(mpp.getName()); + T property = properties.get(mpp.getName()); + if (notIdentityOrVersion(mpp) && !property.isNullableConfigured()) { + property.setNullable(defaultNullable); + } + return property; } else if (properties != null) { - Property property = properties.get(IDENTITY_PROPERTY); + T property = properties.get(IDENTITY_PROPERTY); if (property != null && mpp.getName().equals(property.getName())) { - return (T) property; + return property; } } T defaultMapping = properties != null ? properties.get("*") : null; if (defaultMapping != null) { try { - return (T) defaultMapping.clone(); + T property = (T) defaultMapping.clone(); + if (notIdentityOrVersion(mpp) && !property.isNullableConfigured()) { + property.setNullable(defaultNullable); + } + return property; } catch (CloneNotSupportedException e) { - return BeanUtils.instantiateClass(getPropertyMappedFormType()); + T property = BeanUtils.instantiateClass(getPropertyMappedFormType()); + if (notIdentityOrVersion(mpp)) { + property.setNullable(defaultNullable); + } + return property; } } else { - return BeanUtils.instantiateClass(getPropertyMappedFormType()); + T property = BeanUtils.instantiateClass(getPropertyMappedFormType()); + if (notIdentityOrVersion(mpp)) { + property.setNullable(defaultNullable); + } + return property; } } + + private static boolean notIdentityOrVersion(PersistentProperty property) { + return !GormProperties.IDENTITY.equals(property.getName()) && + !GormProperties.VERSION.equals(property.getName()); + } } diff --git a/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/Property.groovy b/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/Property.groovy index 3f5bbde9dec..2b3383e6bda 100644 --- a/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/Property.groovy +++ b/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/Property.groovy @@ -34,7 +34,6 @@ import org.springframework.validation.DataBinder /** * Base class for classes returned from {@link org.grails.datastore.mapping.model.PropertyMapping#getMappedForm()} * - * @author Graeme Rocher * @since 1.0 */ @CompileStatic @@ -49,6 +48,7 @@ class Property implements Cloneable { * @return Whether the property is nullable */ boolean nullable = false + private boolean nullableConfigured = false /** * @return Whether this property is a database reference such as a foreign key @@ -169,6 +169,15 @@ class Property implements Cloneable { return lazy == Boolean.TRUE } + void setNullable(boolean nullable) { + this.nullable = nullable + this.nullableConfigured = true + } + + boolean isNullableConfigured() { + return nullableConfigured + } + void setLazy(Boolean lazy) { this.lazy = lazy } diff --git a/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/model/AbstractMappingContext.java b/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/model/AbstractMappingContext.java index 8afab3ed450..e8e139b619f 100644 --- a/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/model/AbstractMappingContext.java +++ b/grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/model/AbstractMappingContext.java @@ -109,6 +109,7 @@ protected void initialize(ConnectionSourceSettings settings) { AbstractGormMappingFactory gormMappingFactory = (AbstractGormMappingFactory) mappingFactory; gormMappingFactory.setDefaultConstraints(settings.getDefault().getConstraints()); gormMappingFactory.setDefaultMapping(settings.getDefault().getMapping()); + gormMappingFactory.setDefaultNullable(settings.getDefault().isNullable()); } } diff --git a/grails-datastore-core/src/test/groovy/org/grails/datastore/mapping/document/config/DocumentMappingContextSpec.groovy b/grails-datastore-core/src/test/groovy/org/grails/datastore/mapping/document/config/DocumentMappingContextSpec.groovy new file mode 100644 index 00000000000..35dda07cafa --- /dev/null +++ b/grails-datastore-core/src/test/groovy/org/grails/datastore/mapping/document/config/DocumentMappingContextSpec.groovy @@ -0,0 +1,89 @@ +/* + * 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 + * + * https://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.grails.datastore.mapping.document.config + +import grails.gorm.annotation.Entity +import org.grails.datastore.mapping.core.connections.ConnectionSourceSettings +import spock.lang.Specification + +class DocumentMappingContextSpec extends Specification { + + void "properties are nullable by default"() { + when: + def entity = new DocumentMappingContext('default', new ConnectionSourceSettings()) + .addPersistentEntity(DocumentEntityWithName) + + then: + entity.getPropertyByName('name').mapping.mappedForm.nullable + } + + void "explicit constraints preserve the default nullable mapping"() { + when: + def entity = new DocumentMappingContext('default', new ConnectionSourceSettings()) + .addPersistentEntity(ConstrainedDocumentEntity) + + then: + entity.getPropertyByName('name').mapping.mappedForm.nullable + } + + void "wildcard mappings preserve the default nullable mapping"() { + when: + def entity = new DocumentMappingContext('default', new ConnectionSourceSettings()) + .addPersistentEntity(WildcardDocumentEntity) + + then: + entity.getPropertyByName('name').mapping.mappedForm.nullable + } + + void "default nullable can be disabled"() { + given: + def settings = new ConnectionSourceSettings() + settings.default.nullable = false + + when: + def entity = new DocumentMappingContext('default', settings) + .addPersistentEntity(DocumentEntityWithName) + + then: + !entity.getPropertyByName('name').mapping.mappedForm.nullable + } +} + +@Entity +class DocumentEntityWithName { + String name +} + +@Entity +class ConstrainedDocumentEntity { + String name + + static constraints = { + name maxSize: 100 + } +} + +@Entity +class WildcardDocumentEntity { + String name + + static mapping = { + '*' cache: true + } +} diff --git a/grails-datastore-core/src/test/groovy/org/grails/datastore/mapping/keyvalue/mapping/KeyValueMappingFactoryTests.groovy b/grails-datastore-core/src/test/groovy/org/grails/datastore/mapping/keyvalue/mapping/KeyValueMappingFactoryTests.groovy index 681898b1b9d..01908f6c375 100644 --- a/grails-datastore-core/src/test/groovy/org/grails/datastore/mapping/keyvalue/mapping/KeyValueMappingFactoryTests.groovy +++ b/grails-datastore-core/src/test/groovy/org/grails/datastore/mapping/keyvalue/mapping/KeyValueMappingFactoryTests.groovy @@ -22,6 +22,7 @@ import org.grails.datastore.mapping.keyvalue.mapping.config.Family import org.grails.datastore.mapping.keyvalue.mapping.config.KeyValue import org.grails.datastore.mapping.keyvalue.mapping.config.KeyValueMappingContext import org.grails.datastore.mapping.keyvalue.mapping.config.KeyValuePersistentEntity +import org.grails.datastore.mapping.core.connections.ConnectionSourceSettings import org.grails.datastore.mapping.model.PersistentProperty import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @@ -67,13 +68,40 @@ class KeyValueMappingFactoryTests { PersistentProperty prop = entity.getPropertyByName('nonFormulaProperty') assert !prop.mapping.mappedForm.derived - assert !prop.mapping.mappedForm.nullable + assert prop.mapping.mappedForm.nullable // Formula properties should be flagged as derived prop = entity.getPropertyByName('formulaProperty') assert prop.mapping.mappedForm.derived } + @Test + void testExplicitConstraintsPreserveDefaultNullable() { + def entity = new KeyValueMappingContext('myspace') + .addPersistentEntity(ConstrainedTestEntity) + + assert entity.getPropertyByName('name').mapping.mappedForm.nullable + } + + @Test + void testWildcardMappingPreservesDefaultNullable() { + def entity = new KeyValueMappingContext('myspace') + .addPersistentEntity(WildcardTestEntity) + + assert entity.getPropertyByName('name').mapping.mappedForm.nullable + } + + @Test + void testDefaultNullableCanBeDisabled() { + def settings = new ConnectionSourceSettings() + settings.default.nullable = false + + def entity = new KeyValueMappingContext('myspace', settings) + .addPersistentEntity(NullableTestEntity) + + assert !entity.getPropertyByName('name').mapping.mappedForm.nullable + } + @Test void testParentEntity() { KeyValuePersistentEntity entity = context.getPersistentEntity(TestEntity.name) @@ -99,4 +127,27 @@ class KeyValueMappingFactoryTests { formulaProperty(formula: 'foo(bar)') } } + + class ConstrainedTestEntity { + Long id + String name + + static constraints = { + name maxSize: 100 + } + } + + class NullableTestEntity { + Long id + String name + } + + class WildcardTestEntity { + Long id + String name + + static mapping = { + '*' cache: true + } + } } diff --git a/grails-doc/src/en/guide/upgrading/upgrading80x.adoc b/grails-doc/src/en/guide/upgrading/upgrading80x.adoc index bd34bb094ab..4556cab7f90 100644 --- a/grails-doc/src/en/guide/upgrading/upgrading80x.adoc +++ b/grails-doc/src/en/guide/upgrading/upgrading80x.adoc @@ -1333,9 +1333,13 @@ class Book { } ---- -This is a *validation-layer* change only. -Column/DDL nullability is governed separately by the mapping layer and is unaffected. -Command objects are also unaffected: `Validateable` command-object fields remain required by default. +This change also affects persistence mapping. Columns for unconstrained properties are +nullable by default, so schema creation and schema migration can produce nullable columns +where Grails 7 produced `NOT NULL` columns. Review existing schemas and declare +`nullable: false` explicitly for properties that must remain required before running +schema migration or diff tools. + +Command objects are unaffected: Validateable command-object properties remain non-nullable by default, except for collections and maps, which remain nullable by default. To restore the legacy required-by-default behaviour for an entire application, set the new `grails.gorm.default.nullable` property to `false`: diff --git a/grails-test-examples/graphql/grails-test-app/grails-app/domain/grails/test/app/Address.groovy b/grails-test-examples/graphql/grails-test-app/grails-app/domain/grails/test/app/Address.groovy index 98c769a5024..b5c9ee73b00 100644 --- a/grails-test-examples/graphql/grails-test-app/grails-app/domain/grails/test/app/Address.groovy +++ b/grails-test-examples/graphql/grails-test-app/grails-app/domain/grails/test/app/Address.groovy @@ -26,5 +26,8 @@ class Address { Integer zip static constraints = { + city nullable: false + state nullable: false + zip nullable: false } } diff --git a/grails-test-examples/graphql/grails-test-app/grails-app/domain/grails/test/app/User.groovy b/grails-test-examples/graphql/grails-test-app/grails-app/domain/grails/test/app/User.groovy index 154fa844faf..375049925bd 100644 --- a/grails-test-examples/graphql/grails-test-app/grails-app/domain/grails/test/app/User.groovy +++ b/grails-test-examples/graphql/grails-test-app/grails-app/domain/grails/test/app/User.groovy @@ -32,7 +32,8 @@ class User { Address address //embedded domain class static constraints = { - manager nullable: true + profile nullable: false + address nullable: false } static embedded = ['address', 'profile'] diff --git a/grails-test-examples/graphql/grails-test-app/grails-app/domain/grails/test/app/UserRole.groovy b/grails-test-examples/graphql/grails-test-app/grails-app/domain/grails/test/app/UserRole.groovy index a601f10a204..ec4c3799454 100644 --- a/grails-test-examples/graphql/grails-test-app/grails-app/domain/grails/test/app/UserRole.groovy +++ b/grails-test-examples/graphql/grails-test-app/grails-app/domain/grails/test/app/UserRole.groovy @@ -65,6 +65,8 @@ class UserRole implements Serializable { static constraints = { + user nullable: false + role nullable: false role validator: { Role r, UserRole ur -> if (ur.user?.id) { UserRole.withNewSession { diff --git a/grails-test-examples/graphql/grails-test-app/src/main/groovy/grails/test/app/pogo/Profile.groovy b/grails-test-examples/graphql/grails-test-app/src/main/groovy/grails/test/app/pogo/Profile.groovy index 670472acdb3..5c11b8963fc 100644 --- a/grails-test-examples/graphql/grails-test-app/src/main/groovy/grails/test/app/pogo/Profile.groovy +++ b/grails-test-examples/graphql/grails-test-app/src/main/groovy/grails/test/app/pogo/Profile.groovy @@ -28,4 +28,10 @@ class Profile { String firstName String lastName + static constraints = { + email nullable: false + firstName nullable: false + lastName nullable: false + } + } diff --git a/grails-test-examples/hibernate5/grails-hibernate/grails-app/domain/functional/tests/Organization.groovy b/grails-test-examples/hibernate5/grails-hibernate/grails-app/domain/functional/tests/Organization.groovy new file mode 100644 index 00000000000..a7776a4d52e --- /dev/null +++ b/grails-test-examples/hibernate5/grails-hibernate/grails-app/domain/functional/tests/Organization.groovy @@ -0,0 +1,24 @@ +/* + * 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 + * + * https://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 functional.tests + +class Organization { + String name +} diff --git a/grails-test-examples/hibernate5/grails-hibernate/src/integration-test/groovy/functional/tests/OrganizationValidationSpec.groovy b/grails-test-examples/hibernate5/grails-hibernate/src/integration-test/groovy/functional/tests/OrganizationValidationSpec.groovy new file mode 100644 index 00000000000..59f5368441c --- /dev/null +++ b/grails-test-examples/hibernate5/grails-hibernate/src/integration-test/groovy/functional/tests/OrganizationValidationSpec.groovy @@ -0,0 +1,45 @@ +/* + * 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 + * + * https://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 functional.tests + +import spock.lang.Specification + +import grails.gorm.transactions.Rollback +import grails.testing.mixin.integration.Integration + +@Integration +class OrganizationValidationSpec extends Specification { + + @Rollback + void "saving an organization without its name succeeds as nullable is true by default"() { + given: + def organization = new Organization() + + expect: + organization.validate() + !organization.hasErrors() + + when: + def savedOrganization = organization.save(flush: true) + + then: + savedOrganization == organization + Organization.count() == 1 + } +} diff --git a/grails-test-suite-persistence/src/test/groovy/grails/web/databinding/GrailsWebDataBinderSpec.groovy b/grails-test-suite-persistence/src/test/groovy/grails/web/databinding/GrailsWebDataBinderSpec.groovy index c716a2fd3ee..43a1abcad97 100644 --- a/grails-test-suite-persistence/src/test/groovy/grails/web/databinding/GrailsWebDataBinderSpec.groovy +++ b/grails-test-suite-persistence/src/test/groovy/grails/web/databinding/GrailsWebDataBinderSpec.groovy @@ -21,11 +21,18 @@ package grails.web.databinding import groovy.transform.CompileStatic import groovy.transform.Sortable +import spock.lang.Issue +import spock.lang.Specification +import spock.lang.Unroll + +import org.springframework.context.support.StaticMessageSource + import grails.config.Settings import grails.databinding.BindUsing import grails.databinding.BindingFormat import grails.databinding.DataBindingSource import grails.databinding.SimpleMapDataBindingSource +import grails.databinding.converters.ValueConverter import grails.databinding.errors.BindingError import grails.databinding.events.DataBindingListenerAdapter import grails.persistence.Entity @@ -33,10 +40,6 @@ import grails.testing.gorm.DataTest import grails.validation.DeferredBindingActions import grails.validation.Validateable import org.grails.config.PropertySourcesConfig -import org.springframework.context.support.StaticMessageSource -import spock.lang.Issue -import spock.lang.Specification -import spock.lang.Unroll class GrailsWebDataBinderSpec extends Specification implements DataTest { @@ -46,9 +49,9 @@ class GrailsWebDataBinderSpec extends Specification implements DataTest { void setupSpec() { mockDomains( - AssociationBindingAuthor, AssociationBindingBook, AssociationBindingPage, Author, Child, - CollectionContainer, DataBindingBook, Fidget, Foo, GeneratedBindingChild, GeneratedBindingParent, - Parent, Publication, Publisher, Team, Widget + AssociationBindingAuthor, AssociationBindingBook, AssociationBindingPage, Author, BinderNullabilityEntity, + Child, CollectionContainer, DataBindingBook, Fidget, Foo, GeneratedBindingChild, GeneratedBindingParent, + Parent, Publication, Publisher, Team, Widget ) } @@ -58,6 +61,7 @@ class GrailsWebDataBinderSpec extends Specification implements DataTest { void cleanup() { Locale.setDefault(defaultLocale) + binder.convertEmptyStringsToNull = true grailsApplication.config.setAt(Settings.DATABINDING_DENY_BY_DEFAULT, null) DataBindingUtils.clearBindingCaches() GrailsWebDataBinder.resetWarnedBindingShapes() @@ -1355,6 +1359,38 @@ class GrailsWebDataBinderSpec extends Specification implements DataTest { afterBindingArgs[0]['propertyName'] == 'someNumber' } + void 'blank binding errors respect domain property nullability'() { + + given: + binder.convertEmptyStringsToNull = false + binder.registerConverter(new ValueConverter() { + boolean canConvert(Object value) { + value instanceof String + } + + Object convert(Object value) { + throw new IllegalArgumentException('Blank status') + } + + Class getTargetType() { + BinderNullabilityStatus + } + }) + def entity = new BinderNullabilityEntity() + + when: + binder.bind(entity, new SimpleMapDataBindingSource( + optionalStatus: '', + requiredStatus: '' + )) + + then: + entity.hasErrors() + entity.errors.errorCount == 1 + entity.errors.getFieldError('requiredStatus').code == 'typeMismatch' + entity.errors.getFieldError('optionalStatus') == null + } + void 'Test binding a List'() { given: @@ -1939,7 +1975,6 @@ class Team { members: Author, states: String ] - } @Entity @@ -1973,7 +2008,6 @@ class Publisher { class SomeNonDomainClass { Publication publication List listOfLong - } @Entity @@ -1982,9 +2016,12 @@ class Publication { String title Author author + static constraints = { + publisher nullable: true + } + @SuppressWarnings('unused') static belongsTo = [publisher: Publisher] - } @Entity @@ -2065,13 +2102,11 @@ class ParentWidget implements Validateable { @Entity class Fidget extends ParentWidget { String name - } @Entity class Parent { Child child - } @Entity @@ -2160,7 +2195,6 @@ class DataBindingBook { topics: String, importantPageNumbers: Integer ] - } @Entity @@ -2179,12 +2213,10 @@ class CollectionContainer { collectionOfWidgets: Widget, sortedSetOfWidgets: Widget ] - } class DocumentHolder { List objectIds - } class ObjectId { @@ -2194,7 +2226,6 @@ class ObjectId { ObjectId(String str) { value = str } - } class PrimitiveContainer implements Validateable { @@ -2206,19 +2237,30 @@ class PrimitiveContainer implements Validateable { long someLong float someFloat double someDouble - } @SuppressWarnings('unused') class SomeValidateableClass implements Validateable { Integer someNumber +} + +@Entity +class BinderNullabilityEntity { + BinderNullabilityStatus optionalStatus + BinderNullabilityStatus requiredStatus + static constraints = { + requiredStatus nullable: false + } +} + +enum BinderNullabilityStatus { + ACTIVE } @Entity class AssociationBindingPage { Integer number - } @Entity @@ -2230,7 +2272,6 @@ class AssociationBindingBook { static belongsTo = [author: AssociationBindingAuthor] static hasMany = [pages: AssociationBindingPage] - } @Entity @@ -2241,7 +2282,6 @@ class AssociationBindingAuthor { List books static hasMany = [books: AssociationBindingBook] - } @Entity @@ -2299,17 +2339,14 @@ class Foo { class NonDomainClassWithMapProperty { String name Map albums - } class NonDomainClassWithSetOfDomainInstances { Set publishers - } class Album { String title - } @SuppressWarnings('unused') @@ -2324,11 +2361,9 @@ class AlbumHolder { Album getAlbum() { return new Album(title: album) } - } @SuppressWarnings('unused') class ListCommand implements Validateable { List myLongList - } diff --git a/grails-web-databinding/src/main/groovy/grails/web/databinding/GrailsWebDataBinder.groovy b/grails-web-databinding/src/main/groovy/grails/web/databinding/GrailsWebDataBinder.groovy index e559583b342..7110d200fc2 100644 --- a/grails-web-databinding/src/main/groovy/grails/web/databinding/GrailsWebDataBinder.groovy +++ b/grails-web-databinding/src/main/groovy/grails/web/databinding/GrailsWebDataBinder.groovy @@ -901,7 +901,7 @@ class GrailsWebDataBinder extends SimpleDataBinder { otherSide = ((Association) property).inverseSide } } - if (otherSide != null && List.isAssignableFrom(otherSide.getType()) && !property.isNullable()) { + if (otherSide != null && List.isAssignableFrom(otherSide.getType())) { DeferredBindingActions.addBindingAction(new Runnable() { void run() { if (obj[propName] != null && otherSide instanceof OneToMany) {