----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55818/#review162601 -----------------------------------------------------------
Fix it, then Ship it! ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileBuilder.java (line 121) <https://reviews.apache.org/r/55818/#comment233969> Instead of using `String.format` eagerly, please use http://google.github.io/guava/releases/snapshot/api/docs/com/google/common/base/Preconditions.html#checkArgument-boolean-java.lang.String-java.lang.Object...- - Attila Doroszlai On Jan. 21, 2017, 6:04 p.m., Balázs Bence Sári wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55818/ > ----------------------------------------------------------- > > (Updated Jan. 21, 2017, 6:04 p.m.) > > > Review request for Ambari, Attila Doroszlai, Attila Magyar, Laszlo Puskas, > Sandor Magyari, and Sebastian Toader. > > > Bugs: AMBARI-19662 > https://issues.apache.org/jira/browse/AMBARI-19662 > > > Repository: ambari > > > Description > ------- > > When the user makes a syntax error in a quick link definition, such as: > > { "linKKK_name": "namenode_ui", "visible": true } > > the filter will be misinterpreted as an accept-all filter, due to the fact > that the parser would expect a proper "link_name" tag to interpret it as a > link name filter. This would lead to unexpected and difficult to debug > behaviour. > > To prevent this, quick link profile evaluation should throw an exception > whenever it finds unknown tags in a filter definition > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileBuilder.java > fca1155 > > ambari-server/src/main/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParser.java > 150b7d4 > > ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileBuilderTest.java > 1cc3fd3 > > ambari-server/src/test/java/org/apache/ambari/server/state/quicklinksprofile/QuickLinksProfileParserTest.java > 57badb8 > ambari-server/src/test/resources/inconsistent_quicklinks_profile_3.json > PRE-CREATION > > Diff: https://reviews.apache.org/r/55818/diff/ > > > Testing > ------- > > - Wrote new unit tests > - Tested manually: installing cluster via blueprint with both good and bad > quick link profile in the request, viewing quick links with both good and bad > quick link profiles set via Ambari settings API > - run the unit test suite for ambari-server. The only failing test was > org.apache.ambari.server.state.ServicePropertiesTest.validatePropertySchemaOfServiceXMLs > which currently fails on the trunk CI too. > > > Thanks, > > Balázs Bence Sári > >
