[GitHub] [trafficcontrol] moltzaum opened a new pull request #3583: Added IDImpl struct

[GitBox <gi...@apache.org>]

moltzaum opened a new pull request #3583: Added IDImpl struct
URL: https://github.com/apache/trafficcontrol/pull/3583
 
 
   ## What does this PR (Pull Request) do?
   
   IDImpl is a struct that implements GetKeys and SetKeys for the common case where the only key is a single numerical ID. The idea is to make a single, common implementation that can be used across all endpoints that can use it. As a bonus these endpoints don't need to make a GetKeys or SetKeys function, nor do they have to provide the json and db tags and we know the behavior we want is implemented.
   
   The downside is that current swagger comments may be broken (although we weren't exactly using it anyway).
   
   This is mostly a test to get feedback. I've had an idea like this for awhile, but didn't implement it because I had doubts. I think it is relatively useful, although it is debatable whether or not this works well as a layer of abstraction since "every abstraction layer has a cost". Also, some might perfer to keep the tc structs 'clean'.
   
   For now, I've only made the CDN struct use the IDImpl. IDImpl only works for Nullable structs (the structs that we define endpoints on).
   
   ## Which Traffic Control components are affected by this PR?
   
   The main affected component is Traffic Ops, but since I've edited a TC struct, it could affect any component that uses the structs. go is smart enough so that `StructType.EmbeddedType.Field` can be called as `StructType.Field` as long as that field name doesn't exist higher up in the embedded struct hierarchy, so moving the field to an embedded type shouldn't affect much.
   
   ## What is the best way to verify this PR?
   
   Tests already exist for CDNs and should pass.
   
   ## The following criteria are ALL met by this PR
   
   - [x] Tests already exist for affected pieces of code
   - [ ] This PR includes documentation OR I have explained why documentation is unnecessary
   - [x] This PR includes an update to CHANGELOG.md OR such an update is not necessary
   - [x] This PR includes any and all required license headers
   - [x] This PR ensures that database migration sequence is correct OR this PR does not include a database migration
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://www.apache.org/security/) for details)
   
   <!--
   Licensed to the Apache Software Foundation (ASF) under one
   or more contributor license agreements.  See the NOTICE file
   distributed with this work for additional information
   regarding copyright ownership.  The ASF licenses this file
   to you under the Apache License, Version 2.0 (the
   "License"); you may not use this file except in compliance
   with the License.  You may obtain a copy of the License at
   
       http://www.apache.org/licenses/LICENSE-2.0
   
   Unless required by applicable law or agreed to in writing,
   software distributed under the License is distributed on an
   "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
   KIND, either express or implied.  See the License for the
   specific language governing permissions and limitations
   under the License.
   -->
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services