Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Vector usages with the java.util.ArrayList. #234

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Replace Vector usages with the java.util.ArrayList. #234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions jspwiki-event/src/main/java/org/apache/wiki/event/WikiEventManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ Licensed to the Apache Software Foundation (ASF) under one
import java.util.ConcurrentModificationException;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
import java.util.Vector;

/**
* A singleton class that manages the addition and removal of WikiEvent listeners to a event source, as well as the firing of events
Expand Down Expand Up @@ -135,8 +135,8 @@ public final class WikiEventManager {
/* The Map of client object to WikiEventDelegate. */
private final Map< Object, WikiEventDelegate > m_delegates = new HashMap<>();

/* The Vector containing any preloaded WikiEventDelegates. */
private final Vector< WikiEventDelegate > m_preloadCache = new Vector<>();
/* The List containing any preloaded WikiEventDelegates. */
private final List< WikiEventDelegate > m_preloadCache = Collections.synchronizedList( new ArrayList<>() );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pr-wide comment:

please note that, per Collections.synchronizedList javadoc,

It is imperative that the user manually synchronize on the returned list when traversing it via Iterator, Spliterator or Stream: [...]

enhanced fors use internally an Iterator, so they also must be synchronized. Would you mind rechecking the access to the new Lists in order to ensure they remain synchronized?

Copy link
Member Author

@arturobernalg arturobernalg May 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @juanpablo-santos
Another approach we can take to avoid explicitly synchronizing on the list during iteration is to use a concurrent collection instead of a synchronized collection.

Instead of using Collections.synchronizedList(), we could use java.util.concurrent.CopyOnWriteArrayList or java.util.concurrent.ConcurrentLinkedQueue, both of which are thread-safe and don't require explicit synchronization during iteration.
WDYT?


