Create a basic mailchimp client #1
Conversation
TickleThePanda
force-pushed
the
mailing_list_client
branch
from
8863784
to
f2b3ba8
Compare
| import org.springframework.boot.autoconfigure.SpringBootApplication; | ||
|
|
||
| @SpringBootApplication | ||
| @EnableAutoConfiguration |
|
|
||
| private static final Logger LOGGER = LogManager.getLogger(); | ||
|
|
||
| @GetMapping() |
| import java.util.Set; | ||
|
|
||
| /** | ||
| * An client for mailing lists service with the following functionality: |
| return DigestUtils.md5Hex(email.toLowerCase()); | ||
| } | ||
|
|
||
| private final MailchimpInternalHttpClient httpClient; |
There was a problem hiding this comment.
Could we move this private field right to the top of the class and then move the private constructor to just below getInstance?
There was a problem hiding this comment.
What about moving the convertEmailToId to the top instead, I think that improves the readability more?
| * Configuration for the Mailchimp client. | ||
| */ | ||
| public class MailchimpClientConfiguration { | ||
| private static final String SCHEME = "https"; |
There was a problem hiding this comment.
Would any of these benefit from being pulled out into a properties file of some kind, but with the ability to use these values as the defaults if a value isn't provided in the properties file?
Or if not possible due to this being a reusable component, have a builder that can be used to create the configuration, again using these values as the defaults if none are setup using the builder's methods?
There was a problem hiding this comment.
I don't think there's any benefit from these being pulled out into a properties file; if any of this changes then it would have been a major/breaking change on the mailchimp side of the API and I guess a lot of our code would change with that. Mailchimp still have their old versions of their API running in the same location as well - I just wanted it to be more explicit here what each of these values meant.
| } | ||
|
|
||
| private <T> T doRequest(HttpUriRequest request, Class<T> responseClass) throws MailingListClientException { | ||
| LOG.debug("making {} request for {}", request::getMethod, request::getURI); |
There was a problem hiding this comment.
Is it worth adding something to the logging page to say that method references should be used for debug logging for performance...or should it be assumed knowledge?
| */ | ||
| @GetMapping | ||
| @ResponseBody | ||
| public void handshake() { |
There was a problem hiding this comment.
Do you need the @ResponseBody?
| * | ||
| * @param listId the ID of the list from which the member will be | ||
| * removed from | ||
| * @param email the email of the member who will be removed |
There was a problem hiding this comment.
throws exception if deletion failed
test what happens when a non-existent user is removed from list
| public static MailchimpInternalHttpClient getInstance(MailchimpClientConfiguration configuration) { | ||
|
|
||
| CredentialsProvider provider = new BasicCredentialsProvider(); | ||
| Credentials credentials = new UsernamePasswordCredentials("user", configuration.getApiKey()); |
There was a problem hiding this comment.
comment saying that mailchimp doesn't care about what the user is
|
|
||
| private final MailchimpClientConfiguration configuration; | ||
|
|
||
| MailchimpInternalHttpClient( |
There was a problem hiding this comment.
Comment warning that this should not be used.
There was a problem hiding this comment.
Deprecated annotation
| } | ||
|
|
||
| // dash is required in the api key | ||
| private static final MailchimpClientConfiguration config = new MailchimpClientConfiguration("abcd-ab1"); |
There was a problem hiding this comment.
Check variable name
| } | ||
|
|
||
| @Nested | ||
| @DisplayName("a non-existant member") |
mailing-list-client- This project provides an API for integrating with a mailing list service. It provides an implementation of the API for Mailchimp.mailchimp-webhooks- This project provides an small example Spring Boot client that can listen to webhooks from Mailchimp that register user details changing in Mailchimp.Things it doesn't address yet: