-
Notifications
You must be signed in to change notification settings - Fork 63
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
add non-public tweet metrics and misc fields for API v2 #366
base: develop
Are you sure you want to change the base?
Conversation
* @param requestTweetFieldsScope | ||
* @return a TweetList object containing a list of tweets and the next token if recursiveCall is set to false | ||
*/ | ||
public TweetList getUserTimeline(String userId, AdditionalParameters additionalParameters, REQUEST_TWEET_FIELDS_SCOPE requestTweetFieldsScope); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Little question, isn't it possible to add the requestTweetFieldsScope
parameter inside the additionalParameters
object?
It will avoid creating new constructors no ?
Hey, Thank you very much for your time !
Good question, personally I would throw an exception (and use For the other points, it looks relevant. Finally for the tests, you should create json sample files and try to deserialize them and be sure that all fields are OK (as I did for TweetV2 object for example). |
Hi, thanks for the feedback. Will work on it. At the moment I'm still trying requests and learn something about the partial API errors. |
|
GitGuardian id | Secret | Commit | Filename | |
---|---|---|---|---|
- | Generic High Entropy Secret | 5d49d4a | src/test/resources/tests/dm_simple_example_v2.json | View secret |
- | Generic High Entropy Secret | 5d49d4a | src/test/resources/tests/dm_simple_example_v2.json | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Our GitHub checks need improvements? Share your feedbacks!
@@ -104,14 +104,12 @@ public class TwitterClient implements ITwitterClientV1, ITwitterClientV2, ITwitt | |||
.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) | |||
.setSerializationInclusion(JsonInclude.Include.NON_NULL) | |||
.findAndRegisterModules(); | |||
public static final String TWEET_FIELDS = "tweet.fields"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the same code style to avoid this
Did you leave it up ? Can I close it? |
Well.. I ran out of breath for this pull request back in March and just left it as it was in my project. In retrospect it should not have been a pull request at this point, since the error handling I added was erroneous. Now I want to improve the error handling, so I merged. I think there must be dto objects for mixed state (where you have returned objects and error messages at the same time). Like the class TweetError, but that class is never used, as far as I can see? So I'm now working on it again, so you can leave it open at the moment. Well, now non-public data and error handling are intertwined, which is not good. I should maybe revert the error handling to have a clean state for this pull-request. Sorry for the chaos. |
@redouane59 I did another update with some minor changes discusses earlier. Wanted to test it in my project, but the update of com.fasterxml.jackson didn't work out with the rest of my project. Things got incompatible. I think my set up here is a mess. I lack the routine regarding integration. Maybe it will work out later. |
Do you have a similar problem than #427 ? |
Well. There are some design decisions that need to be discussed.
Tweet interface:
Obviously, API v1 and v2 are parting ways now even more. How to deal with that in the Interface
Tweet
and in the classTweetV1
with the v2 only getters likegetImpressionCount()
? Now it returns 0. Should it better throw an exception? Return -1? Changing return type to return null?Non-public:
What to return on non-public metrics in the DTOs, if requested in a public context? This metrics do not exist then and just returning 0 (which is happening) is misleading. To get around it, I added some
has
-methods (e.g.hasOrganicMetrics()
). (For consistency reasons I even added them for public metrics in tweets and user.) For the single metrics I still would prefer returningInteger null
instead ofint 0
.Error management:
There seems to be a whole world, where twitter returns with http 200, but has only errors in the response. This happens on insufficient access rights for non-public data. Couldn't even find docs on that on twitter. I throw
NoPermissionException
on a certain condition now, but that is highly preliminarily. Further knowledge is still missing here what can happen.Consumer key context:
To determine whether requesting with consumer key for user context or app-only bearer token, a var
useConsumerKey
inRequestHelperV2
is used. But that would mean constructor explosion. So I did not add it to all constructors. I guess that is not final. Furthermore, it maybe would be nice to determine that per request, not perTwitterClient
/RequestHelperV2
.Tests:
I skimmed over the tests but did not really had an idea.. I need some suggestions here what to do.