Skip to content

add entity-type to message when no UUID is found for a DB ID #5163

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
nvazquez merged 5 commits into from
Jul 21, 2021
Merged

add entity-type to message when no UUID is found for a DB ID #5163

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
96b5e99
add entity-type to message
Jun 28, 2021
3464dcc
Update server/src/main/java/com/cloud/uuididentity/UUIDManagerImpl.java
DaanHoogland Jun 30, 2021
38c9a9c
update for review comments
DaanHoogland Jul 13, 2021
63aa3cd
syntax
Jul 16, 2021
03246f6
Avoid NPE when entity type is null, and throw appropriate exception
sureshanaparti Jul 19, 2021
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
10 changes: 7 additions & 3 deletions server/src/main/java/com/cloud/uuididentity/UUIDManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,16 @@ public <T> String getUuid(Class<T> entityType, Long customId) {
if (customId == null) {
return null;
}
if (entityType == null ) {
throw new InvalidParameterValueException("Unknown entity type");
}

Identity identity = (Identity) this._entityMgr.findById(entityType, customId);
Copy link
Copy Markdown
Contributor

@sureshanaparti sureshanaparti Jul 1, 2021

Choose a reason for hiding this comment

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

@DaanHoogland when entityType is null, I think, it fails here (with NPE below stack), can you please check that.

Copy link
Copy Markdown
Contributor Author

@DaanHoogland DaanHoogland Jul 2, 2021

Choose a reason for hiding this comment

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

Yhe NPE would be thrown in the call to findById() @sureshanaparti @rhtyd

    @Override
    public <T, K extends Serializable> T findById(Class<T> entityType, K id) {
        GenericDao<? extends T, K> dao = (GenericDao<? extends T, K>)GenericDaoBase.getDao(entityType);
        return dao.findById(id);
    }

I'm fine with applying the extra check in this case but I think it'll be unreachable code.

Copy link
Copy Markdown
Contributor Author

@DaanHoogland DaanHoogland Jul 13, 2021

Choose a reason for hiding this comment

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

@sureshanaparti , as shown above, this NPE is going to happen out of scope of this change anyway, in these kinds of bits of code. What do you suggest to do?

Copy link
Copy Markdown
Contributor

@sureshanaparti sureshanaparti Jul 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@DaanHoogland my suggestion is if entityType is null, better throw exception as below

InvalidParameterValueException("Unknown entity type")

and when identity is null (here entityType is not null), better throw exception as below

InvalidParameterValueException(String.format("Unable to find UUID for id [%s] of type [%s]",
                                                                    customId, entityType.getSimpleName()));

Copy link
Copy Markdown
Contributor Author

@DaanHoogland DaanHoogland Jul 19, 2021

Choose a reason for hiding this comment

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

sounds good @sureshanaparti

if (identity == null) {
throw new InvalidParameterValueException("Unable to find UUID for id " + customId);
throw new InvalidParameterValueException(String.format("Unable to find UUID for id [%s] of type [%s]",
customId, entityType.getSimpleName()));

}
return identity.getUuid();

}

}