Update aws-lambda-java-core version and fix bug in the multiple skills usecase

prakshai commented 3 years ago

Update aws-lambda-java-core version with related code changes
Fix bug to ensure multiple skills usecase behaves as expected
If skillId in the ASK request doesn't match the one used when initializing the skill, let the fallthrough happen to the next skill.

Description

The change updates aws-lambda-java-core and applies code changes required to build successfully post the version upgrade.
We have a usecase where we chose to host multiple skills as part of the same application using this skills stream handler constructor: https://github.com/alexa/alexa-skills-kit-sdk-for-java/blob/2.0.x/ask-sdk-lambda-support/src/com/amazon/ask/SkillStreamHandler.java#L65. When we try to invoke the second skill within that handler we see an exception "com.amazon.ask.exception.UnhandledSkillException: Unhandled exception" in our logs. Upon debugging, the issue seems to be when running multiple skills, the second skill's (possibly any skills apart from the first skill in the list) handlers are ignored.
If skillId in the ASK request doesn't match the one used when initializing the skill, let the fallthrough happen to the next skill. Only fail will all skillIds in the list don't match the skillId in the ASK request. Fixing the workflow here by not throwing and exception but returning a null to be handled as appropriate.

Motivation and Context

When trying to build with this project as a dependency on Android platform the build fails with "The prefix "xsi" for attribute "xsi:schemaLocation" associated with an element type "project" is not bound". Root cause: without these declarations, the XML file cannot be validated correctly. Refer: https://github.com/aws/aws-lambda-java-libs/issues/89

As a work around, for now, currently we are excluding the aws-lambda-java-core dependency and forcing the new version to be used during build. But this proposed change will allow us to clean up that work around.

Testing

Built using mvn clean install. Built successfully. Tested by running two skills within the same application and both skills executed as expected.

Screenshots (if appropriate)

Types of changes

[x ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

[x ] My code follows the code style of this project
[ ] My change requires a change to the documentation
[ ] I have updated the documentation accordingly
[ ] I have read the README document
[X ] I have added tests to cover my changes
[x ] All new and existing tests passed

License

[ x] By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

prakshai commented 3 years ago

Some of the files that are failing checkstyle aren't even the ones I edited. Regardless, will be more than happy to fix them if there are some instructions on how to fix them.

prakshai commented 3 years ago

The PR looks to be missing tests for a scenario where all the skills in the collection are visited but none can handle the skill request and ends up throwing an exception?

I thought this should cover for that: https://github.com/alexa/alexa-skills-kit-sdk-for-java/blob/2.0.x/ask-sdk-lambda-support/tst/com/amazon/ask/SkillStreamHandlerTest.java#L91-L95

prakshai commented 3 years ago

I see that the latest travis build is failing because of an unused import

Pushed a change to fix it

alexa / alexa-skills-kit-sdk-for-java