Re: svn commit: r824220 - in /archiva/trunk/archiva-modules/archiva-base/archiva-repository-layer/src: main/java/org/apache/maven/archiva/repository/scanner/functors/ test/java/org/apache/maven/archiva/repository/metadata/ test/java/org/apache/maven/

[Jevica Arianne Zurbano]

On Tue, Oct 13, 2009 at 8:12 AM, Brett Porter <brett@...> wrote:

>
> On 12/10/2009, at 3:29 PM, jzurbano@... wrote:
>
>  -
>> /**
>>  * ConsumerWantsFilePredicate
>>  *
>> @@ -62,8 +63,19 @@
>>                    // Timestamp finished points to the last successful
>> scan, not this current one.
>>                    if ( basefile.lastModified() < changesSince )
>>                    {
>> -                        // Skip file as no change has occured.
>> -                        satisfies = false;
>> +                        // MRM-1246
>> +                        // compares the lastModified of the version-level
>> (basefile) and the project-level (parent) metadata
>> +                        File parent =
>> basefile.getParentFile().getParentFile();
>> +
>> +                        if ( parent.lastModified() >
>> basefile.lastModified() )
>> +                        {
>> +                            satisfies = true;
>> +                        }
>> +                        else
>> +                        {
>> +                            // Skip file as no change has occurred.
>> +                            satisfies = false;
>> +                        }
>>                    }
>>
>
> is this doing what it intends? It seems to be comparing the timestamp of
> the parent directory, not the parent metadata file, and not all files
> passing through the predicate are metadata files.
>

It does update the parent metadata file. But I missed that not all are of
metadata files.
Will make modifications for this to check the consumer type.


>
> Does disabling this change cause the tests to fail to verify they are
> working?
>

I added a test to check the metadata before and after it has been updated.

But I haven't added a test on the fix yet. I am still figuring out where to
put the test since
the test should include scanning with the consumer.

However, the repo scanning tests are ran before the consumers are built.

Any thoughts on this?


>
> - Brett
>
>

[brettporter]

On 13/10/2009, at 2:52 PM, Jevica Arianne Zurbano wrote:

> On Tue, Oct 13, 2009 at 8:12 AM, Brett Porter <brett@...>  
> wrote:
>
>>
>> On 12/10/2009, at 3:29 PM, jzurbano@... wrote:
>>
>> -
>>> /**
>>> * ConsumerWantsFilePredicate
>>> *
>>> @@ -62,8 +63,19 @@
>>>                   // Timestamp finished points to the last  
>>> successful
>>> scan, not this current one.
>>>                   if ( basefile.lastModified() < changesSince )
>>>                   {
>>> -                        // Skip file as no change has occured.
>>> -                        satisfies = false;
>>> +                        // MRM-1246
>>> +                        // compares the lastModified of the  
>>> version-level
>>> (basefile) and the project-level (parent) metadata
>>> +                        File parent =
>>> basefile.getParentFile().getParentFile();
>>> +
>>> +                        if ( parent.lastModified() >
>>> basefile.lastModified() )
>>> +                        {
>>> +                            satisfies = true;
>>> +                        }
>>> +                        else
>>> +                        {
>>> +                            // Skip file as no change has occurred.
>>> +                            satisfies = false;
>>> +                        }
>>>                   }
>>>
>>
>> is this doing what it intends? It seems to be comparing the  
>> timestamp of
>> the parent directory, not the parent metadata file, and not all files
>> passing through the predicate are metadata files.
>>
>
> It does update the parent metadata file. But I missed that not all  
> are of
> metadata files.
> Will make modifications for this to check the consumer type.

but basefile.getParentFile().getParentFile() is ".." for "maven-
metadata.xml", ie a directory. I don't think it is checking the right  
timestamp. I also don't think building knowledge of the repository  
format into this level of abstraction is appropriate.

>
>
>>
>> Does disabling this change cause the tests to fail to verify they are
>> working?
>>
>
> I added a test to check the metadata before and after it has been  
> updated.
>
> But I haven't added a test on the fix yet. I am still figuring out  
> where to
> put the test since
> the test should include scanning with the consumer.
>
> However, the repo scanning tests are ran before the consumers are  
> built.
>
> Any thoughts on this?

Usually you'd use a mock consumer to test whether it is called when  
expected.

- Brett