Using Atlassian Cache together with Active Objects

Jira Development Jira Data Center
, , , ,
1

Hi!

I am currently developing my own plugin that utilizes active objects. In order to lessen the load on the DB, I want to utilize atlassian-cache for this. I have written code and it seems to work, but I’m unsure if I am thinking correctly. Is my implementation below correct?

public class PluginServiceImpl implements PluginService {

  private final Cache<Integer, Plugin> cache;
  private final CacheSettings cacheSettings = new CacheSettingsBuilder()
            .expireAfterWrite(60, TimeUnit.MINUTES)
            .maxEntries(100000)
            .flushable()
            .replicateAsynchronously()
            .statisticsEnabled()
            .build();
  private static final Logger log = LoggerFactory.getLogger(PluginServiceImpl.class);
  private final ActiveObjects ao;

  @Inject
  public PluginServiceImpl(@JiraImport ActiveObjects ao, @JiraImport CacheManager cacheManager) {
      this.ao = checkNotNull(ao);
      cache = cacheManager.getCache(PluginServiceImpl.class.getName() + ".cache", new PluginCacheLoader(), cacheSettings);
  }

@Override
  public Plugin createPlugin(Boolean active, String title, String description, String pluginIcon, String content) {
    Transaction txn = Txn.begin();
    try {
      final Plugin plugin = ao.create(Plugin.class, new DBParam("CREATED_DATE", new java.util.Date()));
      plugin.setActive(active);
      plugin.setTitle(title);
      plugin.setDescription(description);
      plugin.setPluginIcon(pluginIcon);
      plugin.setContent(content);
      plugin.save();
      cache.putIfAbsent(plugin.getID(), plugin);
      txn.commit();
      return plugin;
    } finally {
      txn.finallyRollbackIfNotCommitted();
    }
  }

@Override
  public JSONObject allActivePluginsAsJSON() {
    Transaction txn = Txn.begin();
    try {
      JSONObject jo = new JSONObject();
      JSONArray ja = new JSONArray();
      Plugin[] plugins = ao.find(Plugin.class, "ACTIVE = ?", "TRUE");
      for (Plugin plugin : plugins) {
        log.error("plugin id: " + plugin.getID());
        log.error("cache result: " + cache.get(plugin.getID()).getContent());
        try {
          JSONObject tempJO = new JSONObject();
          tempJO.put("id", plugin.getID());
          tempJO.put("title", plugin.getTitle());
          tempJO.put("description", plugin.getDescription());
          tempJO.put("plugin_icon", plugin.getPluginIcon());
          tempJO.put("content", plugin.getContent());
          ja.put(tempJO);
        } catch (JSONException ex) {
          txn.finallyRollbackIfNotCommitted();
        }
      }
      jo.put("activePlugins", ja);
      txn.commit();
      return jo;
    } finally {
      txn.finallyRollbackIfNotCommitted();
    }
  }
private class PluginCacheLoader implements CacheLoader<Integer, Plugin> {
    @Override
    @Nonnull
    public Plugin load(@Nonnull Integer pluginId) {
        return getPlugin(pluginId);
    }
  }

}

When I check the log, I can indeed see that it is able to find the active objects in the cache. But this also worked for Plugin objects that were created before the implementation, e.g. I haven’t used “cache.put” on them. Why is this so?


2022-12-16 15:30:33,273+0900 http-nio-2990-exec-18 ERROR admin 930x33819x1 ayk0wu 0:0:0:0:0:0:0:1 / [c.r.j.custompluginapp.ao.PluginServiceImpl] plugin id: 1
2022-12-16 15:30:33,279+0900 http-nio-2990-exec-18 ERROR admin 930x33819x1 ayk0wu 0:0:0:0:0:0:0:1 / [c.r.j.custompluginapp.ao.PluginServiceImpl] cache result: Testing 123
2022-12-16 15:30:33,280+0900 http-nio-2990-exec-18 ERROR admin 930x33819x1 ayk0wu 0:0:0:0:0:0:0:1 / [c.r.j.custompluginapp.ao.PluginServiceImpl] plugin id: 2
2022-12-16 15:30:33,282+0900 http-nio-2990-exec-18 ERROR admin 930x33819x1 ayk0wu 0:0:0:0:0:0:0:1 / [c.r.j.custompluginapp.ao.PluginServiceImpl] cache result: There

Thanks for the help!

2

It looks like you are using the Cache#get() method to retrieve a value from the cache and the Cache#putIfAbsent() method to insert a value into the cache if it does not already exist.

In the createPlugin() method, you are calling cache.putIfAbsent() to insert the newly created Plugin object into the cache if it does not already exist. This means that the Plugin object will be inserted into the cache only if it does not already have an entry for the given key (in this case, the ID of the Plugin object).

