orientdb-labs icon indicating copy to clipboard operation
orientdb-labs copied to clipboard

[OEP 6] Public-internal API separation on a package level

Open taburet opened this issue 9 years ago • 7 comments

Summary:

Separate public and internal classes/interfaces/enums on a package level to achieve a cleaner API. Move everything that is not a part of the public API to internal packages. An internal package is a package which name contains ".internal." infix compared to its public counterpart, if any.

Goals:

  • Obvious separation between public and internal APIs on a per class/interface/enum basis.

Non-Goals:

  • Packaging reorganization for the existing public APIs.
  • Remove, deprecate or lower visibility of anything.
  • Add API annotations/javadocs to existing code base.

Success metrics:

  • Public-internal separation clearly visible to everyone.
  • Public things sit in their unchanged public packages.
  • Internal things sit in their new internal packages.
  • Nothing public is broken.

Motivation:

It's really hard to understand what is considered public and what is not in the current ODB code base. This is bad. ODB developers may break public things unexpectedly. Developers of the ODB-based solutions may introduce dependencies on things that are considered internal by ODB developers.

Description:

Package-based separation will provide clean indication of a public API. For users, if you don't see "internal" in a package name, it's public, you may use anything from it safely. If you see "internal" in the package name, you still may use anything from it, but at your own risk. Java API documentation should be updated to mention this fact.

For ODB contributors, if you see "internal" in a package name, you may change anything without a fear of breaking things. If you don't see "internal", be careful while changing something, it's a public API. Think twice before removing, changing or deprecating something, think triple before adding something new, we have to support it.

For each Java file in the ODB code base following steps should be taken:

  1. Decide is it public or not.
  2. If it's public, skip it.
  3. Create a new internal package to host it, if package doesn't exist.
  4. Move it to the new package.

For consistency, we must choose only one internal package naming scheme:

  1. $project.$module.internal.$subpackage: com.orientechnologies.orient.etl.internal.source, com.orientechnologies.orient.core.internal.command.script, com.orientechnologies.lucene.internal.
  2. $project.$module.$subpackage.internal: com.orientechnologies.orient.etl.source.internal, com.orientechnologies.orient.core.command.script.internal, com.orientechnologies.lucene.internal.
  3. $project.internal.$module.$subpackage: com.orientechnologies.orient.internal.etl.source, com.orientechnologies.orient.internal.core.command.script, com.orientechnologies.internal.lucene.

Alternatives:

  • Leave it as it is.
  • Apply API annotations/javadocs, leave packages alone.
  • Declare internal things as package-private. May introduce problems with visibility/inheritance/testability.

Risks and assumptions:

  • No one may really know what is public/internal at this point.
  • Too many ODB users may already have dependencies on internal things.
  • Too much team communication may be required to establish the separation.
  • Too many classes/interfaces may have a mixture of public-internal methods.

Impact matrix

  • [ ] Storage engine
  • [ ] SQL
  • [ ] Protocols
  • [ ] Indexes
  • [ ] Console
  • [x] Java API
  • [ ] Geospatial
  • [ ] Lucene
  • [ ] Security
  • [ ] Hooks
  • [ ] EE

taburet avatar Jun 23 '16 13:06 taburet

Just come to my mind. We can easily migrate to JDK 9 modules after such refactoring and support OSGi modules.

andrii0lomakin avatar Jun 27 '16 07:06 andrii0lomakin

Also, +1 for the third approach. The structure of our modules is too fuzzy.

andrii0lomakin avatar Jun 27 '16 07:06 andrii0lomakin

yes agree on the convention, i've no preferences on the actual format, so please just choose one, in any case with the new API( OEP 2,3,5 ) will be much more hard for user interact with internal api, and will be more clear for us as well what is public because it need to be declared on some specific interfaces.

tglman avatar Jun 30 '16 16:06 tglman

Hi,

Another thing i was thinking on this topic, as soon as all the refactor of the API is done we could move the public api to an "orientdb-api" project, and the client and core(embedded) will depends on it, this will make really clear what is public what is not, and make possible to have a client independent to the core, so no need to change the packages to internal.

WDYT?

tglman avatar Oct 05 '16 11:10 tglman

@tglman good idea, it's a good practice, I think, to define a public API in terms of interfaces and place them into a separate "module".

taburet avatar Oct 05 '16 11:10 taburet

I'm not sure in which release we can get this completely done, but i'll start to work on that direction an all the refactors i'm working on, so i we agree with the module solution we can declare this OEP done and close it.

tglman avatar Oct 11 '16 12:10 tglman

@tglman wait guys we close ? Is it already implemented ?

andrii0lomakin avatar Oct 11 '16 13:10 andrii0lomakin