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".