In the allActivePluginsAsJSON() method, you are calling cache.get() to retrieve the Plugin object from the cache for each active plugin. If the Plugin object is not present in the cache, the CacheLoader#load() method will be called to load it from the database.

It seems that the CacheLoader#load() method is returning the Plugin object from the database even if it is not present in the cache. This is likely because the CacheLoader#load() method is not checking whether the Plugin object is present in the cache before returning it.

To fix this issue, you can modify the CacheLoader#load() method to check whether the Plugin object is present in the cache before returning it. Here is an example of how you can do this:

private class PluginCacheLoader implements CacheLoader<Integer, Plugin> {
    @Override
    @Nonnull
    public Plugin load(@Nonnull Integer pluginId) {
        Plugin plugin = cache.get(pluginId);
        if (plugin == null) {
            plugin = getPlugin(pluginId);
            cache.putIfAbsent(pluginId, plugin);
        }
        return plugin;
    }
}

This should ensure that the CacheLoader#load() method returns the Plugin object from the cache if it is present, and only loads it from the database if it is not present in the cache.

I hope this helps!

3

Thanks so much for your reply Florian!

I have updated my code and I now have something like this (I included my getPlugin-method so that you can see):

public class PluginServiceImpl implements PluginService {

  private final Cache<Integer, Plugin> cache;
  private final CacheSettings cacheSettings = new CacheSettingsBuilder()
            .expireAfterWrite(60, TimeUnit.MINUTES)
            .maxEntries(100000)
            .flushable()
            .replicateAsynchronously()
            .statisticsEnabled()
            .build();
  private static final Logger log = LoggerFactory.getLogger(PluginServiceImpl.class);
  private final ActiveObjects ao;

  @Inject
  public PluginServiceImpl(@JiraImport ActiveObjects ao, @JiraImport CacheManager cacheManager) {
      this.ao = checkNotNull(ao);
      cache = cacheManager.getCache(PluginServiceImpl.class.getName() + ".cache", new PluginCacheLoader(), cacheSettings);
  }

@Override
  public Plugin getPlugin(int pluginId) {
    Transaction txn = Txn.begin();
    try {
      final Plugin[] plugin = ao.find(Plugin.class, Query.select().where("ID = ?", pluginId));
      if (plugin.length != 1) {
        return null;
      } else {
        txn.commit();
        return plugin[0];
      }
    } finally {
      txn.finallyRollbackIfNotCommitted();
    }
  }

@Override
  public Plugin createPlugin(Boolean active, String title, String description, String pluginIcon, String content) {
    Transaction txn = Txn.begin();
    try {
      final Plugin plugin = ao.create(Plugin.class, new DBParam("CREATED_DATE", new java.util.Date()));
      plugin.setActive(active);
      plugin.setTitle(title);
      plugin.setDescription(description);
      plugin.setPluginIcon(pluginIcon);
      plugin.setContent(content);
      plugin.save();
      cache.put(plugin.getID(), plugin);
      txn.commit();
      return plugin;
    } finally {
      txn.finallyRollbackIfNotCommitted();
    }
  }

@Override
  public JSONObject allActivePluginsAsJSON() {
    Transaction txn = Txn.begin();
    try {
      JSONObject jo = new JSONObject();
      JSONArray ja = new JSONArray();
      Plugin[] plugins = ao.find(Plugin.class, "ACTIVE = ?", "TRUE");
      for (Plugin plugin : plugins) {
        log.error("plugin id: " + plugin.getID());
        log.error("cache result: " + cache.get(plugin.getID()).getContent());
        try {
          JSONObject tempJO = new JSONObject();
          tempJO.put("id", plugin.getID());
          tempJO.put("title", plugin.getTitle());
          tempJO.put("description", plugin.getDescription());
          tempJO.put("plugin_icon", plugin.getPluginIcon());
          tempJO.put("content", plugin.getContent());
          ja.put(tempJO);
        } catch (JSONException ex) {
          txn.finallyRollbackIfNotCommitted();
        }
      }
      jo.put("activePlugins", ja);
      txn.commit();
      return jo;
    } finally {
      txn.finallyRollbackIfNotCommitted();
    }
  }

private class PluginCacheLoader implements CacheLoader<Integer, Plugin> {
    @Override
    @Nonnull
    public Plugin load(@Nonnull Integer pluginId) {
      Plugin plugin = cache.get(pluginId);
      if (plugin == null) {
        plugin = getPlugin(pluginId);
        cache.putIfAbsent(pluginId, plugin);
      }
      return plugin;
    }
  }

}

