Skip to content
Snippets Groups Projects

Create an object to extract data from MPC user agents #8

Merged Martin Lowe requested to merge github/fork/autumnfound/malowe/master/8 into master

Finished UserAgent implementation w/ validity checks. Implemented call to generate install from valid user agents. Fixed issue with generating dummy install content where java and eclipse version arrays would break causing issues in posted data. Additionally fixed some badly named variables that no longer made sense. Added MPC user agent heading to dummy content script to spoof source.

Change-Id: I32877ad6558edfb04c4d3453cf36e4750ad634c8 Signed-off-by: Martin Lowe martin.lowe@eclipse-foundation.org

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
121 124 @Path("/{listingId}/{version}")
122 125 public Response postInstallMetrics(@PathParam("listingId") String listingId, @PathParam("version") String version,
123 126 Install installDetails) {
124 // update the install details to reflect the current request
125 installDetails.setInstallDate(new Date(System.currentTimeMillis()));
126 installDetails.setListingId(listingId);
127 installDetails.setVersion(version);
127 Install record = null;
128 128
129 // check that connection was opened by MPC, and check for install information
  • Created by: mbarbero

    I don't get why we rely on user agent to get such the "install record". They should be passed via query parameters, or custom headers. Relying on a specific string format for the user agent is flaky, not testable, and not future proof / extensible.

  • Martin Lowe
    Martin Lowe @malowe started a thread on commit 63cd6b36
  • 121 124 @Path("/{listingId}/{version}")
    122 125 public Response postInstallMetrics(@PathParam("listingId") String listingId, @PathParam("version") String version,
    123 126 Install installDetails) {
    124 // update the install details to reflect the current request
    125 installDetails.setInstallDate(new Date(System.currentTimeMillis()));
    126 installDetails.setListingId(listingId);
    127 installDetails.setVersion(version);
    127 Install record = null;
    128 128
    129 // check that connection was opened by MPC, and check for install information
    • Author Maintainer

      I agree that it isn't the most robust solution. This was captured as a wanted addition to the API as an option for posting. That's why I wrapped additional logic to capture the JSON body, and check that the agent reads as from MPC before allowing the install, as they are the only consumer of this endpoint outside of our initial set up (where we can spoof the agent). This way, should the MPC agent change, there is a backup of reading the JSON.

      If you want to look at removing this, which I wouldn't be very opposed to, we should talk w/ Chris + potentially the guys from MPC to see how important this feature is.

  • Christopher Guindon
  • 121 124 @Path("/{listingId}/{version}")
    122 125 public Response postInstallMetrics(@PathParam("listingId") String listingId, @PathParam("version") String version,
    123 126 Install installDetails) {
    124 // update the install details to reflect the current request
    125 installDetails.setInstallDate(new Date(System.currentTimeMillis()));
    126 installDetails.setListingId(listingId);
    127 installDetails.setVersion(version);
    127 Install record = null;
    128 128
    129 // check that connection was opened by MPC, and check for install information
  • Created by mbarbero

    Review: Approved

  • Please register or sign in to reply
    Loading