Skip to content

Add common project #1043

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.

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
merged 9 commits into from
Mar 8, 2019
Merged

Add common project #1043

merged 9 commits into from
Mar 8, 2019

Conversation

lpatino10
Copy link
Contributor

This PR adds a new common project with an SdkCommon class to handle logic that's common to the services, but doesn't belong in the core since it's project-specific. Currently, that means it handles building the User-Agent and X-IBMCloud-SDK-Analytics values that will be added to each service request.

This PR won't contain the changes to the service methods, but for reference, the code will look something like this:

public ServiceCall<QueryResponse> query(QueryOptions queryOptions) {
  ...
  RequestBuilder builder = RequestBuilder.post(RequestBuilder.constructHttpUrl(getEndPoint(), pathSegments, pathParameters));
  builder.query(VERSION, versionDate);
  if (queryOptions.loggingOptOut() != null) {
    builder.header("X-Watson-Logging-Opt-Out", queryOptions.loggingOptOut());
  }

  // Add any SDK-specific "default" headers.
  Map<String, String> defaultHeaders = SdkCommon.getDefaultHeaders("discovery", "v1", "query");
  for (Entry<String, String> header : defaultHeaders.entrySet()) {
    builder.header(header.getKey(), header.getValue());
  }
  ...
  final JsonObject contentJson = new JsonObject();
  ...
  builder.bodyJson(contentJson);
  return createServiceCall(builder.build(), ResponseConverterUtils.getObject(QueryResponse.class));
}

@lpatino10
Copy link
Contributor Author

Leaving this as a draft until the core library is updated from this PR. Then, I'll update dependencies on this side to get everything working.

);

headers.put(HttpHeaders.X_IBMCLOUD_SDK_ANALYTICS, sdkAnalyticsHeaderValue);
headers.put(HttpHeaders.USER_AGENT, getUserAgent());
Copy link
Contributor

Choose a reason for hiding this comment

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

technically speaking... :) the user agent header value could be stored in a static field and not computed each time getDefaultHeaders() is called. I'm sure the performance difference is negligible, but I couldn't stop myself from mentioning it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any performance improvement is a good one! Done

@lpatino10 lpatino10 marked this pull request as ready for review March 8, 2019 15:26
@lpatino10 lpatino10 merged commit 82c2fb9 into v7.0.0 Mar 8, 2019
@lpatino10 lpatino10 deleted the common-class branch March 8, 2019 20:26
@watson-github-bot
Copy link
Contributor

🎉 This PR is included in version 7.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants