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.

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

Fix string to objectId mapping error when using query method. #4519

Closed

Conversation

grwang91
Copy link
Contributor

@grwang91 grwang91 commented Oct 4, 2023

  • You have read the Spring Data contribution guidelines.
  • You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.
  • You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

Fixed objectId conversion issue.
Merged common codes into method.

Resolves: #4490

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 4, 2023
@christophstrobl
Copy link
Member

@grwang91 thank you for the the PR!

@christophstrobl christophstrobl self-assigned this Oct 9, 2023
resultDbo.put(inKey, ids);
} else if (valueDbo.containsKey("$ne")) {
resultDbo.put("$ne", convertId(valueDbo.get("$ne"), getIdTypeForField(documentField)));
} else if (containsCompareOperator(valueDbo)) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious if we need to explicitly check on the presence of certain keys here. What might also work would be to check if the key is a keyword and then call convertIdField with the value.

private Object convertIdField(Field documentField, Object source) {

    // ... logic for $in,...

    for(Entry<String,Object> entry : valueDbo.entrySet()) {
        if(isKeyword(entry.getKey())) {
            resultDbo.put(entry.getKey(), convertIdField(documentField, entry.getValue()));
        } else {
            resultDbo.put(entry.getKey(), getMappedValue(documentField, entry.getValue()));
        }
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for suggestion. I will apply it!

@christophstrobl
Copy link
Member

I think I'd like to see the changes contained in #4517 go in first and then bring in additional logic for handling the id conversion from this PR.

@grwang91
Copy link
Contributor Author

grwang91 commented Oct 12, 2023

I think I'd like to see the changes contained in #4517 go in first and then bring in additional logic for handling the id conversion from this PR.

Do you mean applying this PR after merging #4517 to main branch?

@mp911de mp911de assigned mp911de and unassigned christophstrobl Feb 8, 2024
@christophstrobl christophstrobl self-assigned this Feb 8, 2024
@mp911de mp911de added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 12, 2024
@mp911de mp911de added this to the 4.3 M1 (2024.0.0) milestone Feb 12, 2024
mp911de pushed a commit that referenced this pull request Feb 12, 2024
@mp911de
Copy link
Member

mp911de commented Feb 12, 2024

Thank you for your contribution. That's merged and polished now.

@mp911de mp911de closed this Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

String to ObjectId mapping error when using query method
4 participants