Bug 54451 - SearchHistory API change
SearchHistory API change
Status: RESOLVED FIXED
Product: utilities
Classification: Unclassified
Component: Search
4.x
All All
: P1 (vote)
: 4.x
Assigned To: Martin Roskanin
issues@utilities
: API_REVIEW_FAST
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-02-03 10:20 UTC by Martin Roskanin
Modified: 2005-02-15 09:21 UTC (History)
2 users (show)

See Also:
Issue Type: TASK
:


Attachments
API change diffs (5.42 KB, text/plain)
2005-02-03 10:43 UTC, Martin Roskanin
Details
updated diff (7.94 KB, text/plain)
2005-02-03 13:34 UTC, Martin Roskanin
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Roskanin 2005-02-03 10:20:09 UTC
API change is needed because of the fix of issue
#54028
It is necessary to fire further
PropertyChangeEvent to fix it. (Details in the
issue #54028)It is event ADD_TO_HISTORY, that will
be fired after adding new SearchPattern to
history. Old value of event would be null, new
value would be added pattern.
Also pattern checking before adding to history is
a subject of the API change.

If pattern is null, its search expression is null
or empty, or there is already the same pattern
item in the top of the history list, the pattern
will not be added (event will not be fired)

I will create some test(s) and attach it together
with API change diffs.
Comment 1 Martin Roskanin 2005-02-03 10:43:17 UTC
Created attachment 20171 [details]
API change diffs
Comment 2 Jaroslav Tulach 2005-02-03 12:09:44 UTC
1. diff -u is much easier to read
2. should not you increase the version number in manifest.mf to 3.6?
3. in tests it is useful to check that some things do not happen. Like:
>     public void testAddToSearchHistoryListener() throws Exception{
>         final boolean fired[] = new boolean[2];
>         PropertyChangeListener pcl = new PropertyChangeListener(){
>                 public void propertyChange(PropertyChangeEvent evt){
>                     if (evt!=null &&
SearchHistory.ADD_TO_HISTORY.equals(evt.getPropertyName())){
>                         fired[0] = true;
>                     } else {
>                         fired[1] = true;
>                 }
>         };
>         SearchHistory.getDefault().addPropertyChangeListener(pcl);
>        
SearchHistory.getDefault().add(SearchPattern.create("searchtext",true,true,false));
>         SearchHistory.getDefault().removePropertyChangeListener(pcl);
>         assertTrue(fired[0]);
>         assertFalse("Only the expected change is fired", fired[1]);
>     }

Otherwise, this change seems to have all I was asking for.
Comment 3 Martin Roskanin 2005-02-03 13:33:29 UTC
Thanks for the review and test tip.
Ad 1: I am attaching diff -u with recommended test changes. 
ad 2: Should a version be increased if the API change is compatible?
Comment 4 Martin Roskanin 2005-02-03 13:34:22 UTC
Created attachment 20174 [details]
updated diff
Comment 5 mslama 2005-02-03 13:42:43 UTC
Yes version is always increased. And you did it already in apichanges.xml.
Comment 6 Martin Roskanin 2005-02-03 13:56:19 UTC
OK, I have increased a version to 3.6 in manifest.mf
Comment 7 Martin Roskanin 2005-02-14 09:19:14 UTC
I am about to commit the changes within 24 hours
Comment 8 Martin Roskanin 2005-02-15 09:21:33 UTC
fixed in [maintrunk]

/cvs/openidex/manifest.mf,v  <--  manifest.mf
new revision: 1.34; previous revision: 1.33

/cvs/openidex/api/apichanges.xml,v  <--  apichanges.xml
new revision: 1.4; previous revision: 1.3

/cvs/openidex/src/org/openidex/search/SearchHistory.java,v  <-- 
SearchHistory.java
new revision: 1.4; previous revision: 1.3

/cvs/openidex/test/unit/src/org/openidex/search/SearchHistoryTest.java,v
 <--  SearchHistoryTest.java
new revision: 1.2; previous revision: 1.1


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo