Skip to content

Improve how identifiers are treated #84

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

Merged
stIncMale merged 6 commits into from
Apr 10, 2025
Merged

Improve how identifiers are treated #84

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4f9ef0e
Improve how identifiers are treated
stIncMale Apr 4, 2025
fb7bd7d
Replace incorrect "single quote" usage with "backtick"
stIncMale Apr 10, 2025
96b8c78
Leave mixed case only in `WithSpaceAndDotAndMixedCase`
stIncMale Apr 10, 2025
2be5580
Rename `InBackticks`/`InDoubleQuotes` to `StartingAndEndingWithBackti…
stIncMale Apr 10, 2025
2578ac7
Add tests for square brackets
stIncMale Apr 10, 2025
00e9496
Explicitly forbid field names with `.`, `$`
stIncMale Apr 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
250 changes: 250 additions & 0 deletions src/integrationTest/java/com/mongodb/hibernate/IdentifierIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,250 @@
/*
* Copyright 2025-present MongoDB, Inc.
*
* Licensed 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 com.mongodb.hibernate;

import static com.mongodb.hibernate.internal.MongoConstants.ID_FIELD_NAME;
import static org.assertj.core.api.Assertions.assertThat;

import com.mongodb.client.MongoCollection;
import com.mongodb.hibernate.junit.InjectMongoCollection;
import com.mongodb.hibernate.junit.MongoExtension;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.Id;
import jakarta.persistence.Table;
import org.bson.BsonDocument;
import org.bson.BsonInt32;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.hibernate.testing.orm.junit.SessionFactoryScopeAware;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;

@SessionFactory(exportSchema = false)
@DomainModel(
annotatedClasses = {
IdentifierIntegrationTests.WithSpaceAndDotMixedCase.class,
IdentifierIntegrationTests.InSingleQuotesMixedCase.class,
IdentifierIntegrationTests.StartingWithSingleQuote.class,
IdentifierIntegrationTests.EndingWithSingleQuote.class,
IdentifierIntegrationTests.InDoubleQuotesMixedCase.class,
IdentifierIntegrationTests.StartingWithDoubleQuote.class,
IdentifierIntegrationTests.EndingWithDoubleQuote.class
})
@ExtendWith(MongoExtension.class)
class IdentifierIntegrationTests implements SessionFactoryScopeAware {
@InjectMongoCollection(WithSpaceAndDotMixedCase.COLLECTION_NAME)
private static MongoCollection<BsonDocument> mongoCollectionWithSpaceAndDotMixedCase;

@InjectMongoCollection(InSingleQuotesMixedCase.ACTUAL_COLLECTION_NAME)
private static MongoCollection<BsonDocument> mongoCollectionInSingleQuotesMixedCase;

@InjectMongoCollection(StartingWithSingleQuote.COLLECTION_NAME)
private static MongoCollection<BsonDocument> mongoCollectionStartingWithSingleQuote;

@InjectMongoCollection(EndingWithSingleQuote.COLLECTION_NAME)
private static MongoCollection<BsonDocument> mongoCollectionEndingWithSingleQuote;

@InjectMongoCollection(InDoubleQuotesMixedCase.ACTUAL_COLLECTION_NAME)
private static MongoCollection<BsonDocument> mongoCollectionInDoubleQuotesMixedCase;

@InjectMongoCollection(StartingWithDoubleQuote.COLLECTION_NAME)
private static MongoCollection<BsonDocument> mongoCollectionStartingWithDoubleQuote;

@InjectMongoCollection(EndingWithDoubleQuote.COLLECTION_NAME)
private static MongoCollection<BsonDocument> mongoCollectionEndingWithDoubleQuote;

private SessionFactoryScope sessionFactoryScope;

@Test
void withSpaceAndDotMixedCase() {
var item = new WithSpaceAndDotMixedCase();
sessionFactoryScope.inTransaction(session -> session.persist(item));
assertThat(mongoCollectionWithSpaceAndDotMixedCase.find())
.containsExactly(new BsonDocument()
.append(ID_FIELD_NAME, new BsonInt32(item.id))
.append(WithSpaceAndDotMixedCase.FIELD_NAME, new BsonInt32(item.v)));
sessionFactoryScope.inTransaction(session -> session.get(WithSpaceAndDotMixedCase.class, item.id));
}

@Test
void inSingleQuotesMixedCase() {
var item = new InSingleQuotesMixedCase();
sessionFactoryScope.inTransaction(session -> session.persist(item));
assertThat(mongoCollectionInSingleQuotesMixedCase.find())
.containsExactly(new BsonDocument()
.append(ID_FIELD_NAME, new BsonInt32(item.id))
.append(InSingleQuotesMixedCase.ACTUAL_FIELD_NAME, new BsonInt32(item.v)));
sessionFactoryScope.inTransaction(session -> session.get(InSingleQuotesMixedCase.class, item.id));
}

@Test
void startingWithSingleQuote() {
var item = new StartingWithSingleQuote();
sessionFactoryScope.inTransaction(session -> session.persist(item));
assertThat(mongoCollectionStartingWithSingleQuote.find())
.containsExactly(new BsonDocument()
.append(ID_FIELD_NAME, new BsonInt32(item.id))
.append(StartingWithSingleQuote.FIELD_NAME, new BsonInt32(item.v)));
sessionFactoryScope.inTransaction(session -> session.get(StartingWithSingleQuote.class, item.id));
}

@Test
void endingWithSingleQuote() {
var item = new EndingWithSingleQuote();
sessionFactoryScope.inTransaction(session -> session.persist(item));
assertThat(mongoCollectionEndingWithSingleQuote.find())
.containsExactly(new BsonDocument()
.append(ID_FIELD_NAME, new BsonInt32(item.id))
.append(EndingWithSingleQuote.FIELD_NAME, new BsonInt32(item.v)));
sessionFactoryScope.inTransaction(session -> session.get(EndingWithSingleQuote.class, item.id));
}

@Test
void inDoubleQuotesMixedCase() {
var item = new InDoubleQuotesMixedCase();
sessionFactoryScope.inTransaction(session -> session.persist(item));
assertThat(mongoCollectionInDoubleQuotesMixedCase.find())
.containsExactly(new BsonDocument()
.append(ID_FIELD_NAME, new BsonInt32(item.id))
.append(InDoubleQuotesMixedCase.ACTUAL_FIELD_NAME, new BsonInt32(item.v)));
sessionFactoryScope.inTransaction(session -> session.get(InDoubleQuotesMixedCase.class, item.id));
}

@Test
void startingWithDoubleQuote() {
var item = new StartingWithDoubleQuote();
sessionFactoryScope.inTransaction(session -> session.persist(item));
assertThat(mongoCollectionStartingWithDoubleQuote.find())
.containsExactly(new BsonDocument()
.append(ID_FIELD_NAME, new BsonInt32(item.id))
.append(StartingWithDoubleQuote.FIELD_NAME, new BsonInt32(item.v)));
sessionFactoryScope.inTransaction(session -> session.get(StartingWithDoubleQuote.class, item.id));
}

@Test
void endingWithDoubleQuote() {
var item = new EndingWithDoubleQuote();
sessionFactoryScope.inTransaction(session -> session.persist(item));
assertThat(mongoCollectionEndingWithDoubleQuote.find())
.containsExactly(new BsonDocument()
.append(ID_FIELD_NAME, new BsonInt32(item.id))
.append(EndingWithDoubleQuote.FIELD_NAME, new BsonInt32(item.v)));
sessionFactoryScope.inTransaction(session -> session.get(EndingWithDoubleQuote.class, item.id));
Comment thread
stIncMale marked this conversation as resolved.
}

@Override
public void injectSessionFactoryScope(SessionFactoryScope sessionFactoryScope) {
this.sessionFactoryScope = sessionFactoryScope;
}

@Entity
@Table(name = WithSpaceAndDotMixedCase.COLLECTION_NAME)
static class WithSpaceAndDotMixedCase {
static final String COLLECTION_NAME = "collection with space and .dot Mixed Case";
static final String FIELD_NAME = "field with space Mixed Case";
Comment thread
NathanQingyangXu marked this conversation as resolved.
Outdated

@Id
int id;

@Column(name = WithSpaceAndDotMixedCase.FIELD_NAME)
int v;
}

@Entity
@Table(name = InSingleQuotesMixedCase.COLLECTION_NAME)
Comment thread
NathanQingyangXu marked this conversation as resolved.
Outdated
static class InSingleQuotesMixedCase {
Copy link
Copy Markdown
Contributor

@NathanQingyangXu NathanQingyangXu Apr 6, 2025

Choose a reason for hiding this comment

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

the MixedCase is a little bit unclear, though further code reading shows that the Mixed case simply means single quotes both at the beginning and at the end.

Optional. Avoid the confusing MixedCase here by using more intuitive verbiage like InSingleQuotesDelimiters or simply StartingAndEndingWithSingleQuote.

Copy link
Copy Markdown
Member Author

@stIncMale stIncMale Apr 10, 2025

Choose a reason for hiding this comment

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

though further code reading shows that the Mixed case simply means single quotes both at the beginning and at the end.

No, "Mixed Case" means what is usually does - text containing both uppercase and lowercase letters. Since it wasn't clear, I removed it from all but one place in 96b8c78.

static final String COLLECTION_NAME = "`collection in single quotes Mixed Case`";
static final String FIELD_NAME = "`field in single quotes Mixed Case`";
static final String ACTUAL_COLLECTION_NAME = "collection in single quotes Mixed Case";
static final String ACTUAL_FIELD_NAME = "field in single quotes Mixed Case";
Comment thread
NathanQingyangXu marked this conversation as resolved.
Outdated

@Id
int id;

@Column(name = InSingleQuotesMixedCase.FIELD_NAME)
int v;
}

@Entity
@Table(name = StartingWithSingleQuote.COLLECTION_NAME)
static class StartingWithSingleQuote {
static final String COLLECTION_NAME = "`collection starting with single quote";
static final String FIELD_NAME = "`field starting with single quote";

