feat(assets): Add create API with insert to fail on duplicates

[kotkar-pallavi]

Summary

• Adding a new method to create new assets for Create API.
• The current behavior is to create new assets as part of the upsert method. This new create method can be used to explicitly create assets.
• Create method will throw an exception if we try to insert records with a duplicate primary key instead of doing an update.

Testing Done

Local build and added unit tests

Checklist

• The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
• Links to related issues (if applicable)
• Docs related to the changes have been added/updated (if applicable)

[jphui]

Solid PR, but a few high-level questions and potential gaps

1. (Ebean)LocalAccess is never directly invoked by the metadata-graph-assets service:

    assetModelPegasusInterop.getPegasusAspectsFromAsset(asset).stream()
       .forEach(aspect -> dao.add((URN) pegasusUrn, aspect, auditStamp, pegasusTrackingContext, pegasusParams));

It only has access to (Ebean)LocalDAO, through which LocalAccess is invoked:

EbeanLocalDAO.java:

  public <ASPECT extends RecordTemplate> void updateEntityTables(@Nonnull URN urn, @Nonnull Class<ASPECT> aspectClass) {
...
      ASPECT aspect = toRecordTemplate(aspectClass, result).orElse(null);
      _localAccess.add(urn, aspect, aspectClass, auditStamp, null, false);
...

I believe you will probably need to introduce a new set of methods inside EbeanLocalDao.java -- and / or perhaps BaseLocalDao.java -- in order to access the functionality here.

2. Design choice of adding a new method (done here in this PR) vs adding a flag for existing functionality

This is more of a question than a comment, but is there a reason that CREATE functionality is being added as a new method here as opposed to just adding a boolean flag or something to other existing add() methods?

At a cursory glance it seems like other than the SQL statement used, the logic flow is basically identical.

Maybe you've thought through future augmentations / feature-adds that are needed and it needs to be separate or something similar?

3. Ingestion callback requirements?

Do you know if Ingestion Callback will be needed for CREATE? If you aren't sure what this is, @ZihanLi58 / I / Yihong can explain the concept -- and Zihan can probably elaborate on the need -- but basically this is something supported by Normal Ingestion at the moment and is implemented at the DAO (not Access) level.

[ZihanLi58]

Can you share more requirement about the create API? basically
is it for createAsset or createAspect?
If createAsset, then I think you might need to accept more aspects in one request?
If createAspect, then we probably only want to fail the request when the urn and the aspect are all existing? If urn exist but aspect does not, we should still create it for user?

[ZihanLi58]

Solid PR, but a few high-level questions and potential gaps

1. (Ebean)LocalAccess is never directly invoked by the metadata-graph-assets service:

    assetModelPegasusInterop.getPegasusAspectsFromAsset(asset).stream()
       .forEach(aspect -> dao.add((URN) pegasusUrn, aspect, auditStamp, pegasusTrackingContext, pegasusParams));

It only has access to (Ebean)LocalDAO, through which LocalAccess is invoked:

EbeanLocalDAO.java:

  public <ASPECT extends RecordTemplate> void updateEntityTables(@Nonnull URN urn, @Nonnull Class<ASPECT> aspectClass) {
...
      ASPECT aspect = toRecordTemplate(aspectClass, result).orElse(null);
      _localAccess.add(urn, aspect, aspectClass, auditStamp, null, false);
...

I believe you will probably need to introduce a new set of methods inside EbeanLocalDao.java -- and / or perhaps BaseLocalDao.java -- in order to access the functionality here.

2. Design choice of adding a new method (done here in this PR) vs adding a flag for existing functionality

This is more of a question than a comment, but is there a reason that CREATE functionality is being added as a new method here as opposed to just adding a boolean flag or something to other existing add() methods?

At a cursory glance it seems like other than the SQL statement used, the logic flow is basically identical.

Maybe you've thought through future augmentations / feature-adds that are needed and it needs to be separate or something similar?

3. Ingestion callback requirements?

Do you know if Ingestion Callback will be needed for CREATE? If you aren't sure what this is, @ZihanLi58 / I / Yihong can explain the concept -- and Zihan can probably elaborate on the need -- but basically this is something supported by Normal Ingestion at the moment and is implemented at the DAO (not Access) level.

Based on the question I ask before, you can choose different design choice there. And to make sure the ingestion behavior is consistency, call back is required for create as well

[kotkar-pallavi]

@jphui

1. I have added the method definition and iml in BaseLocalDao.java. Thanks for pointing out.
2. The add method a lot of params not required for create. e.g.: the lambda function is not application on create functionality. I noticed the add method also contains a lot of legacy implementation leading to a very deep call stack. In order to keep the new impl clean and extendable for future improvements I decided to create a separate create method.
3. Added ingestion callbacks.

@ZihanLi58
The create API will be used to explicitly create a new asset. This is avoid the overloaded use of upsert method. We want to keep the interface similar to the existing upsert api, hence allowing create for single aspect instead of a list. Right now, upsert is done iteratively for each aspect.
The create method will throw error if a URN already exists. it does not matter if it contains any assets or not. The user is expected to use the update api to add more aspects and these exceptions will be defined in the asset service.
I added callbacks to create method and exposing it from the BaseLocalDao.java class.

please review. thanks!