Rewrote /user/current to Go #3996

ocket8888 · 2019-10-15T22:17:33Z

What does this PR (Pull Request) do?

This PR fixes Rewrite /user/current to Go #3838

This rewrites the previously unimplemented PUT handler of /user/current into Go. It adds support in both the Go and Python clients, with some associated testing as well as documentation updates.

This differs from the way the endpoint was handled in Perl - in Perl, every field in the request was optional (under the top-level "user" object, obviously) and only those included would be updated - as though it were a PATCH request. This instead demands a full representation of a user as is proper for PUT. Additionally, the Perl returned only a success message, this handler returns a representation of the user as it exists following the update.

Pursuant to a mailing list discussion changing this endpoint's behavior - albeit for the better - was shot down, and I re-rewrote it to implement Perl-style PATCH-like behavior.

Which Traffic Control components are affected by this PR?

Documentation
Traffic Control Client (Python)
Traffic Control Client (Go)
Traffic Ops

What is the best way to verify this PR?

Build a TO RPM, start Traffic Ops, make a PUT request to /user/current, run the Go client/API integration tests, build and read the documentation.

The following criteria are ALL met by this PR

This PR includes tests
This PR includes documentation
This PR includes an update to CHANGELOG.md
This PR includes any and all required license headers
This PR does not include a database migration
This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

asf-ci · 2019-10-15T22:44:51Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4533/

ocket8888 · 2019-10-18T18:51:45Z

retest this please

asf-ci · 2019-10-18T19:25:32Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4578/

docs/source/api/user_current.rst

traffic_ops/traffic_ops_golang/routing/routes.go

mhoppa · 2019-10-21T15:58:31Z

traffic_ops/traffic_ops_golang/user/current.go

think about using sqlx structscan? and sqlx for the query above instead of using $1,$2,$3,$4. With a big parametrized query like this I worry about maintenance/debugging

asf-ci · 2019-10-21T21:11:21Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4599/

asf-ci · 2019-10-29T22:46:01Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4670/

lib/go-tc/users.go

asf-ci · 2019-10-31T15:54:21Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4681/

mhoppa

Manually testing looks great.

One thing I did notice is the perl implementation requires certain fields to be defined. Do we want to carry that requirement through?

{
    "alerts": [
        {
            "level": "error",
            "text": "fullName is required"
        },
        {
            "level": "error",
            "text": "email is required"
        },
        {
            "level": "error",
            "text": "role is required"
        },
        {
            "level": "error",
            "text": "username is required"
        }
    ]
}

traffic_ops/traffic_ops_golang/user/current.go

lib/go-tc/users.go

traffic_ops/traffic_ops_golang/user/current.go

mhoppa

manual tests work and so do API tests

asf-ci · 2019-11-01T17:43:31Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4689/

ocket8888 · 2019-11-01T17:53:45Z

Eh. I don't think that difference is actually going to break anyone, and I did make sure identifiers can't be null. And to be honest I'm sick to death of working on this.

lib/go-tc/users.go

traffic_ops/traffic_ops_golang/user/current.go

asf-ci · 2019-11-01T20:49:36Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4694/

lib/go-tc/users.go

asf-ci · 2019-11-04T17:36:49Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4704/

asf-ci · 2019-11-04T17:56:51Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4705/

asf-ci · 2019-11-07T17:46:31Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4726/

asf-ci · 2019-11-11T18:45:02Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4767/

mhoppa · 2019-11-20T20:49:12Z

can you rebase?

traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go

lib/go-tc/users.go

traffic_ops/traffic_ops_golang/user/current.go

asf-ci · 2019-12-16T22:05:26Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4943/

asf-ci · 2019-12-16T22:08:45Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4944/

asf-ci · 2019-12-17T19:28:49Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4948/

asf-ci · 2019-12-17T20:06:49Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4949/

mitchell852

unit tests, api tests, ui tests and manual TP test passed.

ocket8888 added Traffic Ops related to Traffic Ops documentation related to documentation tests related to tests and/or testing infrastructure TC Client (python) related to the Python implementation of a TC client TO Client (Go) related to the Go implementation of a TC client labels Oct 15, 2019

ocket8888 added this to the Go Rewrite milestone Oct 15, 2019

mhoppa reviewed Oct 21, 2019

View reviewed changes

ocket8888 force-pushed the user-current-go branch from 121193c to 85d0d51 Compare October 21, 2019 20:41

mitchell852 added the tech debt rework due to choosing easy/limited solution label Oct 24, 2019