/* Singleton instance of the WikiEventManager. */
private static WikiEventManager c_instance;
Expand Down Expand Up @@ -324,7 +324,7 @@ private WikiEventDelegate getDelegateFor( final Object client ) {
} else if( !m_preloadCache.isEmpty() ) {
// then see if any of the cached delegates match the class of the incoming client
for( int i = m_preloadCache.size()-1 ; i >= 0 ; i-- ) { // start with most-recently added
final WikiEventDelegate delegate = m_preloadCache.elementAt( i );
final WikiEventDelegate delegate = m_preloadCache.get( i );
if( delegate.getClientClass() == null || delegate.getClientClass().equals( client.getClass() ) ) {
// we have a hit, so use it, but only on a client we haven't seen before
if( !m_delegates.containsKey( client ) ) {
Expand Down
6 changes: 3 additions & 3 deletions jspwiki-main/src/main/java/org/apache/wiki/auth/acl/AclImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ Licensed to the Apache Software Foundation (ASF) under one
import java.security.Permission;
import java.security.Principal;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Enumeration;
import java.util.List;
import java.util.Vector;


/**
Expand All @@ -37,7 +37,7 @@ Licensed to the Apache Software Foundation (ASF) under one
public class AclImpl implements Acl, Serializable {

private static final long serialVersionUID = 1L;
private final Vector< AclEntry > m_entries = new Vector<>();
private final List< AclEntry > m_entries = Collections.synchronizedList( new ArrayList<>() );

/**
* Constructs a new AclImpl instance.
Expand Down Expand Up @@ -109,7 +109,7 @@ public synchronized boolean removeEntry( final AclEntry entry ) {
/** {@inheritDoc} */
@Override
public Enumeration< AclEntry > aclEntries() {
return m_entries.elements();
return Collections.enumeration(m_entries);
}

/** {@inheritDoc} */
Expand Down
6 changes: 4 additions & 2 deletions jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/Group.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ Licensed to the Apache Software Foundation (ASF) under one
import org.apache.wiki.auth.GroupPrincipal;

import java.security.Principal;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
import java.util.Vector;
import java.util.List;

/**
* <p>
Expand Down Expand Up @@ -61,7 +63,7 @@ public class Group {

static final String[] RESTRICTED_GROUPNAMES = new String[] { "Anonymous", "All", "Asserted", "Authenticated" };

private final Vector<Principal> m_members = new Vector<>();
private final List<Principal> m_members = Collections.synchronizedList( new ArrayList<>() );

private String m_creator;

Expand Down
10 changes: 6 additions & 4 deletions jspwiki-main/src/main/java/org/apache/wiki/filters/PingWeblogsComFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ Licensed to the Apache Software Foundation (ASF) under one

import java.net.MalformedURLException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Hashtable;
import java.util.List;
import java.util.Properties;
import java.util.Vector;

Expand Down Expand Up @@ -80,13 +82,13 @@ public void postSave( final Context context, final String pagecontent ) {

try {
final XmlRpcClient xmlrpc = new XmlRpcClient(m_pingURL);
final Vector< String > params = new Vector<>();
params.addElement( "The Butt Ugly Weblog" ); // FIXME: Must be settable
params.addElement( engine.getURL( ContextEnum.PAGE_VIEW.getRequestContext(), blogName, null ) );
final List< String > params = new ArrayList<>();
params.add( "The Butt Ugly Weblog" ); // FIXME: Must be settable
params.add( engine.getURL( ContextEnum.PAGE_VIEW.getRequestContext(), blogName, null ) );

LOG.debug( "Pinging weblogs.com with URL: {}", engine.getURL( ContextEnum.PAGE_VIEW.getRequestContext(), blogName, null ) );

xmlrpc.executeAsync("weblogUpdates.ping", params,
xmlrpc.executeAsync("weblogUpdates.ping", new Vector<>(params),
new AsyncCallback() {
@Override
public void handleError( final Exception ex, final URL url, final String method ) {
Expand Down
6 changes: 3 additions & 3 deletions jspwiki-main/src/main/java/org/apache/wiki/filters/SpamFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,13 @@ Licensed to the Apache Software Foundation (ASF) under one
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.Iterator;
import java.util.List;
import java.util.Properties;
import java.util.Random;
import java.util.StringTokenizer;
import java.util.Vector;
import java.util.concurrent.ThreadLocalRandom;


Expand Down Expand Up @@ -199,11 +199,11 @@ public class SpamFilter extends BasePageFilter {
private static final Logger C_SPAMLOG = LogManager.getLogger( "SpamLog" );
private static final Logger LOG = LogManager.getLogger( SpamFilter.class );

private final Vector<Host> m_temporaryBanList = new Vector<>();
private final List <Host> m_temporaryBanList = Collections.synchronizedList( new ArrayList<>() );

private int m_banTime = 60; // minutes

private final Vector<Host> m_lastModifications = new Vector<>();
private final List <Host> m_lastModifications = Collections.synchronizedList( new ArrayList<>() );

/** How many times a single IP address can change a page per minute? */
private int m_limitSinglePageChanges = 5;
Expand Down
12 changes: 6 additions & 6 deletions jspwiki-main/src/main/java/org/apache/wiki/ui/TemplateManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Licensed to the Apache Software Foundation (ASF) under one

import javax.servlet.jsp.PageContext;
import javax.servlet.jsp.jstl.fmt.LocaleSupport;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.LinkedHashMap;
Expand All @@ -37,7 +38,6 @@ Licensed to the Apache Software Foundation (ASF) under one
import java.util.ResourceBundle;
import java.util.Set;
import java.util.TimeZone;
import java.util.Vector;


/**
Expand Down Expand Up @@ -316,14 +316,14 @@ static String getJSLocalizedStrings( final Context context ) {
* @param resource The resource to add.
*/
static void addResourceRequest( final Context ctx, final String type, final String resource ) {
HashMap< String, Vector< String > > resourcemap = ctx.getVariable( RESOURCE_INCLUDES );
HashMap< String, List< String > > resourcemap = ctx.getVariable( RESOURCE_INCLUDES );
if( resourcemap == null ) {
resourcemap = new HashMap<>();
}

Vector< String > resources = resourcemap.get( type );
List< String > resources = resourcemap.get( type );
if( resources == null ) {
resources = new Vector<>();
resources = new ArrayList<>();
}
String resolvedResource = resource;
if( StringUtils.startsWith( resource, "engine://" ) ) {
Expand Down Expand Up @@ -368,12 +368,12 @@ static void addResourceRequest( final Context ctx, final String type, final Stri
* @return a String array for the resource requests
*/
static String[] getResourceRequests( final Context ctx, final String type ) {
final HashMap< String, Vector< String > > hm = ctx.getVariable( RESOURCE_INCLUDES );
final HashMap< String, List< String > > hm = ctx.getVariable( RESOURCE_INCLUDES );
if( hm == null ) {
return new String[0];
}

final Vector<String> resources = hm.get( type );
final List<String> resources = hm.get( type );
if( resources == null ){
return new String[0];
}
Expand Down
10 changes: 6 additions & 4 deletions jspwiki-main/src/main/java/org/apache/wiki/xmlrpc/AbstractRPCHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@ Licensed to the Apache Software Foundation (ASF) under one
import org.apache.xmlrpc.AuthenticationFailed;

import java.security.Permission;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.Date;
import java.util.Hashtable;
import java.util.List;
import java.util.Set;
import java.util.Vector;

/**
* Provides definitions for RPC handler routines.
Expand Down Expand Up @@ -69,16 +70,16 @@ public void initialize( final Context context ) {

protected abstract Hashtable encodeWikiPage( Page p );

public Vector getRecentChanges( final Date since ) {
public List<?> geRecentChanges(final Date since ){
checkPermission( PagePermission.VIEW );
final Set< Page > pages = m_engine.getManager( PageManager.class ).getRecentChanges();
final Vector< Hashtable< ?, ? > > result = new Vector<>();
final List< Hashtable< ?, ? > > result = new ArrayList<>();

// Transform UTC into local time.
final Calendar cal = Calendar.getInstance();
cal.setTime( since );
cal.add( Calendar.MILLISECOND, cal.get( Calendar.ZONE_OFFSET ) +
(cal.getTimeZone().inDaylightTime( since ) ? cal.get( Calendar.DST_OFFSET ) : 0 ) );
(cal.getTimeZone().inDaylightTime( since ) ? cal.get( Calendar.DST_OFFSET ) : 0 ) );

for( final Page page : pages ) {
if( page.getLastModified().after( cal.getTime() ) ) {
Expand Down Expand Up @@ -113,4 +114,5 @@ public int getRPCVersionSupported() {
return RPC_VERSION;
}

public abstract List< Hashtable< String, Object > > getRecentChanges(Date since);
}
13 changes: 7 additions & 6 deletions jspwiki-main/src/test/java/org/apache/wiki/parser/JSPWikiMarkupParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ Licensed to the Apache Software Foundation (ASF) under one
import java.io.IOException;
import java.io.StringReader;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Vector;
import java.util.List;

import static org.apache.wiki.TestEngine.with;

Expand All @@ -53,7 +54,7 @@ public class JSPWikiMarkupParserTest {

static final String PAGE_NAME = "testpage";

Vector< String > created = new Vector<>();
List< String > created = new ArrayList<>();

TestEngine testEngine = TestEngine.build( with( "jspwiki.translatorReader.matchEnglishPlurals", "true" ) );

Expand All @@ -65,7 +66,7 @@ public void tearDown() {

private void newPage( final String name ) throws WikiException {
testEngine.saveText( name, "<test>" );
created.addElement( name );
created.add( name );
}

private String translate( final String src ) throws IOException {
Expand Down Expand Up @@ -461,7 +462,7 @@ public void testAttachmentLink() throws Exception {
public void testAttachmentLink2() throws Exception {
final TestEngine testEngine2 = TestEngine.build( with( "jspwiki.encoding", StandardCharsets.ISO_8859_1.name() ) );
testEngine2.saveText( "Test", "foo " );
created.addElement( "Test" );
created.add( "Test" );

final Attachment att = Wiki.contents().attachment( testEngine2, "Test", "TestAtt.txt" );
att.setAuthor( "FirstPost" );
Expand All @@ -478,7 +479,7 @@ public void testAttachmentLink2() throws Exception {
public void testAttachmentLink3() throws Exception {
final TestEngine testEngine2 = TestEngine.build();
testEngine2.saveText( "TestPage", "foo " );
created.addElement( "TestPage" );
created.add( "TestPage" );

final Attachment att = Wiki.contents().attachment( testEngine2, "TestPage", "TestAtt.txt" );
att.setAuthor( "FirstPost" );
Expand All @@ -494,7 +495,7 @@ public void testAttachmentLink3() throws Exception {
public void testAttachmentLink4() throws Exception {
final TestEngine testEngine2 = TestEngine.build();
testEngine2.saveText( "TestPage", "foo " );
created.addElement( "TestPage" );
created.add( "TestPage" );

final Attachment att = Wiki.contents().attachment( testEngine2, "TestPage", "TestAtt.txt" );
att.setAuthor( "FirstPost" );
Expand Down
3 changes: 1 addition & 2 deletions jspwiki-main/src/test/java/org/apache/wiki/providers/CounterProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ Licensed to the Apache Software Foundation (ASF) under one
import java.util.Date;
import java.util.List;
import java.util.Properties;
import java.util.Vector;

/**
* A provider who counts the hits to different parts.
Expand Down Expand Up @@ -136,7 +135,7 @@ public int getPageCount()
@Override
public List< Page > getVersionHistory( final String page )
{
return new Vector<>();
return new ArrayList<>();
}

@Override
Expand Down
3 changes: 1 addition & 2 deletions jspwiki-main/src/test/java/org/apache/wiki/providers/VerySimpleProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ Licensed to the Apache Software Foundation (ASF) under one
import java.util.Date;
import java.util.List;
import java.util.Properties;
import java.util.Vector;

/**
* This is a simple provider that is used by some of the tests. It has some specific behaviours, like it always contains a single page.
Expand Down Expand Up @@ -137,7 +136,7 @@ public int getPageCount()
@Override
public List< Page > getVersionHistory( final String page )
{
return new Vector<>();
return new ArrayList<>();
}

/**
Expand Down
4 changes: 2 additions & 2 deletions jspwiki-main/src/test/java/org/apache/wiki/ui/TemplateManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ Licensed to the Apache Software Foundation (ASF) under one
import org.mockito.junit.jupiter.MockitoExtension;

import java.util.HashMap;
import java.util.List;
import java.util.Properties;
import java.util.Vector;
import java.util.stream.Stream;


Expand All @@ -55,7 +55,7 @@ void shouldCheckAddResourceRequest( final String type, final String res, final S
}

Mockito.doAnswer( invocationOnMock -> {
final HashMap< String, Vector< String > > map = invocationOnMock.getArgument( 1, HashMap.class );
final HashMap< String, List< String >> map = invocationOnMock.getArgument( 1, HashMap.class );
Assertions.assertEquals( expected, map.get( type ).get( 0 ) );
return null;
} ).when( ctx ).setVariable( Mockito.eq( TemplateManager.RESOURCE_INCLUDES ), Mockito.any( HashMap.class ) );
Expand Down
Loading