You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@usergrid.apache.org by Malaka Mahanama <ma...@gmail.com> on 2014/04/05 23:45:06 UTC
USERGRID-22 bug fix for review
Hi All,
This is to get a bug fix reviewed and to get feedback. Do go easy on me as
this is my first commit to user-grid...
*Pull request:* https://github.com/usergrid/usergrid/pull/99
*Description (as per the bug):*
"if I try to get a resource by name that doesn't exist
https://api.usergrid.com/mdobs/sandbox/books/Scooter
I get a service_resource_not_found
but when I throw spaces into the url
https://api.usergrid.com/mdobs/sandbox/books/Scooter%20guide
I get a null_pointer error instead"
*Fix:*
If the resource name is invalid we get the service_not_found, this is
because the query 'Identifiers' in this case are added but are not valid
ones.
However if the url has the %20 inbetween like in the example, then the
query identifier becomes null. Later methods are being invoked on these
null identifiers that cause the null pointer.
The fix I suggest is a generic one, we do a check if the identifier is null
and handle the output accordingly in 3 places. This could stop this error
from happening for other reasons similar to %20.
1)Query.java - containsNameOrEmailIdentifiersOnly()
public boolean containsNameOrEmailIdentifiersOnly() {
if ( hasQueryPredicates() )
{ return false; }
if ( ( identifiers == null ) || identifiers.isEmpty() ) { return false; }
for ( Identifier identifier : identifiers ) {
//this IF check was added as part of stopping the issue, without it
identifier.isEmail() below would cause a null pointer.
if (identifier == null)
{ return false; }
if ( !identifier.isEmail() && !identifier.isName() )
{ return false; }
}
return true;
}
2) Query.java - containsUuidIdentifiersOnly()
boolean containsUuidIdentifiersOnly() {
if ( hasQueryPredicates() ) { return false; }
if ( ( identifiers == null ) || identifiers.isEmpty() ) { return false; }
for ( Identifier identifier : identifiers ) {
//this IF check was added as part of stopping the issue, without it
identifier.isUUID() below would cause a null pointer.
if (identifier == null){ return true; }
if ( !identifier.isUUID() ) { return false; }
}
return true;
}"
3)Query.java - getSingleUuidIdentifier()
" @JsonIgnore
public UUID getSingleUuidIdentifier() {
if ( !containsSingleUuidIdentifier() )
{ return null; }
//this IF check was added as part of stopping the issue, without it
'identifiers.get( 0 ).getUUID()' below would cause a null pointer.
if(identifiers.get( 0 )==null)
{ return null; }
return ( identifiers.get( 0 ).getUUID() );
}
The above 3 places were changed as part of the fix. I think these are basic
checks and risk of regression is very low.
Looking at a few other bugs as well, hoping to resolve them soon.
Appreciate comments and feedback for this one.
Thanks and Regards,
Malaka Mahanama
Virtusa ~ Sri-Lanka
Re: USERGRID-22 bug fix for review
Posted by Rod Simpson <ro...@rodsimpson.com>.
Awesome, Malaka!! Thank you for submitting!!!
Rod
--
Rod Simpson
@rockerston
rodsimpson.com
On April 5, 2014 at 3:45:34 PM, Malaka Mahanama (malaka.open.source@gmail.com) wrote:
Hi All,
This is to get a bug fix reviewed and to get feedback. Do go easy on me as
this is my first commit to user-grid...
*Pull request:* https://github.com/usergrid/usergrid/pull/99
*Description (as per the bug):*
"if I try to get a resource by name that doesn't exist
https://api.usergrid.com/mdobs/sandbox/books/Scooter
I get a service_resource_not_found
but when I throw spaces into the url
https://api.usergrid.com/mdobs/sandbox/books/Scooter%20guide
I get a null_pointer error instead"
*Fix:*
If the resource name is invalid we get the service_not_found, this is
because the query 'Identifiers' in this case are added but are not valid
ones.
However if the url has the %20 inbetween like in the example, then the
query identifier becomes null. Later methods are being invoked on these
null identifiers that cause the null pointer.
The fix I suggest is a generic one, we do a check if the identifier is null
and handle the output accordingly in 3 places. This could stop this error
from happening for other reasons similar to %20.
1)Query.java - containsNameOrEmailIdentifiersOnly()
public boolean containsNameOrEmailIdentifiersOnly() {
if ( hasQueryPredicates() )
{ return false; }
if ( ( identifiers == null ) || identifiers.isEmpty() ) { return false; }
for ( Identifier identifier : identifiers ) {
//this IF check was added as part of stopping the issue, without it
identifier.isEmail() below would cause a null pointer.
if (identifier == null)
{ return false; }
if ( !identifier.isEmail() && !identifier.isName() )
{ return false; }
}
return true;
}
2) Query.java - containsUuidIdentifiersOnly()
boolean containsUuidIdentifiersOnly() {
if ( hasQueryPredicates() ) { return false; }
if ( ( identifiers == null ) || identifiers.isEmpty() ) { return false; }
for ( Identifier identifier : identifiers ) {
//this IF check was added as part of stopping the issue, without it
identifier.isUUID() below would cause a null pointer.
if (identifier == null){ return true; }
if ( !identifier.isUUID() ) { return false; }
}
return true;
}"
3)Query.java - getSingleUuidIdentifier()
" @JsonIgnore
public UUID getSingleUuidIdentifier() {
if ( !containsSingleUuidIdentifier() )
{ return null; }
//this IF check was added as part of stopping the issue, without it
'identifiers.get( 0 ).getUUID()' below would cause a null pointer.
if(identifiers.get( 0 )==null)
{ return null; }
return ( identifiers.get( 0 ).getUUID() );
}
The above 3 places were changed as part of the fix. I think these are basic
checks and risk of regression is very low.
Looking at a few other bugs as well, hoping to resolve them soon.
Appreciate comments and feedback for this one.
Thanks and Regards,
Malaka Mahanama
Virtusa ~ Sri-Lanka