You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@archiva.apache.org by Joakim Erdfelt <jo...@erdfelt.com> on 2008/04/09 06:39:56 UTC
[plexus work] archiva-checksums
I've been taking a stab and removing some of our dependencies on various
plexus components.
First up, plexus-digest.
I've taken the varied code from many locations and come up with a stand
alone archiva-checksum lib/component that I hope to be able to integrate
into archiva/trunk.
So ... uhm ... consider this a call for discussion. ;-)
- Joakim
Re: [plexus work] archiva-checksums
Posted by Joakim Erdfelt <jo...@erdfelt.com>.
I've made all of the changes.
I'll let it percolate over the next 24 hours (or so) and then integrate
it into trunk.
- Joakim
Re: [plexus work] archiva-checksums
Posted by Joakim Erdfelt <jo...@erdfelt.com>.
Brett Porter wrote:
>>> - I think it can use more tests
>>
>> Agreed. Have any special use cases you want fleshed out?
>
> Nah, just coverage for the other classes.
Will do.
>
>>> - what about passing referenceFile as a final field, since every
>>> method uses it?
>>
>> I started ChecksumFile as a utility class, but that might not be the
>> best decision.
>> Any opinions if I make ChecksumFile an instantiated class?
>>
>> ChecksumFile cf = new ChecksumFile(new File("random.jar"));
>> if( cf.valid() == false ) {
>> cf.createChecksum( ChecksumAlgorithm.SHA1 );
>> }
>>
>> wdyt?
>
> would that be validateChecksum( ChecksumAlgorithm) ?
Yup. that is more appropriate.
Still, do you see any issue with making the class instantiated vs utility?
- Joakim
Re: [plexus work] archiva-checksums
Posted by Brett Porter <br...@apache.org>.
On 12/04/2008, at 12:36 PM, Joakim Erdfelt wrote:
> Brett Porter wrote:
>> Looks good to me. I say roll it into trunk.
>>
>> A couple of uncertainties:
>> - isn't doFinal required?
>
> doFinal ?
sorry, got confused with JCE :) it's digest() in MessageDigest, which
is there.
>
>
>> - I think it can use more tests
>
> Agreed. Have any special use cases you want fleshed out?
Nah, just coverage for the other classes.
>> - what about passing referenceFile as a final field, since every
>> method uses it?
>
> I started ChecksumFile as a utility class, but that might not be the
> best decision.
> Any opinions if I make ChecksumFile an instantiated class?
>
> ChecksumFile cf = new ChecksumFile(new File("random.jar"));
> if( cf.valid() == false ) {
> cf.createChecksum( ChecksumAlgorithm.SHA1 );
> }
>
> wdyt?
would that be validateChecksum( ChecksumAlgorithm) ?
- Brett
--
Brett Porter
brett@apache.org
http://blogs.exist.com/bporter/
Re: [plexus work] archiva-checksums
Posted by Joakim Erdfelt <jo...@erdfelt.com>.
Brett Porter wrote:
> Looks good to me. I say roll it into trunk.
>
> A couple of uncertainties:
> - isn't doFinal required?
doFinal ?
> - I think it can use more tests
Agreed. Have any special use cases you want fleshed out?
>
>
> Here's some nitpicking that might reduce the amount of code and make
> it clearer:
> - rename Hash to ChecksumAlgorithm (might as well be consistent and
> use checksum everywhere instead of alternating through Hash, Digest,
> Checksum?)
> - likewise Hasher -> Checksum
Name changes make sense, I'll make these changes this weekend.
> - there some duplicated code in isValidChecksums and fixChecksums that
> can be factored out
Good catch, I'll put that on my todo's too.
> - what about passing referenceFile as a final field, since every
> method uses it?
I started ChecksumFile as a utility class, but that might not be the
best decision.
Any opinions if I make ChecksumFile an instantiated class?
ChecksumFile cf = new ChecksumFile(new File("random.jar"));
if( cf.valid() == false ) {
cf.createChecksum( ChecksumAlgorithm.SHA1 );
}
wdyt?
> - but if not, it'd be better if the arg order of getChecksumFile and
> createChecksum and calculateChecksum were consistent (I would go with
> File first)
> - I'd rename ChecksumFile as ChecksummedFile
> - parseChecksum can be private?
I left that as public just to have it be easily unit tested. ;-)
> - the two isValidChecksum(s) methods do different things.
> isValidChecksum should be isValidChecksum(Hash hash) { return
> isValidChecksums( new Hash[] { hash } ); }. Maybe another static
> convenience method is needed for the checksum file checkChecksumFile(
> File checksumFile ).
Legitimate fix. Adding to my todo.
>
> - Brett
>
> On 09/04/2008, at 2:39 PM, Joakim Erdfelt wrote:
>> I've been taking a stab and removing some of our dependencies on
>> various plexus components.
>>
>> First up, plexus-digest.
>>
>> I've taken the varied code from many locations and come up with a
>> stand alone archiva-checksum lib/component that I hope to be able to
>> integrate into archiva/trunk.
>>
>> So ... uhm ... consider this a call for discussion. ;-)
>>
>> - Joakim
>
> --
> Brett Porter
> brett@apache.org
> http://blogs.exist.com/bporter/
>
Re: [plexus work] archiva-checksums
Posted by Brett Porter <br...@apache.org>.
Looks good to me. I say roll it into trunk.
A couple of uncertainties:
- isn't doFinal required?
- I think it can use more tests
Here's some nitpicking that might reduce the amount of code and make
it clearer:
- rename Hash to ChecksumAlgorithm (might as well be consistent and
use checksum everywhere instead of alternating through Hash, Digest,
Checksum?)
- likewise Hasher -> Checksum
- there some duplicated code in isValidChecksums and fixChecksums that
can be factored out
- what about passing referenceFile as a final field, since every
method uses it?
- but if not, it'd be better if the arg order of getChecksumFile
and createChecksum and calculateChecksum were consistent (I would go
with File first)
- I'd rename ChecksumFile as ChecksummedFile
- parseChecksum can be private?
- the two isValidChecksum(s) methods do different things.
isValidChecksum should be isValidChecksum(Hash hash) { return
isValidChecksums( new Hash[] { hash } ); }. Maybe another static
convenience method is needed for the checksum file
checkChecksumFile( File checksumFile ).
- Brett
On 09/04/2008, at 2:39 PM, Joakim Erdfelt wrote:
> I've been taking a stab and removing some of our dependencies on
> various plexus components.
>
> First up, plexus-digest.
>
> I've taken the varied code from many locations and come up with a
> stand alone archiva-checksum lib/component that I hope to be able to
> integrate into archiva/trunk.
>
> So ... uhm ... consider this a call for discussion. ;-)
>
> - Joakim
--
Brett Porter
brett@apache.org
http://blogs.exist.com/bporter/
Re: [plexus work] archiva-checksums
Posted by Joakim Erdfelt <jo...@erdfelt.com>.
Not sure I understand what you are asking. :-(
There was a suggestion in the #archiva irc channel that we should offer
the code in archiva-checksum to the commons-codec project. An idea I
fully support, but have no idea how to proceed with. ;-)
Also, Brett asked a passing question about how well archiva-checksum
handles streaming.
I'd like to get some more information on expectations, then I can put
together a few test cases to ensure that functionality.
- Joakim
James William Dumay wrote:
> Joakim,
> Is this based on any work from commons or have you implemented this
> stuff from whats provided in the JDK?
>
> James
>
> On Tue, 2008-04-08 at 21:54 -0700, Joakim Erdfelt wrote:
>
>> Joakim Erdfelt wrote:
>>
>>> I've been taking a stab and removing some of our dependencies on
>>> various plexus components.
>>>
>>> First up, plexus-digest.
>>>
>>> I've taken the varied code from many locations and come up with a
>>> stand alone archiva-checksum lib/component that I hope to be able to
>>> integrate into archiva/trunk.
>>>
>>> So ... uhm ... consider this a call for discussion. ;-)
>>>
>>> - Joakim
>>>
>>>
>> Some more details...
>>
>> These are 4 classes present in archiva-checksum
>> 1) org/apache/archiva/checksum/Hash.java
>> 2) org/apache/archiva/checksum/Hasher.java
>> 3) org/apache/archiva/checksum/Hex.java
>> 4) org/apache/archiva/checksum/ChecksumFile.java
>>
>> This is how the replacements work ...
>>
>> Hash.class is an enum identifying the types of Hash functions (currently
>> only SHA1 and MD5), with a few details on ids that the hash techniques
>> expose.
>> This is similar in scope, but not a 100% replacement for ...
>> org/codehaus/plexus/digest/Sha1Digester.java
>> org/codehaus/plexus/digest/Md5Digester.java
>> org/codehaus/plexus/digest/StreamingSha1Digester.java
>>
>> Hasher.class is the main hashing routines, stream based, and has a few
>> optimizations for performing bulk or group hashing based on a single
>> stream, without processing the stream multiple times (one for each hash).
>> Hasher.class replaces the following classes in plexus-digest
>> org/codehaus/plexus/digest/Digester.java
>> org/codehaus/plexus/digest/StreamingDigester.java
>>
>> Hex.class is just a simple utility class to convert a byte array to a
>> Hex string.
>> It is a suitable replacement for plexus-digest
>> org/codehaus/plexus/digest/Hex.java
>>
>> ChecksumFile.class is the file specific implementation, for dealing with
>> the .md5 and .sha1 files (parsing, validating, creating, etc..)
>> It is a replacement for plexus-digest
>> org/codehaus/plexus/digest/ChecksumFile.java
>> org/codehaus/plexus/digest/DigestUtils.java
>> And also covers the logic present in archiva-common too
>> org/apache/maven/archiva/common/utils/Checksums.java
>>
>> - Joakim
>>
>
>
Re: [plexus work] archiva-checksums
Posted by James William Dumay <ja...@atlassian.com>.
Joakim,
Is this based on any work from commons or have you implemented this
stuff from whats provided in the JDK?
James
On Tue, 2008-04-08 at 21:54 -0700, Joakim Erdfelt wrote:
> Joakim Erdfelt wrote:
> > I've been taking a stab and removing some of our dependencies on
> > various plexus components.
> >
> > First up, plexus-digest.
> >
> > I've taken the varied code from many locations and come up with a
> > stand alone archiva-checksum lib/component that I hope to be able to
> > integrate into archiva/trunk.
> >
> > So ... uhm ... consider this a call for discussion. ;-)
> >
> > - Joakim
> >
> Some more details...
>
> These are 4 classes present in archiva-checksum
> 1) org/apache/archiva/checksum/Hash.java
> 2) org/apache/archiva/checksum/Hasher.java
> 3) org/apache/archiva/checksum/Hex.java
> 4) org/apache/archiva/checksum/ChecksumFile.java
>
> This is how the replacements work ...
>
> Hash.class is an enum identifying the types of Hash functions (currently
> only SHA1 and MD5), with a few details on ids that the hash techniques
> expose.
> This is similar in scope, but not a 100% replacement for ...
> org/codehaus/plexus/digest/Sha1Digester.java
> org/codehaus/plexus/digest/Md5Digester.java
> org/codehaus/plexus/digest/StreamingSha1Digester.java
>
> Hasher.class is the main hashing routines, stream based, and has a few
> optimizations for performing bulk or group hashing based on a single
> stream, without processing the stream multiple times (one for each hash).
> Hasher.class replaces the following classes in plexus-digest
> org/codehaus/plexus/digest/Digester.java
> org/codehaus/plexus/digest/StreamingDigester.java
>
> Hex.class is just a simple utility class to convert a byte array to a
> Hex string.
> It is a suitable replacement for plexus-digest
> org/codehaus/plexus/digest/Hex.java
>
> ChecksumFile.class is the file specific implementation, for dealing with
> the .md5 and .sha1 files (parsing, validating, creating, etc..)
> It is a replacement for plexus-digest
> org/codehaus/plexus/digest/ChecksumFile.java
> org/codehaus/plexus/digest/DigestUtils.java
> And also covers the logic present in archiva-common too
> org/apache/maven/archiva/common/utils/Checksums.java
>
> - Joakim
Re: [plexus work] archiva-checksums
Posted by Joakim Erdfelt <jo...@erdfelt.com>.
Joakim Erdfelt wrote:
> I've been taking a stab and removing some of our dependencies on
> various plexus components.
>
> First up, plexus-digest.
>
> I've taken the varied code from many locations and come up with a
> stand alone archiva-checksum lib/component that I hope to be able to
> integrate into archiva/trunk.
>
> So ... uhm ... consider this a call for discussion. ;-)
>
> - Joakim
>
Some more details...
These are 4 classes present in archiva-checksum
1) org/apache/archiva/checksum/Hash.java
2) org/apache/archiva/checksum/Hasher.java
3) org/apache/archiva/checksum/Hex.java
4) org/apache/archiva/checksum/ChecksumFile.java
This is how the replacements work ...
Hash.class is an enum identifying the types of Hash functions (currently
only SHA1 and MD5), with a few details on ids that the hash techniques
expose.
This is similar in scope, but not a 100% replacement for ...
org/codehaus/plexus/digest/Sha1Digester.java
org/codehaus/plexus/digest/Md5Digester.java
org/codehaus/plexus/digest/StreamingSha1Digester.java
Hasher.class is the main hashing routines, stream based, and has a few
optimizations for performing bulk or group hashing based on a single
stream, without processing the stream multiple times (one for each hash).
Hasher.class replaces the following classes in plexus-digest
org/codehaus/plexus/digest/Digester.java
org/codehaus/plexus/digest/StreamingDigester.java
Hex.class is just a simple utility class to convert a byte array to a
Hex string.
It is a suitable replacement for plexus-digest
org/codehaus/plexus/digest/Hex.java
ChecksumFile.class is the file specific implementation, for dealing with
the .md5 and .sha1 files (parsing, validating, creating, etc..)
It is a replacement for plexus-digest
org/codehaus/plexus/digest/ChecksumFile.java
org/codehaus/plexus/digest/DigestUtils.java
And also covers the logic present in archiva-common too
org/apache/maven/archiva/common/utils/Checksums.java
- Joakim