revaturelabs / caliber-2-meta

Meta Repository for Caliber 2
5 stars 1 forks source link

Walk through for nonfunctional intercoms in Caliber2 - Is there a REST query format standard we need? #64

Open Jwkellenberger opened 5 years ago

Jwkellenberger commented 5 years ago

The original issue

It turns out the H2 data was fine and properly running. But, the data presenting via queries was being converted to junk values (-1) Query: http://localhost:10000/assessment/all/assessment/batch/2150?week=1

[{"assessmentId":2066,"rawScore":40,"assessmentTitle":null,"assessmentType":"Verbal","weekNumber":1,
"batchId":-1,"assessmentCategory":-1},
{"assessmentId":2067,"rawScore":40,"assessmentTitle":null,"assessmentType":"Exam","weekNumber":1,
"batchId":-1,"assessmentCategory":-1},
{"assessmentId":2068,"rawScore":20,"assessmentTitle":null,"assessmentType":"Other","weekNumber":1,
"batchId":-1,"assessmentCategory":-1}]

vs the data base entries:

Assessment [assessmentId=2066, rawScore=40, assessmentTitle=null, assessmentType=Verbal, weekNumber=1, 
batchId=2150, assessmentCategory=1]
Assessment [assessmentId=2067, rawScore=40, assessmentTitle=null, assessmentType=Exam, weekNumber=1, 
batchId=2150, assessmentCategory=1]
Assessment [assessmentId=2068, rawScore=20, assessmentTitle=null, assessmentType=Other, weekNumber=1, 
batchId=2150, assessmentCategory=1]

The Culprit

I was just poking around at the controller on the Assessment service And this method verifying the data with the batch and category service was responsible for the edits: assessment-service/src/main/java/com/revature/caliber/services/AssessmentService.java

private List<Assessment> checkBatchAndCategory(List<Assessment> assessmentList){
        Map<Integer, Boolean> batchConnected = new HashMap<>();
        Map<Integer, Boolean> categoryConnected = new HashMap<>();

        for(int i = 0; i < assessmentList.size(); i++) {
            Assessment a = assessmentList.get(i);
            Integer tempBatch = a.getBatchId();
            Integer tempCategory = a.getAssessmentCategory();

            if(!batchConnected.containsKey(a.getBatchId())) {
                boolean result = false;

                if(contactBatchService(a)) {
                    result = true;
                }
                batchConnected.put(tempBatch, result);
            }
            if(!categoryConnected.containsKey(a.getAssessmentCategory())) {
                boolean result = false;

                if(contactCategoryService(a)) {
                    result = true;
                }
                categoryConnected.put(tempCategory, result);
            }

            if(!batchConnected.get(tempBatch)) a.setBatchId(-1);
            if(!categoryConnected.get(tempCategory)) a.setAssessmentCategory(-1);
        }

        return assessmentList;
    }

More Digging

It turns out it was not properly connecting to the batch and category service

Output logs tracking the issue

2019-06-29 20:11:10 DEBUG class:47 - Inside findAssessmentsByBatch
Assessment [assessmentId=2074, rawScore=40, assessmentTitle=null, assessmentType=Verbal, weekNumber=4, batchId=2150, assessmentCategory=12]
Assessment [assessmentId=2075, rawScore=40, assessmentTitle=null, assessmentType=Exam, weekNumber=4, batchId=2150, assessmentCategory=12]
Assessment [assessmentId=2076, rawScore=20, assessmentTitle=null, assessmentType=Project, weekNumber=4, batchId=2150, assessmentCategory=12]
2019-06-29 20:11:10 WARN  class:78 - Could not connect with BatchService
2019-06-29 20:11:10 WARN  class:79 - status 404 reading BatchClient#getBatchById(Integer); content:
{"timestamp":1561853470129,"status":404,"error":"Not Found","message":"/all/batch/2150","path":"/all/batch/2150"}
2019-06-29 20:11:10 WARN  class:105 - Could not connect with CategoryService
2019-06-29 20:11:10 WARN  class:106 - status 404 reading CategoryClient#getCategoryById(Integer); content:
{"timestamp":1561853470136,"status":404,"error":"Not Found","message":"/all/category/12","path":"/all/category/12"}
Assessment [assessmentId=2074, rawScore=40, assessmentTitle=null, assessmentType=Verbal, weekNumber=4, 
batchId=-1, assessmentCategory=-1]
Assessment [assessmentId=2075, rawScore=40, assessmentTitle=null, assessmentType=Exam, weekNumber=4, 
batchId=-1, assessmentCategory=-1]
Assessment [assessmentId=2076, rawScore=20, assessmentTitle=null, assessmentType=Project, weekNumber=4, 
batchId=-1, assessmentCategory=-1]

