Object v2 channel contract tests mc#248
Conversation
| public class Include implements ParameterEnricher { | ||
| static final String INCLUDE_PARAM_NAME = "include"; | ||
| static final String INCLUDE_CUSTOM_PARAM_VALUE = "custom"; | ||
| static final String INCLUDE_CUSTOM_PARAM_VALUE = "custom,type,status"; |
There was a problem hiding this comment.
Shouldn't this be handled by ObjectApiEndpoint?
There was a problem hiding this comment.
The thing is that method Include#enrichParameters is executed after ObjectApiEndpoint#createBaseParams. So even you set proper values in ObjectApiEndpoint#createBaseParams then they might be overritten in Include#enrichParameters.
I could revert the change you pointed out but then modification of Include#enrichParameters is required from
from
public Map<String, String> enrichParameters(Map<String, String> baseParams) {
final Map<String, String> enrichedMap = new HashMap<>(baseParams);
if (!inclusionFlags.isEmpty()) {
enrichedMap.put(INCLUDE_PARAM_NAME, join(inclusionFlags));
}
return enrichedMap;
}
to
public Map<String, String> enrichParameters(Map<String, String> baseParams) {
final Map<String, String> enrichedMap = new HashMap<>(baseParams);
if (!inclusionFlags.isEmpty()) {
String currentIncludeValue = enrichedMap.get(INCLUDE_PARAM_NAME);
inclusionFlags.add(currentIncludeValue);
enrichedMap.put(INCLUDE_PARAM_NAME, join(inclusionFlags));
}
return enrichedMap;
}
Do you see other options?
There was a problem hiding this comment.
Maybe remove change in ObjectApiEndpoint and that one here and put something like this:
static final Collection<String> DEFAULT_INCLUDE_FLAGS = Arrays.asList("type", "status");
private final List<String> inclusionFlags = new ArrayList<>(DEFAULT_INCLUDE_FLAGS);
That way you should have it regardless of custom being added or not. The question that I have for you though. Are you sure we should add always both type and status? I'm pretty sure it's a status only for memberships, but somehow I don't see it in Kotlin implementation so I don't know anymore
There was a problem hiding this comment.
As discussed on a cal I am working on this.
| @Given("the data for {string} member") | ||
| fun the_data_for_member(memberName: String) { | ||
| val channelMember: PNMembers = loadMember(memberName) | ||
| channelMembersState.uuids = listOf(PNUUID.uuid(channelMember.uuid.id)) |
There was a problem hiding this comment.
I wonder why populate this uuids list here. Nothing in the step says that it should be done. I'd implemented it by just storing the member data in the state class. Later steps should decide what to do with it
There was a problem hiding this comment.
Makes sense. I will change it.
| setChannelMetadataState.pnChannelMetadata?.name = pnChannelMetadata.name | ||
| setChannelMetadataState.pnChannelMetadata?.description = pnChannelMetadata.description | ||
| setChannelMetadataState.pnChannelMetadata?.status = pnChannelMetadata.status | ||
| setChannelMetadataState.pnChannelMetadata?.type = pnChannelMetadata.type |
There was a problem hiding this comment.
Why not like this?
setChannelMetadataState.pnChannelMetadata = pnChannelMetadata
There was a problem hiding this comment.
Good catch. I will modify it.
src/main/java/com/pubnub/api/endpoints/objects_api/CompositeParameterEnricher.java
Outdated
Show resolved
Hide resolved
src/main/java/com/pubnub/api/endpoints/objects_api/utils/IncludeParamValue.java
Outdated
Show resolved
Hide resolved
src/test/java/com/pubnub/api/endpoints/objects_api/utils/IncludeTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/pubnub/api/endpoints/objects_api/utils/IncludeTest.java
Outdated
Show resolved
Hide resolved
Changes after code review.
Changes after code review.
Changes after code review.
No description provided.