awslabs / awslabs/aws-mobile-appsync-sdk-android

Subscription ENUM input variables are nulled before being sent out via websocket and startSubscription

Open
#352 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Java
Stars
106
Forks
56
PR merge metrics
No merged PRs in 30d

Description

**Describe the bug**
The `startSubscription` method in `WebSocketConnectionManager` attempts to convert a `Subscription`'s variables into JSON by just calling `JSONObject(map)` over the subscription's `valueMap()` (see line 123 of `WebSocketConnectionManager`).

This is dangerous, because the `JSONObject(map)` constructor, in android API 19 to 30, cannot handle maps that have values which are enums, it nulls them out (specifically, see the `wrap(Object o)` method of `JSONObject` in these API versions).

This means that when a Subscription has a non-nullable input variable that is an apollo generated ENUM, `WebSocketConnectionManager` will null it before sending it to the service, which ofcourse will cause the service to throw an error.

`WebSocketConnectionManager` should use the generated `Variables.marshaller().marshal(..)` method to properly JSON-ify the subscription's variables. This can be seen in PR #351 .

**To Reproduce**
Steps to reproduce the behavior:
1. create a graphql subscription with a non-nullable ENUM as input.
2. attempt to execute a subscription execution on that subscription
3. observe that line 123 of WebSocketConnectionManager nulls the enum's value in the JSON Object.
4. observe error message recieved from backend on that subscription.

**To Reproduce via Unit Test**
to quickly observe the nulling effect, run the following unit test:
```java
@RunWith(RobolectricTestRunner.class)
@Config(sdk = 28)
public class WebSocketConnectionManagerUnitTest {

Subscription mockSubscription;
Operation.Variables mockVariables;

Map mapOfPrimitivesAndEnums = new HashMap<>();

@Test
public void testSubscriptionValueMapWithEnumIsInvalidJSON() {
mockSubscription = Mockito.mock(Subscription.class);
mockVariables = Mockito.mock(Operation.Variables.class);

mapOfPrimitivesAndEnums.put("str", "strVal");
mapOfPrimitivesAndEnums.put("enum", TestEnum.ENUM_VALUE);

Mockito.when(mockSubscription.variables()).thenReturn(mockVariables);
Mockito.when(mockVariables.valueMap()).thenReturn(Collections.unmodifiableMap(mapOfPrimitivesAndEnums));

JSONObject jsonObject = new JSONObject(mockSubscription.variables().valueMap());
assertNull(jsonObject.opt("enum")); // check that enum has been nulled out.
}
}

enum TestEnum {
ENUM_VALUE
}
```

**Expected behavior**
Enums sent via `WebSocketConnectionManager` should NOT be nulled before sent out. I imagine this is probably a bug for any non primitive object too (my PR should fix), i just happened to have observed it for enums.

**Screenshots**
If applicable, add screenshots to help explain your problem.

**Environment(please complete the following information):**
- AppSync SDK Version: 3.1.4

**Device Information (please complete the following information):**
- Device: Any
- Android API Version: from 19 to 30

**Additional context**
Add any other context about the problem here.

Contributor guide

Open the contributing guide

Research direction

Start in WebSocketConnectionManager at the line 123 conversion of subscription variables, then compare the proposed approach in PR #351. Use the provided Robolectric test scenario with an enum input and confirm that enum values are preserved when subscriptions are sent, with the relevant unit tests passing.

Written by the indexing model from the issue text.

Assessment

Tech stack
android, graphql, java
Domain
mobile-dev, networking
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.