mhoppa reviewed Oct 31, 2019

View reviewed changes

lib/go-tc/users.go Outdated Show resolved Hide resolved

ocket8888 force-pushed the user-current-go branch from 16e21a5 to a4317d2 Compare October 31, 2019 15:42

mhoppa reviewed Nov 1, 2019

View reviewed changes

traffic_ops/traffic_ops_golang/user/current.go Outdated Show resolved Hide resolved

lib/go-tc/users.go Outdated Show resolved Hide resolved

traffic_ops/traffic_ops_golang/user/current.go Outdated Show resolved Hide resolved

mhoppa approved these changes Nov 1, 2019

View reviewed changes

ChrisHines reviewed Nov 1, 2019

View reviewed changes

awilliams reviewed Nov 4, 2019

View reviewed changes

lib/go-tc/users.go Outdated Show resolved Hide resolved

lib/go-tc/users.go Outdated Show resolved Hide resolved

ocket8888 force-pushed the user-current-go branch from 7065204 to a50e728 Compare November 7, 2019 17:38

rawlinp suggested changes Dec 5, 2019

View reviewed changes

ocket8888 force-pushed the user-current-go branch from 715dbbc to ac250df Compare December 16, 2019 21:50

ocket8888 added 18 commits December 17, 2019 11:01

Rewrote /user/current to Go

dfc20a0

Update CHANGELOG

640d5e7

add client method for updating current user

e97e76c

add client/TO integration testing

19ecf58

updated documentation

875dbd1

Added Python client support

733bdba

Reworked PUT /user/current to behave like a PATCH

1deb0f5

Added built TO binary to gitignore

0d76776

go fmt

0ab48fc

Use API functions to write responses

093e513

Remove unnecessary 'else'

fa7f2f7

Changed method name

d0ed851

fix typo

7947b20

simplify return

a45a709

Simplified password update

903d176

More verbose parameter name

2d83ddb

JOIN clause now more appropriate for the schema

0a776c9

relaxed restriction on password confirmation to match Perl

6c12c6e

ocket8888 force-pushed the user-current-go branch from dee0e0b to 6c12c6e Compare December 17, 2019 18:01

mitchell852 approved these changes Dec 19, 2019

View reviewed changes

mitchell852 merged commit 052f5e9 into apache:master Dec 19, 2019

ocket8888 deleted the user-current-go branch December 19, 2019 21:50

mhoppa mentioned this pull request Jan 9, 2020

Fix role name not being set on users get #4282

Merged

6 tasks

tinfoil-knight mentioned this pull request Mar 10, 2023

return correct error on duplicate email while updating user #7394

Merged

3 tasks

Rewrote /user/current to Go #3996

Rewrote /user/current to Go #3996

Uh oh!

Conversation

ocket8888 commented Oct 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR (Pull Request) do?

Which Traffic Control components are affected by this PR?

What is the best way to verify this PR?

The following criteria are ALL met by this PR

Uh oh!

asf-ci commented Oct 15, 2019

Uh oh!

ocket8888 commented Oct 18, 2019

Uh oh!

asf-ci commented Oct 18, 2019

Uh oh!

Uh oh!

Uh oh!

mhoppa Oct 21, 2019

Choose a reason for hiding this comment

Uh oh!

asf-ci commented Oct 21, 2019

Uh oh!

asf-ci commented Oct 29, 2019

Uh oh!

Uh oh!

asf-ci commented Oct 31, 2019

Uh oh!

mhoppa left a comment • edited by ocket8888 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mhoppa left a comment

Choose a reason for hiding this comment

Uh oh!

asf-ci commented Nov 1, 2019

Uh oh!

ocket8888 commented Nov 1, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asf-ci commented Nov 1, 2019

Uh oh!

Uh oh!

Uh oh!

asf-ci commented Nov 4, 2019

Uh oh!

asf-ci commented Nov 4, 2019

Uh oh!

asf-ci commented Nov 7, 2019

Uh oh!

asf-ci commented Nov 11, 2019

Uh oh!

mhoppa commented Nov 20, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asf-ci commented Dec 16, 2019

Uh oh!

asf-ci commented Dec 16, 2019

Uh oh!

asf-ci commented Dec 17, 2019

Uh oh!

asf-ci commented Dec 17, 2019

Uh oh!

mitchell852 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

ocket8888 commented Oct 15, 2019 •

edited

Loading

mhoppa left a comment •

edited by ocket8888

Loading