@Id
int id;

@Column(name = StartingWithSingleQuote.FIELD_NAME)
int v;
}

@Entity
@Table(name = EndingWithSingleQuote.COLLECTION_NAME)
static class EndingWithSingleQuote {
static final String COLLECTION_NAME = "collection ending with single quote`";
static final String FIELD_NAME = "field ending with single quote`";

@Id
int id;

@Column(name = EndingWithSingleQuote.FIELD_NAME)
int v;
}

@Entity
@Table(name = InDoubleQuotesMixedCase.COLLECTION_NAME)
static class InDoubleQuotesMixedCase {
static final String COLLECTION_NAME = "\"collection in double quotes Mixed Case\"";
static final String FIELD_NAME = "\"field in double quotes Mixed Case\"";
static final String ACTUAL_COLLECTION_NAME = "collection in double quotes Mixed Case";
static final String ACTUAL_FIELD_NAME = "field in double quotes Mixed Case";
Copy link
Copy Markdown
Member Author

@stIncMale stIncMale Apr 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Unfortunately, it seems impossible to prevent Hibernate ORM from stripping identifiers like \"collection in double quotes Mixed Case\" of their quotes. Apparently, it has logic somewhere that ignores any configuration/overrides related to quoting, and strips double and single quotes anyway.

Copy link
Copy Markdown
Member Author

@stIncMale stIncMale Apr 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

That place seems to be here https://github.com/hibernate/hibernate-orm/blob/0a4d551b32801ae15e99d2f8ac0750303ecda2b6/hibernate-core/src/main/java/org/hibernate/boot/model/naming/Identifier.java#L159-L173 - it always considers identifiers enclosed in a set of statically-defined characters as quoted.

Copy link
Copy Markdown
Contributor

@NathanQingyangXu NathanQingyangXu Apr 6, 2025

Choose a reason for hiding this comment

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

yeah, it seems so. If the Hibernate quoting at the start and end is hard-coded or not configurable, I think we could remove those MixedCases testing cases for we don't need to test Hibernate if its behaviour is working as expected.

Copy link
Copy Markdown
Member Author

@stIncMale stIncMale Apr 10, 2025

Choose a reason for hiding this comment

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

The MixedCases tests are for testing that Hibernate ORM does not change the characters in collection/field names with mixed case to upper-/lower-case characters. Currently there is only one test (withSpaceAndDotAndMixedCase) left checking that. Do you still think we should not test this?


@Id
int id;

@Column(name = InDoubleQuotesMixedCase.FIELD_NAME)
int v;
}

