)]}'
{"src/main/java/monasca/api/app/AlarmService.java":[{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"6373dd29894b44f4c4830af51b300ad72f447f53","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    Alarm alarm \u003d repo.findById(tenantId, alarmId);"},{"line_number":72,"context_line":"    Map\u003cString, AlarmSubExpression\u003e subAlarmMetricDefs \u003d repo.findAlarmSubExpressions(alarmId);"},{"line_number":73,"context_line":"    List\u003cMetricDefinition\u003e metrics \u003d repo.findMetrics(tenantId, alarmId);"},{"line_number":74,"context_line":"    repo.deleteById(alarmId);"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    // Notify interested parties of alarm deletion"},{"line_number":77,"context_line":"    String event \u003d"}],"source_content_type":"text/x-java","patch_set":1,"id":"5a890539_3fb18a8f","line":74,"updated":"2014-11-21 23:24:17.000000000","message":"There is a better way to do this from an interface stand-point. deleteById should just take the tenantId and alarmId.\n\nAlso, you shouldn\u0027t have to do this as two separate SQL queries. It would be better to do this as a single SQL query.\n\nSee the SQL queries in the Python API at\n\nmonasca-api/python/monasca/common/repositories/mysqlalarms_repository.py\n\nUnless there is a strong reason why deleteById should have tenantId added as a parameter. Then delete would just invoke deleteById(tenantId, alarmId) and the code in the repository would decide if if was going to implement as two queries or one, but one is better.","commit_id":"03a21341f22767a5213f1ba76dbf2adee95ff26d"},{"author":{"_account_id":12108,"name":"Gary Hessler","email":"gary.hessler@hp.com","username":"garyh"},"change_message_id":"c1ed4db2021f5e8236b9acc7adf36b6aa5781f96","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    Alarm alarm \u003d repo.findById(tenantId, alarmId);"},{"line_number":72,"context_line":"    Map\u003cString, AlarmSubExpression\u003e subAlarmMetricDefs \u003d repo.findAlarmSubExpressions(alarmId);"},{"line_number":73,"context_line":"    List\u003cMetricDefinition\u003e metrics \u003d repo.findMetrics(tenantId, alarmId);"},{"line_number":74,"context_line":"    repo.deleteById(alarmId);"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    // Notify interested parties of alarm deletion"},{"line_number":77,"context_line":"    String event \u003d"}],"source_content_type":"text/x-java","patch_set":1,"id":"5a890539_c4da4e4a","line":74,"in_reply_to":"5a890539_3fb18a8f","updated":"2014-11-24 20:55:12.000000000","message":"I think I understand what you are saying on passing the tenantId to the deleteById method but the alarm, metrics and subAlarmMetricDefs above are all needed by the event processing code below.  If I delete any of the queries above, I won\u0027t have the information needed to create the AlarmDeletedEvent object below.  I may be missing something?","commit_id":"03a21341f22767a5213f1ba76dbf2adee95ff26d"}],"src/main/java/monasca/api/domain/model/alarm/AlarmRepository.java":[{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"6373dd29894b44f4c4830af51b300ad72f447f53","unresolved":false,"context_lines":[{"line_number":11,"context_line":"public interface AlarmRepository {"},{"line_number":12,"context_line":"  /**"},{"line_number":13,"context_line":"   * Deletes all alarms associated with the {@code id}."},{"line_number":14,"context_line":"   */"},{"line_number":15,"context_line":"  void deleteById(String id);"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"  /**"}],"source_content_type":"text/x-java","patch_set":1,"id":"5a890539_7f74f2fd","line":14,"updated":"2014-11-21 23:24:17.000000000","message":"I think this interface should have tenantId added to it similar to findById as deleteById has a potential security hole in it.","commit_id":"03a21341f22767a5213f1ba76dbf2adee95ff26d"},{"author":{"_account_id":12108,"name":"Gary Hessler","email":"gary.hessler@hp.com","username":"garyh"},"change_message_id":"c1ed4db2021f5e8236b9acc7adf36b6aa5781f96","unresolved":false,"context_lines":[{"line_number":11,"context_line":"public interface AlarmRepository {"},{"line_number":12,"context_line":"  /**"},{"line_number":13,"context_line":"   * Deletes all alarms associated with the {@code id}."},{"line_number":14,"context_line":"   */"},{"line_number":15,"context_line":"  void deleteById(String id);"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"  /**"}],"source_content_type":"text/x-java","patch_set":1,"id":"5a890539_84a206ac","line":14,"in_reply_to":"5a890539_7f74f2fd","updated":"2014-11-24 20:55:12.000000000","message":"Agreed","commit_id":"03a21341f22767a5213f1ba76dbf2adee95ff26d"},{"author":{"_account_id":2419,"name":"Roland Hochmuth","email":"roland.hochmuth@hpe.com","username":"roland-hochmuth-s"},"change_message_id":"6373dd29894b44f4c4830af51b300ad72f447f53","unresolved":false,"context_lines":[{"line_number":35,"context_line":"  /**"},{"line_number":36,"context_line":"   * Gets the AlarmSubExpressions mapped by their Ids for an Alarm Id"},{"line_number":37,"context_line":"   */"},{"line_number":38,"context_line":"  Map\u003cString, AlarmSubExpression\u003e findAlarmSubExpressions(String alarmId);"},{"line_number":39,"context_line":"}"}],"source_content_type":"text/x-java","patch_set":1,"id":"5a890539_7f1dd2bd","line":38,"updated":"2014-11-21 23:24:17.000000000","message":"I think this interface should have tenantId added to it similar to the method up above. My concern is that this is exposing a possible security issue.","commit_id":"03a21341f22767a5213f1ba76dbf2adee95ff26d"},{"author":{"_account_id":12108,"name":"Gary Hessler","email":"gary.hessler@hp.com","username":"garyh"},"change_message_id":"c1ed4db2021f5e8236b9acc7adf36b6aa5781f96","unresolved":false,"context_lines":[{"line_number":35,"context_line":"  /**"},{"line_number":36,"context_line":"   * Gets the AlarmSubExpressions mapped by their Ids for an Alarm Id"},{"line_number":37,"context_line":"   */"},{"line_number":38,"context_line":"  Map\u003cString, AlarmSubExpression\u003e findAlarmSubExpressions(String alarmId);"},{"line_number":39,"context_line":"}"}],"source_content_type":"text/x-java","patch_set":1,"id":"5a890539_046e162f","line":38,"in_reply_to":"5a890539_7f1dd2bd","updated":"2014-11-24 20:55:12.000000000","message":"Craig is correct that the AlarmSubExpressions are not queried unless the Alarm is queried successfully first.","commit_id":"03a21341f22767a5213f1ba76dbf2adee95ff26d"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"26b46c8c14c5e900895ce9d64ec328791cc63053","unresolved":false,"context_lines":[{"line_number":35,"context_line":"  /**"},{"line_number":36,"context_line":"   * Gets the AlarmSubExpressions mapped by their Ids for an Alarm Id"},{"line_number":37,"context_line":"   */"},{"line_number":38,"context_line":"  Map\u003cString, AlarmSubExpression\u003e findAlarmSubExpressions(String alarmId);"},{"line_number":39,"context_line":"}"}],"source_content_type":"text/x-java","patch_set":1,"id":"5a890539_8d3b26d0","line":38,"in_reply_to":"5a890539_7f1dd2bd","updated":"2014-11-23 22:41:30.000000000","message":"I agree on the others, but I disagree on this one. This is only called from code that has already validated the alarm exists for the tenant id","commit_id":"03a21341f22767a5213f1ba76dbf2adee95ff26d"}],"src/main/java/monasca/api/infrastructure/persistence/mysql/AlarmMySqlRepositoryImpl.java":[{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"26b46c8c14c5e900895ce9d64ec328791cc63053","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        + \" dimension_set_id \u003d ?\", dimensionSetId);"},{"line_number":87,"context_line":"  }"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"  private static List\u003cMetricDefinition\u003e findMetrics(Handle handle, String tenantId, String alarmId) {"},{"line_number":90,"context_line":"    List\u003cMetricDefinition\u003e metricDefs \u003d new ArrayList\u003c\u003e();"},{"line_number":91,"context_line":"    for (Map\u003cString, Object\u003e row : handle.createQuery(METRIC_DEFS_FOR_ALARM_SQL)"},{"line_number":92,"context_line":"        .bind(\"alarmId\", alarmId).bind(\"tenantId\", tenantId).list()) {"}],"source_content_type":"text/x-java","patch_set":1,"id":"5a890539_cd31aeee","line":89,"updated":"2014-11-23 22:41:30.000000000","message":"Again, here, has the tenant id already been validated? If so, then don\u0027t require it here","commit_id":"03a21341f22767a5213f1ba76dbf2adee95ff26d"},{"author":{"_account_id":12108,"name":"Gary Hessler","email":"gary.hessler@hp.com","username":"garyh"},"change_message_id":"c1ed4db2021f5e8236b9acc7adf36b6aa5781f96","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        + \" dimension_set_id \u003d ?\", dimensionSetId);"},{"line_number":87,"context_line":"  }"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"  private static List\u003cMetricDefinition\u003e findMetrics(Handle handle, String tenantId, String alarmId) {"},{"line_number":90,"context_line":"    List\u003cMetricDefinition\u003e metricDefs \u003d new ArrayList\u003c\u003e();"},{"line_number":91,"context_line":"    for (Map\u003cString, Object\u003e row : handle.createQuery(METRIC_DEFS_FOR_ALARM_SQL)"},{"line_number":92,"context_line":"        .bind(\"alarmId\", alarmId).bind(\"tenantId\", tenantId).list()) {"}],"source_content_type":"text/x-java","patch_set":1,"id":"5a890539_c4adee85","line":89,"in_reply_to":"5a890539_cd31aeee","updated":"2014-11-24 20:55:12.000000000","message":"The tenantId is validated in some but not all cases.","commit_id":"03a21341f22767a5213f1ba76dbf2adee95ff26d"},{"author":{"_account_id":12512,"name":"Deklan Dieterly","email":"deklan.dieterly@hpe.com","username":"deklan"},"change_message_id":"d36976c04d5cb0dcc73e0c07ae1de3554218fdd2","unresolved":false,"context_lines":[{"line_number":106,"context_line":"                   \"from alarm as a \" +"},{"line_number":107,"context_line":"                   \"inner join alarm_definition as ad \" +"},{"line_number":108,"context_line":"                   \"on ad.id \u003d a.alarm_definition_id \" +"},{"line_number":109,"context_line":"                   \"where ad.tenant_id \u003d :tenantId and a.id \u003d :id) as b \" +"},{"line_number":110,"context_line":"                   \"on b.id \u003d alarm.id\";"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"      try (Handle h \u003d db.open()) {"}],"source_content_type":"text/x-java","patch_set":2,"id":"5a890539_f3b3484b","line":109,"updated":"2014-11-26 16:24:01.000000000","message":"i know this was done in the python port, but i\u0027ve been thinking that we should call the temp table \u0027tmptable\u0027 instead of \u0027b\u0027.","commit_id":"2f68c670a8dc7faa50f995b1ca896af8e9816af9"}]}