However, I am bit unsure how I should deal with the allActivePluginsAsJson-method. Currently, to get all plugins, I make a call towards the DB, finding all objects that are active (currently I just test getting the plugin from the cache using a log.error message). This code works just fine, however, since I have to do a “ao.find”-call in order to find these objects, it feels like there is no purpose to use the cache since I already got all objects that I need.

I want to use the cache for this, but I am unsure how to do it in a way so that I don’t put load on the DB. This function will be called on page load in Jira for all users, so I am sure the load will be quite hefty if I just do it from the active-objects table and not cache. Could you help to point me in the right direction? It would be very appreciated.

Also, I noticed that I get this error when overriding the “load()”-method:

cache.putIfAbsent(pluginId, plugin);

Null type safety: The expression of type 'Plugin' needs unchecked conversion to conform to '@Nonnull Plugin'

I guess this is due to the @Nonnull-annontation above the load()-method. Is it safe to ignore this?

Thanks a lot for the help! I will post the final code once I have a working solution!

EDIT: It seems that the proposed code doesn’t work, as it leads to nothing being loaded from the cache. Reading about Guava CacheLoader (which I guess Atlassian Cache is somewhat based upon), something like this should be correct:

// Working code
private class PluginCacheLoader implements CacheLoader<Integer, Plugin> {
    @Override
    @Nonnull
    public Plugin load(@Nonnull Integer pluginId) {
      Plugin plugin = getPlugin(pluginId);
      if (plugin != null) {
        return plugin;
      } else {
        throw new NullPointerException("A plugin with " + pluginId + " doesn't exist.");
      }
    }
  }

The code that doesn’t work is this:

// This code doesn't work
private class PluginCacheLoader implements CacheLoader<Integer, Plugin> {
    @Override
    @Nonnull
    public Plugin load(@Nonnull Integer pluginId) {
      log.error("in the cache");
      Plugin plugin = cache.get(pluginId);
      log.error("cache " + cache.get(pluginId));
      if (cache.get(pluginId) == null) {
        log.error("cache doesn't exist!");
        Plugin plugin = getPlugin(pluginId);
        cache.putIfAbsent(pluginId, plugin);
        return plugin;
      } else {
        return cache.get(pluginId);
      }
    }
  }
4

So I kept investigating and I found these pages that helped me understand my issue:

It turns out that the CacheLoader “load()”-method is only called if the value doesn’t exist in the cache. That means if you try to call the same cached object again, you will get this exception:

java.lang.IllegalStateException: Recursive load of: ...

Therefore, in the load()-method, you should call the desired function immediately and not try to get the cache again. However, in my situation, when I want to use the cache, I don’t know the ids of the desired cached objects, as I just want to get everything that has a certain value in the ActiveObjects table (active = true).

Therefore, I decided to try this solution, where I control the cache manually and only cache objects that has this desired state. Therefore, if a plugin is updated and has its active column changed to false, it will be removed from the cache, and vice versa. However, this means that I don’t use the CacheLoader in any way as of now, but the below CacheLoader-code has been tested and is working correctly.

Here is the code:

@Named
public class PluginServiceImpl implements PluginService {

  private static final Logger log = LoggerFactory.getLogger(PluginServiceImpl.class);

  private final Cache<Integer, Optional<Plugin>> cache;

  @Nonnull
  private final CacheSettings cacheSettings = new CacheSettingsBuilder()
            .expireAfterWrite(60, TimeUnit.MINUTES)
            .maxEntries(100000)
            .flushable()
            .replicateAsynchronously()
            .statisticsEnabled()
            .build();

  private final ActiveObjects ao;

  @Inject
  public PluginServiceImpl(@JiraImport ActiveObjects ao, @JiraImport CacheManager cacheManager) {
      this.ao = checkNotNull(ao);
      cache = cacheManager.getCache(PluginServiceImpl.class.getName() + ".cache", new PluginCacheLoader(), cacheSettings);
  }

  @Override
  public Plugin createPlugin(Boolean active, String title, String description, String pluginIcon, String content) {
    Transaction txn = Txn.begin();
    try {
      final Plugin plugin = ao.create(Plugin.class, new DBParam("CREATED_DATE", new java.util.Date()));
      plugin.setActive(active);
      plugin.setTitle(title);
      plugin.setDescription(description);
      plugin.setPluginIcon(pluginIcon);
      plugin.setContent(content);
      plugin.save();
      cache.put(plugin.getID(), Optional.of(plugin));
      txn.commit();
      return plugin;
    } finally {
      txn.finallyRollbackIfNotCommitted();
    }
  }

