refactor: extract duplicate generateIdentifier and getNextSequenceInd…#1193
Merged
maheshkumargangula merged 1 commit intodevelopfrom Feb 27, 2026
Merged
Conversation
…ex to TaxonomyUtil TermActor and CategoryInstanceActor each contained identical private methods: - generateIdentifier(scopeId, code): slugifies scopeId_code - getIndex / getCategoryIndex: computes next SEQUENCE_MEMBERSHIP index from a node Extracted both to a new TaxonomyUtil object in the utils/taxonomy package. Updated all call sites in both actors to use TaxonomyUtil.generateIdentifier() and TaxonomyUtil.getNextSequenceIndex(). Removed the now-redundant private method definitions and the no-longer-needed Slug / RelationTypes imports. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
SonarCloud Analysis Results 🔍Quality Gate Results for Services:Please review the analysis results for each service. Ensure all quality gates are passing before merging. |
maheshkumargangula
approved these changes
Feb 27, 2026
maheshkumargangula
added a commit
that referenced
this pull request
Feb 27, 2026
* refactor: remove redundant and duplicate import statements (#1179) - QuestionActor.scala: remove redundant 'import scala.collection.JavaConverters' (without wildcard) which is entirely subsumed by the subsequent 'import scala.collection.JavaConverters._' - QuestionSetActor.scala: same JavaConverters duplicate as QuestionActor - LockActor.scala: remove 'import scala.collection.immutable.{List, Map}' — both types are already in scope via scala.Predef and importing them explicitly creates a confusing apparent shadow of the builtins Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: prevent FileOutputStream leak in CollectionCSVManager (#1187) new FileOutputStream(csvFile) was passed directly into the OutputStreamWriter constructor. If the OutputStreamWriter constructor threw (e.g. charset encoding error), the FileOutputStream was never assigned to a variable reachable by the finally block, so it leaked. Fix: assign the FileOutputStream to a dedicated variable (fos) first, then pass it to OutputStreamWriter. The finally block now closes fos explicitly, ensuring the file handle is released even when the higher-level writer was never successfully constructed. Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: extract magic literals to named constants (#1189) Replace inline magic strings and numbers with named constants to improve readability and make future changes easier: - ChannelConstants: add COMPOSITE_SEARCH_URL_CONFIG_KEY and COMPOSITE_SEARCH_URL_DEFAULT; ChannelManager uses them in all 4 call sites instead of repeating the raw string literals - DIALConstants: add DEFAULT_ERROR_CORRECTION_LEVEL ("H"), DEFAULT_PIXELS_PER_BLOCK (2), DEFAULT_QR_CODE_MARGIN (3); DIALManager references them instead of inline literals in defaultConfig map - ContentConstants: add MAX_FILE_PATH_SIZE (100); ContentActor uses it for the pre-signed URL file-path length guard and the error message Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: add close() and shutdown hook to KafkaClient to prevent resource leak (#1185) KafkaProducer and KafkaConsumer were never closed, causing open connections and thread leaks when services shut down. This adds: - close(): flushes producer, then closes both producer and consumer with individual try/catch so a failure in one doesn't skip closing the other - registerShutdownHook(): registers a JVM shutdown hook to call close() automatically on normal or signal-based process exit Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * perf: replace LinkedList with ArrayList in SearchCriteria for O(1) indexed access (#1184) LinkedList provides O(n) get(i) which degrades performance when fields and sortOrder are accessed by index during query construction. ArrayList provides O(1) random access with no change to the List<T> interface. All three instantiation sites updated, and the now-unused LinkedList import removed. Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: rename package org.sunbird.mangers -> org.sunbird.managers (#1182) The directory and package declaration contained a typo ('mangers' instead of 'managers'). This caused the file to sit in the wrong package path, making it inconsistent with the surrounding codebase. Changes: - Moved FrameworkManager.scala from sunbird/mangers/ to sunbird/managers/ - Updated package declaration in FrameworkManager.scala - Updated import in FrameworkActor.scala - Updated import in CategoryActor.scala - Updated import in FrameworkManagerTest.scala Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: replace deprecated returnResource() and add JedisPool shutdown hook (#1188) Two resource management issues fixed: 1. returnConnection() used jedisPool.returnResource() which has been deprecated since Jedis 3.0. Replaced with jedis.close() which correctly returns healthy connections to the pool and closes broken ones. 2. JedisPool was never closed, leaving open connections and background threads after process exit. Added closePool() and a JVM shutdown hook to ensure the pool is always properly released on shutdown. Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: replace println/System.out.println with TelemetryManager lo… (#1191) * refactor: replace println/System.out.println with TelemetryManager logging Replace all raw println and System.out.println calls in business logic with structured TelemetryManager calls (debug/error/warn) so that log output is captured by the platform's logging pipeline and not silently dropped in production. Files changed: - ContentActor.scala: warn for missing default licence - ChannelManager.scala: error for category fetch failure (added import) - NodeValidator.scala: debug for singular identifier lookup (added import) - DataSubGraph.scala: debug for subgraph traversal diagnostics (added import) - StorageService.scala: error for getUri exception (added import) - CollectionTOCUtil.scala: error for validateDIALCodes exception - ObjectCategoryDefinitionActor.scala: debug for fallback _all lookup (added import) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Apply suggestion from @maheshkumargangula Co-authored-by: Mahesh Kumar Gangula <6985261+maheshkumargangula@users.noreply.github.com> * Apply suggestion from @maheshkumargangula Co-authored-by: Mahesh Kumar Gangula <6985261+maheshkumargangula@users.noreply.github.com> --------- Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Mahesh Kumar Gangula <6985261+maheshkumargangula@users.noreply.github.com> * fix: prevent duplicate shutdown hooks and replace e.printStackTrace() in CassandraConnector (#1183) Two issues fixed: 1. prepareSession() called registerShutdownHook() on every invocation. If getSession() triggered re-preparation (stale/closed session), multiple hooks were registered for the same operation. Added a shutdownHookRegistered boolean guard to ensure exactly one hook is registered for the lifetime of the process. 2. e.printStackTrace() in the catch block sent stack traces to stderr without context. Replaced with TelemetryManager.error() for consistent, structured logging. 3. close() now iterates sessions individually with per-session error handling so a failure on one session does not abort closing others, and clears the map after shutdown. Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: extract duplicate generateIdentifier and getNextSequenceIndex to TaxonomyUtil (#1193) TermActor and CategoryInstanceActor each contained identical private methods: - generateIdentifier(scopeId, code): slugifies scopeId_code - getIndex / getCategoryIndex: computes next SEQUENCE_MEMBERSHIP index from a node Extracted both to a new TaxonomyUtil object in the utils/taxonomy package. Updated all call sites in both actors to use TaxonomyUtil.generateIdentifier() and TaxonomyUtil.getNextSequenceIndex(). Removed the now-redundant private method definitions and the no-longer-needed Slug / RelationTypes imports. Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: replace .flatMap(f => f) anti-pattern with .flatten (#1181) * refactor: replace .flatMap(f => f) anti-pattern with .flatten .flatMap(f => f) is a verbose identity flatMap — the idiomatic Scala equivalent is simply .flatten. Applied mechanically across 31 files (115 occurrences) to improve readability with no behaviour change. Affected modules: taxonomy-actors, FrameworkManager, FrameworkActor, CategoryInstanceActor, ObjectCategoryDefinitionActor, ContentActor, HierarchyManager, UpdateHierarchyManager, DataNode, DefinitionNode, RelationValidator, VersioningNode, VersionKeyValidator, FrameworkValidator, CopyManager, FlagManager, AcceptFlagManager, AssetCopyManager, DiscardManager, DIALManager, PublishManager, ReviewManager, UploadManager, ItemSetActor, QuestionActor, QuestionSetActor, assessment CopyManager, parseq Task, CollectionMimeTypeMgrImpl. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: replace non-existent TelemetryManager.debug with TelemetryManager.log to resolve build errors * Replace JavaConverters with jdk.CollectionConverters Switch scala.collection.JavaConverters to scala.jdk.CollectionConverters and simplify conversions by replacing JavaConverters.seqAsJavaListConverter(...).asJava with .toList.asJava in QuestionActor and QuestionSetActor. This removes deprecated API usage (Scala 2.13+ compatibility) and keeps runtime behavior unchanged. --------- Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Mahesh Kumar Gangula <mahesh@sanketika.in> --------- Co-authored-by: Kartheek Palla <kartheek@sanketika.in> Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Mahesh Kumar Gangula <6985261+maheshkumargangula@users.noreply.github.com> Co-authored-by: Mahesh Kumar Gangula <mahesh@sanketika.in>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ex to TaxonomyUtil
TermActor and CategoryInstanceActor each contained identical private methods:
Extracted both to a new TaxonomyUtil object in the utils/taxonomy package. Updated all call sites in both actors to use TaxonomyUtil.generateIdentifier() and TaxonomyUtil.getNextSequenceIndex(). Removed the now-redundant private method definitions and the no-longer-needed Slug / RelationTypes imports.