@Entity
@Table(name = StartingWithDoubleQuote.COLLECTION_NAME)
static class StartingWithDoubleQuote {
static final String COLLECTION_NAME = "\"collection starting with double quote";
static final String FIELD_NAME = "\"field starting with double quote";

@Id
int id;

@Column(name = StartingWithDoubleQuote.FIELD_NAME)
int v;
}

@Entity
@Table(name = EndingWithDoubleQuote.COLLECTION_NAME)
static class EndingWithDoubleQuote {
static final String COLLECTION_NAME = "collection ending with double quote\"";
static final String FIELD_NAME = "field ending with double quote\"";

@Id
int id;

@Column(name = EndingWithDoubleQuote.FIELD_NAME)
int v;
}
}
Comment thread
NathanQingyangXu marked this conversation as resolved.
6 changes: 6 additions & 0 deletions src/main/java/com/mongodb/hibernate/dialect/MongoDialect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.hibernate.engine.jdbc.dialect.spi.DialectResolutionInfo;
import org.hibernate.service.ServiceRegistry;
import org.hibernate.sql.ast.SqlAstTranslatorFactory;
import org.jspecify.annotations.Nullable;

/**
* A MongoDB {@link Dialect} for {@linkplain #getMinimumSupportedVersion() version 6.0 and above}. Must be used together
Expand Down Expand Up @@ -91,4 +92,9 @@ public void contribute(TypeContributions typeContributions, ServiceRegistry serv
typeContributions.contributeJavaType(ObjectIdJavaType.INSTANCE);
typeContributions.contributeJdbcType(ObjectIdJdbcType.INSTANCE);
}

@Override
public @Nullable String toQuotedIdentifier(@Nullable String name) {
return name;
Copy link
Copy Markdown
Member Author

@stIncMale stIncMale Apr 4, 2025

Choose a reason for hiding this comment

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

This single change makes all the new tests pass, and without it, they all fail. Nothing else related to quoting both in Dialect (including Dialect.buildIdentifierHelper) and in DatabaseMetaData had any observable effect on the tests added.

Copy link
Copy Markdown
Contributor

@NathanQingyangXu NathanQingyangXu Apr 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It is a great news! Thanks for your PR. With some minor changes it could be merged.

}
}