RFR: 8158295 Add a multi-release jar validation mechanism to jar tool

[Oleg G. Barbashov]

27.07.2016 15:34, Oleg G. Barbashov пишет:
>
> 07.07.2016 23:32, Steve Drach пишет:
>> Hi,
>>
>> Please review the following:
>>
>> webrev: http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/ 
>> <http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/>
>> issue: https://bugs.openjdk.java.net/browse/JDK-8158295 
>> <https://bugs.openjdk.java.net/browse/JDK-8158295>
>>
>> This changeset adds a multi-release jar validator to jar tool. After 
>> the jar tool builds a multi-release jar, the potential resultant jar 
>> file is passed to the validator to assure that the jar file meets the 
>> minimal standards of a multi-release jar, in particular that 
>> versioned classes have the same api as base classes they override.  
>> There are other checks, for example warning if two classes are 
>> identical.  If the jar file is invalid, it is not kept, so —create 
>> will not produce a jar file and —update will keep the input jar file.
>>
>> Thanks
>> Steve
>
> 574     private boolean validate(String fname) {
>  575         boolean valid = true;
>  576         Validator validator = new Validator(this);
>  577
>  578         try (JarFile jf = new JarFile(fname)) {
>  579             AtomicBoolean validHolder = new AtomicBoolean(valid);
>  580             jf.stream()
>  581                     .filter(e -> !e.isDirectory())
>  582                     .filter(e -> !e.getName().equals(MANIFEST_NAME))
>  583                     .filter(e -> !e.getName().endsWith(MODULE_INFO))
>  584                     .sorted(entryComparator)
>  585                     .forEachOrdered(je -> {
>  586                         boolean b = validator.accept(je, jf);
>  587                         if (validHolder.get()) validHolder.set(b);
>  588                     });
>  589             valid = validHolder.get();
>  590         } catch (IOException e) {
>  591 error(formatMsg2("error.validator.jarfile.exception", fname, 
> e.getMessage()));
>  592             valid = false;
>  593         } catch (InvalidJarException e) {
>  594             error(formatMsg("error.validator.bad.entry.name", 
> e.getMessage()));
>  595             valid = false;
>  596         }
>  597         return valid;
>  598     }
>
> (IMHO) Using of AtomicBoolean and forEachOrdered() for stateful 
> validator here looks forced. It may be avoided by using regular 
> iterator with "for" loop:
>
>             Stream<JarEntry> sorted = jf.stream()
>                     .filter(e -> !e.isDirectory())
>                     .filter(e -> !e.getName().equals(MANIFEST_NAME))
>                     .filter(e -> !e.getName().endsWith(MODULE_INFO))
>                     .sorted(entryComparator);
>             for (Iterator<JarEntry> iter = sorted.iterator(); 
> iter.hasNext();) {
>                 if (!validator.accept(iter.next(), jf)) {
>                     valid = false;
>                 }
>             }
>
> or even better by storing "valid" state inside the Validator after 
> turning it to regular Consumer<JarEntry>:
>
>         try (JarFile jf = new JarFile(fname)) {
>             Validator validator = new Validator(this, jf);
>             jf.stream()
>                     .filter(e -> !e.isDirectory())
>                     .filter(e -> !e.getName().equals(MANIFEST_NAME))
>                     .filter(e -> !e.getName().endsWith(MODULE_INFO))
>                     .sorted(entryComparator)
>                     .forEachOrdered(validator);
>             return validator.isValidated();
>         } catch (IOException e) {
>             error(formatMsg2("error.validator.jarfile.exception", 
> fname, e.getMessage()));
>             return false;
>         } catch (NumberFormatException e) {
>             error(formatMsg("error.validator.bad.entry.name", 
> e.getMessage()));
>             return false;
>         }
>
> Moreover what is the reason to have an external loop over the jar's 
> entries? Even if we will use several different filter sets in future 
> it would be better to use it as optional parameter for Validator.
>
I mean that the jar file entries iteration code ("580 
jf.stream().filter(e.... ")    ...    588                     }); ) 
processing should be a part of "Validator".