  @Override
  public Plugin updatePlugin(int pluginId, Boolean active, String title, String description, String pluginIcon, String content, Date modifiedDate) {
    Transaction txn = Txn.begin();
    try {
      final Plugin[] plugin = ao.find(Plugin.class, Query.select().where("ID = ?", pluginId));
      if (plugin.length != 1) {
        return null;
      } else {
        plugin[0].setActive(active);
        plugin[0].setTitle(title);
        plugin[0].setDescription(description);
        plugin[0].setPluginIcon(pluginIcon);
        plugin[0].setContent(content);
        plugin[0].setModifiedDate(modifiedDate);
        plugin[0].save();
        if (active) {
          cache.put(pluginId, Optional.of(plugin[0]));
        } else {
          cache.remove(pluginId);
        }
        cache.put(pluginId, Optional.of(plugin[0]));
        txn.commit();
        return plugin[0];
      }
    } finally {
      txn.finallyRollbackIfNotCommitted();
    }
  }

  @Override
  public int deletePlugin(int pluginId) {
    Transaction txn = Txn.begin();
    try {
      int noOfDeletedRows = ao.deleteWithSQL(Plugin.class, "ID = ?", pluginId);
      cache.remove(pluginId);
      txn.commit();
      return noOfDeletedRows;
    } finally {
      txn.finallyRollbackIfNotCommitted();
    }
  }

  @Override
  public int deleteAllInactivePlugins() {
    Transaction txn = Txn.begin();
    try {
      final Plugin[] plugins = ao.find(Plugin.class, "ACTIVE = ?", "FALSE");
      for (Plugin plugin : plugins) {
        cache.remove(plugin.getID());
      }
      ao.delete(plugins);
      txn.commit();
      return plugins.length;
    } finally {
      txn.finallyRollbackIfNotCommitted();
    }
  }

  @Override
  public JSONObject allActivePluginsAsJSON() {
    JSONObject jo = new JSONObject();
    JSONArray ja = new JSONArray();
    Collection<Integer> keys = cache.getKeys();
    log.error("keys size: " + keys.size());
    if (keys.size() > 0) {
      for (Integer key : keys) {
        Plugin plugin = cache.get(key).orNull();
        try {
          JSONObject tempJO = new JSONObject();
          tempJO.put("id", plugin.getID());
          tempJO.put("title", plugin.getTitle());
          tempJO.put("description", plugin.getDescription());
          tempJO.put("plugin_icon", plugin.getPluginIcon());
          tempJO.put("content", new HtmlFragment(plugin.getContent()));
          ja.put(tempJO);
        } catch (JSONException ex) {}
      }
    } else {
      Transaction txn = Txn.begin();
      try {
        Plugin[] plugins = ao.find(Plugin.class, "ACTIVE = ?", "TRUE");
        for (Plugin plugin : plugins) {
          cache.put(plugin.getID(), Optional.of(plugin));
          try {
            JSONObject tempJO = new JSONObject();
            tempJO.put("id", plugin.getID());
            tempJO.put("title", plugin.getTitle());
            tempJO.put("description", plugin.getDescription());
            tempJO.put("plugin_icon", plugin.getPluginIcon());
            tempJO.put("content", new HtmlFragment(plugin.getContent()));
            ja.put(tempJO);
          } catch (JSONException ex) {}
        }
        txn.commit();
      } finally {
        txn.finallyRollbackIfNotCommitted();
      }
    }
    jo.put("activePlugins", ja);
    return jo;
  }

  @Override
  public Plugin getPlugin(int pluginId) {
    Transaction txn = Txn.begin();
    try {
      final Plugin[] plugin = ao.find(Plugin.class, Query.select().where("ID = ?", pluginId));
      if (plugin.length != 1) {
        return null;
      } else {
        txn.commit();
        return plugin[0];
      }
    } finally {
      txn.finallyRollbackIfNotCommitted();
    }
  }

  @Override
  public Plugin getPlugin(int pluginId, Boolean active) {
    log.error("in the getplugin class");
    Transaction txn = Txn.begin();
    try {
      final Plugin[] plugin = ao.find(Plugin.class, Query.select().where("ID = ? AND ACTIVE = ?", pluginId, active));
      if (plugin.length != 1) {
        return null;
      } else {
        txn.commit();
        return plugin[0];
      }
    } finally {
      txn.finallyRollbackIfNotCommitted();
    }
  }

  private class PluginCacheLoader implements CacheLoader<Integer, Optional<Plugin>> {
    @Override
    @Nonnull
    public Optional<Plugin> load(@Nonnull Integer pluginId) {
      log.error("in the cache");
      return Optional.of(getPlugin(pluginId, Boolean.TRUE));
    }
  }

@florian Thank you a lot of your help. It really helped me to find the problem I was facing.

If you have any comments/suggestions to my solution, I would really appreciate it!