So, what could make them not connect? They seemed fine on my end? http://localhost:10003/category/12

{"categoryId":12,"skillCategory":"JavaScript","categoryOwner":"Training","active":true}

http://localhost:10002/batch/all/batch/2150

{"batchId":2150,"trainingName":"1604 Apr25 Java","trainingType":"Corporate","skillType":"Full Stack Java/JEE","trainer":"Natalie Church","coTrainer":null,"locationId":3,"location":"Ford, 111 Ford Street, Detroit, MI 33333","startDate":1531195200000,"endDate":1542258000000,"goodGrade":85,"passingGrade":70,"weeks":3}

The Simple Mistake that took too long to find

/AssessmentService/src/main/java/com/revature/caliber/intercoms/CategoryClient.java

package com.revature.caliber.intercoms;

import org.springframework.cloud.netflix.feign.FeignClient;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;

import com.revature.caliber.beans.Category;

@FeignClient(name="category-service", fallback=CategoryClientFallback.class)
public interface CategoryClient {

    @GetMapping(value="all/category/{id}")
    public ResponseEntity<Category> getCategoryById(@PathVariable(value="id") Integer categoryId);
}

It turns out that it is querying a method that doesn't exist on the category controller

The proper query, or at least currently implemented one is /category/{id}

@GetMapping(value="all/category/{id}")
         |
         |
         v
@GetMapping(value="/category/{id}")

Progress

The simple string swap allowed for the query to come through, at least for the category

[{"assessmentId":2074,"rawScore":40,"assessmentTitle":null,"assessmentType":"Verbal","weekNumber":4,
"batchId":-1,"assessmentCategory":12},
{"assessmentId":2075,"rawScore":40,"assessmentTitle":null,"assessmentType":"Exam","weekNumber":4,
"batchId":-1,"assessmentCategory":12},
{"assessmentId":2076,"rawScore":20,"assessmentTitle":null,"assessmentType":"Project","weekNumber":4,
"batchId":-1,"assessmentCategory":12}]

Alas, no more -1's

Luckily, the batchid had the same issue

@GetMapping(value="all/batch/{id}") to the proper path:
         |
         |
         v
@GetMapping(value="batch/all/batch/{id}")

For the working file

assessment-service/src/main/java/com/revature/caliber/intercoms/BatchClient.java

package com.revature.caliber.intercoms;

import org.springframework.cloud.netflix.feign.FeignClient;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;

import com.revature.caliber.beans.BatchEntity;

@FeignClient(name = "batch-service", fallback=BatchClientFallback.class)
public interface BatchClient {

    @GetMapping(value="batch/all/batch/{id}")
    public BatchEntity getBatchById(@PathVariable(value="id") Integer batchId);
}

The Result

The ability to actually progress... Whew... http://localhost:10000/assessment/all/assessment/batch/2150?week=4

[{"assessmentId":2074,"rawScore":40,"assessmentTitle":null,"assessmentType":"Verbal","weekNumber":4,
"batchId":2150,"assessmentCategory":12},
{"assessmentId":2075,"rawScore":40,"assessmentTitle":null,"assessmentType":"Exam","weekNumber":4,
"batchId":2150,"assessmentCategory":12},
{"assessmentId":2076,"rawScore":20,"assessmentTitle":null,"assessmentType":"Project","weekNumber":4,
"batchId":2150,"assessmentCategory":12}]

Moving forward

This confirms that the previous project had some plot holes. The convention for the query followed by the Intercom client classes did not align with the queries that currently work on our active services.

Is there a desired convention that we should endeavor to uphold, and change the current query paths for the service, or are they fine as is?

@Quinn-Donnelly @rlayneorr

Quinn-Donnelly commented 5 years ago

@Jwkellenberger Not sure if this one had been clarified in the standup on Monday.

For the URL pattern the inconsistancies are coming from the previous batches that were attempting to replicate the exact url patterns of the first version of caliber. These url patterns do not work within the updated envirment do to how the load balenced routing is setup. Therefore, using the following pattern should be the standard approach from here on out.

{ZUUL URL (ALB url when in prod)}/{service identifier}/ after the final / standard REST conventions should be followed the paths that exist that are starting with /all may be converted to this standard.